diff --git a/README.md b/README.md index b5aa064bc8..46ee64e1bb 100644 --- a/README.md +++ b/README.md @@ -148,14 +148,15 @@ module.exports = { ### Miscellaneous -| Name                                               | Description | 💼 | 🔧 | 💡 | -| :--------------------------------------------------------------------------------------------------------------------- | :---------------------------------------------------------------------------------------------------------------------------- | :- | :- | :- | -| [named-functions-in-promises](docs/rules/named-functions-in-promises.md) | enforce usage of named functions in promises | | | | -| [no-html-safe](docs/rules/no-html-safe.md) | disallow the use of `htmlSafe` | | | | -| [no-incorrect-calls-with-inline-anonymous-functions](docs/rules/no-incorrect-calls-with-inline-anonymous-functions.md) | disallow inline anonymous functions as arguments to `debounce`, `once`, and `scheduleOnce` | ✅ | | | -| [no-invalid-debug-function-arguments](docs/rules/no-invalid-debug-function-arguments.md) | disallow usages of Ember's `assert()` / `warn()` / `deprecate()` functions that have the arguments passed in the wrong order. | ✅ | | | -| [no-restricted-property-modifications](docs/rules/no-restricted-property-modifications.md) | disallow modifying the specified properties | | 🔧 | | -| [require-fetch-import](docs/rules/require-fetch-import.md) | enforce explicit import for `fetch()` | | | | +| Name                                               | Description | 💼 | 🔧 | 💡 | +| :--------------------------------------------------------------------------------------------------------------------- | :----------------------------------------------------------------------------------------------------------------------------------------------- | :- | :- | :- | +| [named-functions-in-promises](docs/rules/named-functions-in-promises.md) | enforce usage of named functions in promises | | | | +| [no-decorators-before-export](docs/rules/no-decorators-before-export.md) | disallow the use of a class decorator on an export declaration. Enforces use of the decorator on the class directly before exporting the result. | ✅ | 🔧 | | +| [no-html-safe](docs/rules/no-html-safe.md) | disallow the use of `htmlSafe` | | | | +| [no-incorrect-calls-with-inline-anonymous-functions](docs/rules/no-incorrect-calls-with-inline-anonymous-functions.md) | disallow inline anonymous functions as arguments to `debounce`, `once`, and `scheduleOnce` | ✅ | | | +| [no-invalid-debug-function-arguments](docs/rules/no-invalid-debug-function-arguments.md) | disallow usages of Ember's `assert()` / `warn()` / `deprecate()` functions that have the arguments passed in the wrong order. | ✅ | | | +| [no-restricted-property-modifications](docs/rules/no-restricted-property-modifications.md) | disallow modifying the specified properties | | 🔧 | | +| [require-fetch-import](docs/rules/require-fetch-import.md) | enforce explicit import for `fetch()` | | | | ### Routes diff --git a/docs/rules/no-decorators-before-export.md b/docs/rules/no-decorators-before-export.md new file mode 100644 index 0000000000..69ceab99c7 --- /dev/null +++ b/docs/rules/no-decorators-before-export.md @@ -0,0 +1,101 @@ +# ember/no-decorators-before-export + +💼 This rule is enabled in the ✅ `recommended` [config](https://github.com/ember-cli/eslint-plugin-ember#-configurations). + +🔧 This rule is automatically fixable by the [`--fix` CLI option](https://eslint.org/docs/latest/user-guide/command-line-interface#--fix). + + + +Disallow the use of a class decorator on an export declaration. Enforces use of the decorator on the class directly before exporting the result. + +## Background + +In the very first TC39 proposal for decorators, a syntax option was considered to allow the following: + +```ts +import classic from 'ember-classic-decorators'; +import Component from '@ember/component'; + +@classic +export default class MyComponent extends Component {} +``` + +However, in the various specs that followed, including in the current proposal that is stage-3, this syntax was explicitly disallowed. The problem it causes is a little more apparent when considering it as a function and placing everything on one line: + +```ts +@classic export default class MyComponent extends Component {} +``` + +The correct way to specify decorators is onto the class itself prior to exporting the class. This mechanism works with the version of decorators which Ember adopted, and is the positioning the stage-3 spec adopted as well. + +```ts +import classic from 'ember-classic-decorators'; +import Component from '@ember/component'; + +@classic +class MyComponent extends Component {} + +export default MyComponent; +``` + +Unfortunately, in order to maximize compatibility with existing decorators, the second rendition of the spec - the rendition of the spec which Ember adopted (there have been four) - allowed this syntax to still be used if using the babel plugin AND also explicitly allowing it in the plugin config. Ember chose to be maximally capatible and enabled this option. + +This can create problems: + +1) being non-spec, non-babel parsers do not support it. This means codemods using jscodeshift, rollup using acorn, and any number of other tools or code compilers error when they encounter these files. + +2) its easy for transpilation to go very wrong if custom babel transforms do not account for this case. + +3) The babel transform for decorators never included support for applying them to unnamed default exports. + +Considering that this syntax was non-spec even at the point that ember adopted decorators, and considering that the correct approach works as expected, we lint against the non-standard approach. + +## Examples + +### Good + +```ts +import classic from 'ember-classic-decorators'; +import Component from '@ember/component'; + +@classic +class MyComponent extends Component {} + +export default MyComponent; +``` + +```ts +import classic from 'ember-classic-decorators'; +import EmberObject from '@ember/object'; + +@classic +class MyKlass extends EmberObject {} + +export { MyKlass }; +``` + +### Bad + +```ts +import classic from 'ember-classic-decorators'; +import Component from '@ember/component'; + +@classic +export default class extends Component {} +``` + +```ts +import classic from 'ember-classic-decorators'; +import Component from '@ember/component'; + +@classic +export default class MyComponent extends Component {} +``` + +```ts +import classic from 'ember-classic-decorators'; +import EmberObject from '@ember/object'; + +@classic +export class MyKlass extends EmberObject {} +``` diff --git a/lib/recommended-rules.js b/lib/recommended-rules.js index 62d9d0b60b..41fad8c204 100644 --- a/lib/recommended-rules.js +++ b/lib/recommended-rules.js @@ -23,6 +23,7 @@ module.exports = { "ember/no-component-lifecycle-hooks": "error", "ember/no-computed-properties-in-native-classes": "error", "ember/no-controller-access-in-routes": "error", + "ember/no-decorators-before-export": "error", "ember/no-deeply-nested-dependent-keys-with-each": "error", "ember/no-duplicate-dependent-keys": "error", "ember/no-ember-super-in-es-classes": "error", @@ -70,4 +71,4 @@ module.exports = { "ember/routes-segments-snake-case": "error", "ember/use-brace-expansion": "error", "ember/use-ember-data-rfc-395-imports": "error" -} \ No newline at end of file +} diff --git a/lib/rules/no-decorators-before-export.js b/lib/rules/no-decorators-before-export.js new file mode 100644 index 0000000000..6042f3f56a --- /dev/null +++ b/lib/rules/no-decorators-before-export.js @@ -0,0 +1,73 @@ +'use strict'; + +/** @type {import('eslint').Rule.RuleModule} */ + +function reportInvalidSyntax(context, node) { + if (isInvalidSyntax(node)) { + const isDefaultExport = node.type === 'ExportDefaultDeclaration'; + const isNamed = node.declaration.id !== null; + const name = isNamed ? node.declaration.id.name : ''; + const numDecorators = node.declaration.decorators.length; + + const message = `Usage of class decorator${numDecorators > 1 ? 's' : ''} on the ${ + isNamed ? '' : 'un-named ' + }${isDefaultExport ? 'default ' : ''}export${ + isNamed ? ` ${name}` : '' + } must occur prior to exporting the class.`; + + if (isNamed) { + const sourceCode = context.getSourceCode(); + const src = sourceCode.getText(node); + context.report({ + node, + message, + fix(fixer) { + if (isDefaultExport) { + let newSrc = src.replace(`export default class ${name}`, `class ${name}`); + newSrc = `${newSrc}\nexport default ${name};\n`; + return fixer.replaceText(node, newSrc); + } + let newSrc = src.replace(`export class ${name}`, `class ${name}`); + newSrc = `${newSrc}\nexport { ${name} };\n`; + return fixer.replaceText(node, newSrc); + }, + }); + } else { + context.report({ node, message }); + } + } +} + +function isInvalidSyntax(node) { + return ( + node.declaration && + node.declaration.type === 'ClassDeclaration' && + node.declaration.decorators?.length + ); +} + +module.exports = { + meta: { + type: 'problem', + docs: { + description: + 'disallow the use of a class decorator on an export declaration. Enforces use of the decorator on the class directly before exporting the result.', + category: 'Miscellaneous', + recommended: true, + url: 'https://github.com/ember-cli/eslint-plugin-ember/tree/master/docs/rules/no-decorators-before-export.md', + }, + fixable: 'code', + schema: [], + }, + + create(context) { + return { + ExportDefaultDeclaration(node) { + return reportInvalidSyntax(context, node); + }, + ExportNamedDeclaration(node) { + return reportInvalidSyntax(context, node); + }, + }; + }, +}; diff --git a/tests/__snapshots__/recommended.js.snap b/tests/__snapshots__/recommended.js.snap index c52e7bdd9c..9f07a902f6 100644 --- a/tests/__snapshots__/recommended.js.snap +++ b/tests/__snapshots__/recommended.js.snap @@ -20,6 +20,7 @@ exports[`recommended rules has the right list 1`] = ` "no-component-lifecycle-hooks", "no-computed-properties-in-native-classes", "no-controller-access-in-routes", + "no-decorators-before-export", "no-deeply-nested-dependent-keys-with-each", "no-duplicate-dependent-keys", "no-ember-super-in-es-classes", diff --git a/tests/lib/rules/no-decorators-before-export.js b/tests/lib/rules/no-decorators-before-export.js new file mode 100644 index 0000000000..c95169c2f3 --- /dev/null +++ b/tests/lib/rules/no-decorators-before-export.js @@ -0,0 +1,70 @@ +const rule = require('../../../lib/rules/no-decorators-before-export'); +const RuleTester = require('eslint').RuleTester; + +const eslintTester = new RuleTester({ + parser: require.resolve('@babel/eslint-parser'), + parserOptions: { + ecmaVersion: 2022, + sourceType: 'module', + requireConfigFile: false, + babelOptions: { + babelrc: false, + plugins: [['@babel/plugin-proposal-decorators', { decoratorsBeforeExport: true }]], + }, + }, +}); + +eslintTester.run('no-decorators-before-export', rule, { + valid: [ + 'export default class {}', + 'export default class Foo {}', + 'class Foo {}\nexport default Foo;', + 'export class Foo {}', + 'class Foo {}\nexport { Foo };', + "import classic from 'ember-classic-decorators';\n@classic\nclass Foo {}\nexport { Foo };", + "import classic from 'ember-classic-decorators';\n@classic\nclass Foo {}\nexport default Foo;", + ], + invalid: [ + // basic + { + code: "import classic from 'ember-classic-decorators';\n@classic\nexport class Foo {}", + output: + "import classic from 'ember-classic-decorators';\n@classic\nclass Foo {}\nexport { Foo };\n", + errors: [ + 'Usage of class decorator on the export Foo must occur prior to exporting the class.', + ], + }, + { + code: "import classic from 'ember-classic-decorators';\n@classic\nexport default class Foo {}", + output: + "import classic from 'ember-classic-decorators';\n@classic\nclass Foo {}\nexport default Foo;\n", + errors: [ + 'Usage of class decorator on the default export Foo must occur prior to exporting the class.', + ], + }, + { + code: "import classic from 'ember-classic-decorators';\n@classic\nexport default class {}", + output: null, + errors: [ + 'Usage of class decorator on the un-named default export must occur prior to exporting the class.', + ], + }, + // ensure interop + { + code: "import classic from 'ember-classic-decorators';\n@classic\nexport class Foo {\n someProp = false;\n}\nexport class Bar {};\n", + output: + "import classic from 'ember-classic-decorators';\n@classic\nclass Foo {\n someProp = false;\n}\nexport { Foo };\n\nexport class Bar {};\n", + errors: [ + 'Usage of class decorator on the export Foo must occur prior to exporting the class.', + ], + }, + { + code: "import classic from 'ember-classic-decorators';\n@classic\nexport default class Foo {\n someProp = false;\n}\nexport class Bar {};\n", + output: + "import classic from 'ember-classic-decorators';\n@classic\nclass Foo {\n someProp = false;\n}\nexport default Foo;\n\nexport class Bar {};\n", + errors: [ + 'Usage of class decorator on the default export Foo must occur prior to exporting the class.', + ], + }, + ], +});