Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

More progress removing prototype extensions #2601

Merged
merged 6 commits into from
Nov 1, 2024

Conversation

RobbieTheWagner
Copy link
Member

Description

Screenshots

@patricklx
Copy link
Collaborator

patricklx commented Oct 25, 2024

it looks like computed.x are incompatible with tracked array. e.g. computed.and in controllers/promise-tree.js.

@RobbieTheWagner
Copy link
Member Author

it looks like computed.x are incompatible with tracked array. e.g. computed.and in controllers/promise-tree.js.

@patricklx I am not sure I understand. What is the fix here? I do not see tracked arrays in that controller.

@patricklx
Copy link
Collaborator

it looks like computed.x are incompatible with tracked array. e.g. computed.and in controllers/promise-tree.js.

@patricklx I am not sure I understand. What is the fix here? I do not see tracked arrays in that controller.

The model is a tracked array. The fix is to use getters instead of computed

@RobbieTheWagner
Copy link
Member Author

RobbieTheWagner commented Oct 25, 2024

@patricklx I updated, but it's got a new error Global error: Uncaught Error: Assertion Failed: EmberObject.create no longer supports defining computed properties. Define computed properties using extend() or reopen() before calling create()

Removing the computed.filter fixes that, but then the test still fails. I was not sure what we wanted to do here.

@RobbieTheWagner RobbieTheWagner force-pushed the remove-more-array-prototype-extensions branch from 752f402 to 068d8f9 Compare October 25, 2024 17:24
@patricklx
Copy link
Collaborator

computed.filter should also be replaced with getter.
I think the issue is in models/promise . There is a custom decorator which returns a computed

@RobbieTheWagner
Copy link
Member Author

@patricklx do you mean dateComputed? Do you know how we would fix it?

@RobbieTheWagner
Copy link
Member Author

@patricklx do you think this is good to merge now or did you see anything else we needed to tweak?

@patricklx
Copy link
Collaborator

@patricklx do you think this is good to merge now or did you see anything else we needed to tweak?

there are still some errors like assembler.find(...).get is not a function, not sure if its releated to prototype extension

@RobbieTheWagner
Copy link
Member Author

@patricklx yeah, this PR is not meant to fix all errors, just a stepping stone.

@RobbieTheWagner RobbieTheWagner merged commit 6408d35 into main Nov 1, 2024
14 of 16 checks passed
@RobbieTheWagner RobbieTheWagner deleted the remove-more-array-prototype-extensions branch November 1, 2024 17:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants