Skip to content

Conversation

tgvrssanthosh
Copy link
Contributor

Summary

Added auto-fixer for no-array-prototype-extensions which replaces getEach with map.

Description

Recently, the proposal to deprecate array prototype extensions has been approved and got merged. The plan is to add autoFixers to replace ember array prototype extensions with the native array methods. In this PR, an auto fixer has been added which will replace the ember array method getEach with the native array method map.

Testing

Modified test case to check if the right output is generated after the fix.

@tgvrssanthosh
Copy link
Contributor Author

@bmish Please help review this PR as well. Thank you!

@bmish bmish changed the title Add getEach to map autofixer for no-array-prototype-extensions rule Add getEach to map autofixer for no-array-prototype-extensions rule Oct 14, 2022
@bmish
Copy link
Member

bmish commented Oct 14, 2022

@tgvrssanthosh can you fix the merge conflict?

@bmish
Copy link
Member

bmish commented Oct 14, 2022

@tgvrssanthosh amazing work so far, a few questions:

  1. It looks like we just have firstObject and lastObject left (Rules that need autofixers #1556) when you plan to get to those? I could do a release once everything is in ideally.
  2. Can you try testing the rule autofixer on a real codebase to make sure we don't have obvious bugs before a release?

@bmish
Copy link
Member

bmish commented Oct 14, 2022

I realized that I missed a lot of methods from MutableArray in my list in #1556 so I have added those to the list.

Also started testing the autofixers on my codebase and opened #1634 for a tweak.

@tgvrssanthosh
Copy link
Contributor Author

@bmish Resolved merge conflicts. Can you please take a look?

Copy link
Member

@bmish bmish left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@bmish bmish merged commit 321dec3 into ember-cli:master Oct 17, 2022
@tgvrssanthosh
Copy link
Contributor Author

tgvrssanthosh commented Oct 17, 2022

@tgvrssanthosh amazing work so far, a few questions:

  1. It looks like we just have firstObject and lastObject left (Rules that need autofixers #1556) when you plan to get to those? I could do a release once everything is in ideally.
  2. Can you try testing the rule autofixer on a real codebase to make sure we don't have obvious bugs before a release?

@bmish I don't think we can write autofixers for mutation methods. As mutation methods are linked to reactivity, we need to rely on @tracked decorator or TrackedArray from tracked-built-ins package and i don't think it is straightforward to come up with autofixers.

@tgvrssanthosh
Copy link
Contributor Author

@bmish Since we came up with autofixers for all the functions other than mutation methods, I believe we can release a minor version.

@bmish
Copy link
Member

bmish commented Oct 17, 2022

@tgvrssanthosh can we still handle firstObject and lastObject at least for now?

@tgvrssanthosh
Copy link
Contributor Author

@tgvrssanthosh can we still handle firstObject and lastObject at least for now?

@bmish Even firstObject and lastObject are plagued with the same problems as explained in the following guide.
https://github.com/smilland/deprecation-app/blob/deprecate-array-prototype-extensions/content/ember/v5/deprecate-array-prototype-extensions.md#observable-properties

@bmish
Copy link
Member

bmish commented Oct 17, 2022

@tgvrssanthosh got it. I'll plan to release everything today along with #1395 as soon as I can get it merged.

@bmish
Copy link
Member

bmish commented Oct 17, 2022

@tgvrssanthosh can you add a note to the rule doc explaining that everything is autofixable except MutableArray methods and firstObject/lastObject with a brief explanation of why and link to documentation about why it's unsafe?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants