Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Deprecate Ember.A #864
base: master
Are you sure you want to change the base?
Deprecate Ember.A #864
Changes from 1 commit
53b64e0
796c13b
1f27a56
12bbd64
360302b
e5c1ed9
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
I think we also may should deprecate
Ember.NativeArray
as part of this. I'd also like to deprecateEmber.Array
altogether, but I'm not sure if that should be a separate thing or not. At the very least it would require the work we're doing here.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.
I think it may just be sufficient to say that maintaining behavior that diverges from a true native Array unnecessarily adds to maintenance and potential confusion. It's actually possible to make Ember.Array work and not cause problems for Ember Data (I think), but it would just be nicer to have it gone!
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.
✅
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.
I think we may also be able to fix this, but it's definitely clear that Ember.A does weird stuff!
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.
💯
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.
This is a very good reason to deprecate Ember.A, ASAP.
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.
Credit to @runspired, I stole this from #848 (comment)
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.
I've started doing some of this work in emberjs/ember.js#20245. This change shouldn't actually require a flag since it's how Ember.A claims to work. However, we could add a short-lived flag to make it easier to validate the behavior in some real apps. Once we successfully validated we'd just remove the flag and have it always on. I don't think we'd have to publicize it in this case.
Once we successfully validate this, I'd like to add individual deprecations to the custom methods on the wrapper. That would help us to quickly identify cases where we're actually relying on the Ember.A behavior.
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.
I'm not sure if I need to express this in the RFC or if this is just for information / reference. I agree with most of it, though I don't share your faith that no one is relying on this behavior. I think it is probably fair to say that the number of apps intentionally relying in this is low, however I think the side effect may have turned into a feature for many.
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.
I think we're already planning on doing this. It wouldn't be considered breaking since the docs always said that Ember.A returns a new object. I think (I hope) it's rare for people to actually rely on the mutating behavior.