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
RFC: Deprecate array prototype extensions #1
RFC: Deprecate array prototype extensions #1
Changes from 8 commits
92b9401
c7e689e
7de7d1d
b28f04e
afc0fed
26b5b73
f9010fa
6823f5a
9d471fa
3117290
bb91b28
fa8b6d2
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.
Have you done any research yet into what can and cannot be codemodded, and what the level of reliability of those codemods is? If so, that would be great to add 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.
Will a codemod have any runtime information available (beyond just the static analysis that's available in a lint rule)? If so, it would be more useful than just a lint rule autofixer. If not, or either way really, I'd love to see as much autofixing capability added to the lint rule ember/no-array-prototype-extensions as possible. Note that the lint rule employs a variety of heuristics to avoid false positives given the limitations of static analysis.
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.
@bmish I don't think so, unless we did a telemetry-powered codemod like the native class codemod did. Which is doable, but also a lot more effort.
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 see, then personally I think it'd be worth focusing on building out the lint rule autofixer, as that would be easily accessible to basically all Ember users who will already have eslint-plugin-ember installed and integrated into their IDE/tooling.
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.
@bmish an auto-fixer is good, but it also has significant limitations: part of the trick here is the way these methods continue to exist on
EmberArray
. I am talking separately with the Framework team about whether we can deprecate or otherwise separate that out in the period between now and 5.0, because of exactly this complexity and entanglement. 😩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.
Thank you folks, I just opened the PR in public repo, emberjs#848 let's move our discussions there! @chriskrycho @bmish