Skip to content

Commit

Permalink
Remove no-array-prototype-extensions rule from recommended config (
Browse files Browse the repository at this point in the history
…#1562)

* Remove no-array-prototype-extensions from recommended

The `no-array-prototype-extensions` rule is trying to do an impossible thing. It's going to produce a never-ending stream of false positives. You cannot statically tell which calls are actually relying on the array prototype extensions.

A correct implementation is probably possible, but only in typescript (where eslint rules can take advantage of type information), not in javascript.

I'm all in favor of pushing people away from using array prototype extensions, but that is going to require runtime implementation. It cannot be checked statically. A rule that is so often wrong will just encourage people not to trust it anyway. I don't think it should be in the recommended set.

Examples:

#1561
#1552
#1547
#1546
#1544
#1543
#1538
#1539
#1536

* docs: mentioned recommend rule removal in changelog

* docs: explain why no-array-prototype-extensions is not in recommended config

Co-authored-by: Bryan Mishkin <698306+bmish@users.noreply.github.com>
  • Loading branch information
ef4 and bmish committed Aug 18, 2022
1 parent 7b76958 commit e7ffcb0
Show file tree
Hide file tree
Showing 6 changed files with 5 additions and 7 deletions.
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@
## v11.0.0 (2022-07-20)

#### :boom: Breaking Change
* [#1517](https://github.com/ember-cli/eslint-plugin-ember/pull/1517) Add `no-array-prototype-extensions` as `recommended` rule ([@bmish](https://github.com/bmish))
* [#1517](https://github.com/ember-cli/eslint-plugin-ember/pull/1517) Add `no-array-prototype-extensions` as `recommended` rule (NOTE: removed as `recommended` in v11.0.6) ([@bmish](https://github.com/bmish))
* [#1515](https://github.com/ember-cli/eslint-plugin-ember/pull/1515) Drop support for ESLint v6 ([@bmish](https://github.com/bmish))
* [#1318](https://github.com/ember-cli/eslint-plugin-ember/pull/1318) Drop support for Node 10, 12, 15, 17 ([@aggmoulik](https://github.com/aggmoulik))
* [#1519](https://github.com/ember-cli/eslint-plugin-ember/pull/1519) Enable `useOptionalChaining` option by default for `no-get` rule ([@bmish](https://github.com/bmish))
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ Rules are grouped by category to help you understand their purpose. Each rule ha
|:--------|:------------|:---------------|:-----------|:---------------|
| [closure-actions](./docs/rules/closure-actions.md) | enforce usage of closure actions || | |
| [new-module-imports](./docs/rules/new-module-imports.md) | enforce using "New Module Imports" from Ember RFC #176 || | |
| [no-array-prototype-extensions](./docs/rules/no-array-prototype-extensions.md) | disallow usage of Ember's `Array` prototype extensions | | | |
| [no-array-prototype-extensions](./docs/rules/no-array-prototype-extensions.md) | disallow usage of Ember's `Array` prototype extensions | | | |
| [no-function-prototype-extensions](./docs/rules/no-function-prototype-extensions.md) | disallow usage of Ember's `function` prototype extensions || | |
| [no-mixins](./docs/rules/no-mixins.md) | disallow the usage of mixins || | |
| [no-new-mixins](./docs/rules/no-new-mixins.md) | disallow the creation of new mixins || | |
Expand Down
4 changes: 2 additions & 2 deletions docs/rules/no-array-prototype-extensions.md
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
# no-array-prototype-extensions

✅ The `"extends": "plugin:ember/recommended"` property in a configuration file enables this rule.

By default, Ember extends certain native JavaScript objects with additional methods. This can lead to problems in some situations. One example is relying on these methods in an addon that is used inside an app that has the extensions disabled.

The prototype extensions for the `Array` object will likely become deprecated in the future.
Expand All @@ -12,6 +10,8 @@ Some alternatives:
* Use lodash helper functions instead of `.uniqBy()`, `.sortBy()` in Ember modules
* Use immutable update style with `@tracked` properties or `TrackedArray` from `tracked-built-ins` instead of `.pushObject`, `removeObject` in Ember modules

Note: this rule is not in the `recommended` configuration because of the risk of false positives.

## Rule Details

This rule will disallow method calls that match any of the forbidden `Array` prototype extension method names.
Expand Down
1 change: 0 additions & 1 deletion lib/recommended-rules.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ module.exports = {
"ember/jquery-ember-run": "error",
"ember/new-module-imports": "error",
"ember/no-actions-hash": "error",
"ember/no-array-prototype-extensions": "error",
"ember/no-arrow-function-computed-properties": "error",
"ember/no-assignment-of-untracked-properties-used-in-tracking-contexts": "error",
"ember/no-attrs-in-components": "error",
Expand Down
2 changes: 1 addition & 1 deletion lib/rules/no-array-prototype-extensions.js
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ module.exports = {
docs: {
description: "disallow usage of Ember's `Array` prototype extensions",
category: 'Deprecations',
recommended: true,
recommended: false,
url: 'https://github.com/ember-cli/eslint-plugin-ember/tree/master/docs/rules/no-array-prototype-extensions.md',
},
fixable: null,
Expand Down
1 change: 0 additions & 1 deletion tests/__snapshots__/recommended.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ Array [
"jquery-ember-run",
"new-module-imports",
"no-actions-hash",
"no-array-prototype-extensions",
"no-arrow-function-computed-properties",
"no-assignment-of-untracked-properties-used-in-tracking-contexts",
"no-attrs-in-components",
Expand Down

0 comments on commit e7ffcb0

Please sign in to comment.