-
-
Notifications
You must be signed in to change notification settings - Fork 407
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
Deprecation of copy and Copyable #322
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wonderfully researched and written @lupestro! Thank you for putting this together.
text/0322-deprecate-copy-copyable.md
Outdated
|
||
Those using the add-on would need to mechanically adjust any uses of `myArray.copy(deep)` to `copy(myArray, deep)` in order to avoid the deprecation message. | ||
|
||
I cannot see a way to notify the user through deprecation at runtime about their own objects that are extending `Copyable` and contain `copy()` methods that will no longer be called when the deprecated `Ember.copy()` method goes away. Our best bet for this might be to supply a new eslint warning that flags the import of `Copyable`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can implement a deprecation at “extend” time such that anything including the copyable mixin received the deprecation (even if copy is never called on it), happy to deep dive the details with you in slack...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That would be welcome - but that would include all arrays, no?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"at extend time" - Since extend
is defined in system.core-object
, also in ember-runtime
, implementing a deprecation at that level might be architecturally sound. The check would be a small tax on every extend, though, which seems severe. @rwjblue Was that where you intended to put it? Or somewhere else in the chain of activity "at extend time"?
Let’s do this!! |
This looks great. This is a general JS problem that should not be part of Ember. |
My initial reaction was "What?! No, I neeeed that!" but yeah I'd be fine switching to an addon. Plus I'd love to change the mechanics of how it copies (or rather that it doesn't copy) arrays. Maybe I'd be better of building my own |
Discussed with the core team (last Friday) and we are 👍 on moving this into Final Comment Period. |
I have removed all usage of |
Thank for all the hard work here, I super excited to be moving forward! |
Here's the initial complete draft