From de59887f9d78af3b323fdb9807d105584b6f2b91 Mon Sep 17 00:00:00 2001 From: NullVoxPopuli <199018+NullVoxPopuli@users.noreply.github.com> Date: Wed, 18 Mar 2026 18:26:18 -0400 Subject: [PATCH 1/2] Extract rule: template-simple-modifiers --- README.md | 1 + docs/rules/template-simple-modifiers.md | 45 ++++++++++ lib/rules/template-simple-modifiers.js | 57 +++++++++++++ tests/lib/rules/template-simple-modifiers.js | 87 ++++++++++++++++++++ 4 files changed, 190 insertions(+) create mode 100644 docs/rules/template-simple-modifiers.md create mode 100644 lib/rules/template-simple-modifiers.js create mode 100644 tests/lib/rules/template-simple-modifiers.js diff --git a/README.md b/README.md index 640e28aa5a..8ba068a131 100644 --- a/README.md +++ b/README.md @@ -245,6 +245,7 @@ rules in templates can be disabled with eslint directives with mustache or html | [template-no-obsolete-elements](docs/rules/template-no-obsolete-elements.md) | disallow obsolete HTML elements | | | | | [template-no-outlet-outside-routes](docs/rules/template-no-outlet-outside-routes.md) | disallow {{outlet}} outside of route templates | | | | | [template-no-page-title-component](docs/rules/template-no-page-title-component.md) | disallow usage of ember-page-title component | | | | +| [template-simple-modifiers](docs/rules/template-simple-modifiers.md) | require simple modifier syntax | | | | | [template-simple-unless](docs/rules/template-simple-unless.md) | require simple conditions in unless blocks | | | | | [template-splat-attributes-only](docs/rules/template-splat-attributes-only.md) | disallow ...spread other than ...attributes | | | | | [template-style-concatenation](docs/rules/template-style-concatenation.md) | disallow string concatenation in inline styles | | | | diff --git a/docs/rules/template-simple-modifiers.md b/docs/rules/template-simple-modifiers.md new file mode 100644 index 0000000000..a24d73248e --- /dev/null +++ b/docs/rules/template-simple-modifiers.md @@ -0,0 +1,45 @@ +# ember/template-simple-modifiers + + + +Require simple modifier syntax. + +The `modifier` helper should have a simple string or path expression as its first argument (the modifier name). Complex expressions should not be used as the first argument. + +## Examples + +This rule **forbids** the following: + +```gjs + +``` + +```gjs + +``` + +This rule **allows** the following: + +```gjs + +``` + +```gjs + +``` + +## Why? + +Using complex expressions as the modifier name reduces readability and makes it harder to understand which modifier is being applied. + +## References + +- [Ember.js Guides - Modifiers](https://guides.emberjs.com/release/components/template-lifecycle-dom-and-modifiers/) diff --git a/lib/rules/template-simple-modifiers.js b/lib/rules/template-simple-modifiers.js new file mode 100644 index 0000000000..ccb2822103 --- /dev/null +++ b/lib/rules/template-simple-modifiers.js @@ -0,0 +1,57 @@ +/** @type {import('eslint').Rule.RuleModule} */ +module.exports = { + meta: { + type: 'suggestion', + docs: { + description: 'require simple modifier syntax', + category: 'Best Practices', + recommended: false, + url: 'https://github.com/ember-cli/eslint-plugin-ember/tree/master/docs/rules/template-simple-modifiers.md', + templateMode: 'both', + }, + fixable: null, + schema: [], + messages: {}, + originallyFrom: { + name: 'ember-template-lint', + rule: 'lib/rules/simple-modifiers.js', + docs: 'docs/rule/simple-modifiers.md', + tests: 'test/unit/rules/simple-modifiers-test.js', + }, + }, + + create(context) { + return { + // This catches {{(modifier ...)}} expressions + GlimmerSubExpression(node) { + // Check if this is a call to the 'modifier' helper + if (node.path && node.path.original === 'modifier') { + // The first param should be a string or a path expression + if (!node.params || node.params.length === 0) { + context.report({ + node, + message: + 'The modifier helper should have a string or a variable name containing the modifier name as a first argument.', + }); + return; + } + + const firstParam = node.params[0]; + + // Check if first param is a string literal or path expression + const isValidFirstParam = + firstParam.type === 'GlimmerStringLiteral' || + (firstParam.type === 'GlimmerPathExpression' && firstParam.original); + + if (!isValidFirstParam) { + context.report({ + node: firstParam, + message: + 'The modifier helper should have a string or a variable name containing the modifier name as a first argument.', + }); + } + } + }, + }; + }, +}; diff --git a/tests/lib/rules/template-simple-modifiers.js b/tests/lib/rules/template-simple-modifiers.js new file mode 100644 index 0000000000..8504ad92dd --- /dev/null +++ b/tests/lib/rules/template-simple-modifiers.js @@ -0,0 +1,87 @@ +const rule = require('../../../lib/rules/template-simple-modifiers'); +const RuleTester = require('eslint').RuleTester; + +const ruleTester = new RuleTester({ + parser: require.resolve('ember-eslint-parser'), + parserOptions: { ecmaVersion: 2022, sourceType: 'module' }, +}); + +ruleTester.run('template-simple-modifiers', rule, { + valid: [ + '', + '', + '', + + '', + '', + '', + '', + '', + ], + invalid: [ + { + code: '', + output: null, + errors: [ + { + message: + 'The modifier helper should have a string or a variable name containing the modifier name as a first argument.', + }, + ], + }, + + { + code: '', + output: null, + errors: [ + { + message: + 'The modifier helper should have a string or a variable name containing the modifier name as a first argument.', + }, + ], + }, + ], +}); + +const hbsRuleTester = new RuleTester({ + parser: require.resolve('ember-eslint-parser/hbs'), + parserOptions: { + ecmaVersion: 2022, + sourceType: 'module', + }, +}); + +hbsRuleTester.run('template-simple-modifiers', rule, { + valid: [ + '
', + '
', + '
', + '
', + '
', + '
', + '', + '
', + ], + invalid: [ + { + code: '
', + output: null, + errors: [ + { + message: + 'The modifier helper should have a string or a variable name containing the modifier name as a first argument.', + }, + ], + }, + { + code: '
', + output: null, + errors: [ + { + message: + 'The modifier helper should have a string or a variable name containing the modifier name as a first argument.', + }, + ], + }, + ], +}); From 7a32d6f9a6124a3cdf55cda881891c7f799f1d8f Mon Sep 17 00:00:00 2001 From: NullVoxPopuli <199018+NullVoxPopuli@users.noreply.github.com> Date: Thu, 19 Mar 2026 16:13:56 -0400 Subject: [PATCH 2/2] Sync with template-lint --- docs/rules/template-simple-modifiers.md | 37 +++++++++---- lib/rules/template-simple-modifiers.js | 55 +++++++++++--------- tests/lib/rules/template-simple-modifiers.js | 2 + 3 files changed, 58 insertions(+), 36 deletions(-) diff --git a/docs/rules/template-simple-modifiers.md b/docs/rules/template-simple-modifiers.md index a24d73248e..fdbd7d4c9c 100644 --- a/docs/rules/template-simple-modifiers.md +++ b/docs/rules/template-simple-modifiers.md @@ -4,7 +4,16 @@ Require simple modifier syntax. -The `modifier` helper should have a simple string or path expression as its first argument (the modifier name). Complex expressions should not be used as the first argument. +This rule strongly advises against passing complex statements or conditionals to the +first argument of the `{{modifier}}` helper. Instead, the first argument should be +either: + +- a string literal such as `{{(modifier "track-interaction")}}` +- a path expression such as `{{(modifier this.trackInteraction)}}` + +A common issue this rule catches is declaring the modifier name conditionally, which +works because `modifier` ignores `null` and `undefined`, but makes the intent much +harder to read. Prefer placing the conditional around the modifier invocation instead. ## Examples @@ -12,7 +21,13 @@ This rule **forbids** the following: ```gjs ``` @@ -26,20 +41,20 @@ This rule **allows** the following: ```gjs -``` - -```gjs - ``` ## Why? -Using complex expressions as the modifier name reduces readability and makes it harder to understand which modifier is being applied. +Using complex expressions as the modifier name reduces readability and makes it harder +to understand which modifier is being applied. ## References -- [Ember.js Guides - Modifiers](https://guides.emberjs.com/release/components/template-lifecycle-dom-and-modifiers/) +- [Ember.js Guides - Modifiers](https://guides.emberjs.com/release/components/template-lifecycle-dom-and-modifiers/#toc_event-handlers) diff --git a/lib/rules/template-simple-modifiers.js b/lib/rules/template-simple-modifiers.js index ccb2822103..4cd377bf08 100644 --- a/lib/rules/template-simple-modifiers.js +++ b/lib/rules/template-simple-modifiers.js @@ -1,3 +1,13 @@ +function isModifierHelper(node) { + return node.path?.original === 'modifier'; +} + +function isValidFirstParam(node) { + return ( + node.type === 'GlimmerStringLiteral' || (node.type === 'GlimmerPathExpression' && node.original) + ); +} + /** @type {import('eslint').Rule.RuleModule} */ module.exports = { meta: { @@ -5,13 +15,15 @@ module.exports = { docs: { description: 'require simple modifier syntax', category: 'Best Practices', - recommended: false, url: 'https://github.com/ember-cli/eslint-plugin-ember/tree/master/docs/rules/template-simple-modifiers.md', templateMode: 'both', }, fixable: null, schema: [], - messages: {}, + messages: { + invalidFirstArgument: + 'The modifier helper should have a string or a variable name containing the modifier name as a first argument.', + }, originallyFrom: { name: 'ember-template-lint', rule: 'lib/rules/simple-modifiers.js', @@ -24,32 +36,25 @@ module.exports = { return { // This catches {{(modifier ...)}} expressions GlimmerSubExpression(node) { - // Check if this is a call to the 'modifier' helper - if (node.path && node.path.original === 'modifier') { - // The first param should be a string or a path expression - if (!node.params || node.params.length === 0) { - context.report({ - node, - message: - 'The modifier helper should have a string or a variable name containing the modifier name as a first argument.', - }); - return; - } + if (!isModifierHelper(node)) { + return; + } - const firstParam = node.params[0]; + const firstParam = node.params?.[0]; - // Check if first param is a string literal or path expression - const isValidFirstParam = - firstParam.type === 'GlimmerStringLiteral' || - (firstParam.type === 'GlimmerPathExpression' && firstParam.original); + if (!firstParam) { + context.report({ + node, + messageId: 'invalidFirstArgument', + }); + return; + } - if (!isValidFirstParam) { - context.report({ - node: firstParam, - message: - 'The modifier helper should have a string or a variable name containing the modifier name as a first argument.', - }); - } + if (!isValidFirstParam(firstParam)) { + context.report({ + node: firstParam, + messageId: 'invalidFirstArgument', + }); } }, }; diff --git a/tests/lib/rules/template-simple-modifiers.js b/tests/lib/rules/template-simple-modifiers.js index 8504ad92dd..d17ef00cf7 100644 --- a/tests/lib/rules/template-simple-modifiers.js +++ b/tests/lib/rules/template-simple-modifiers.js @@ -9,6 +9,7 @@ const ruleTester = new RuleTester({ ruleTester.run('template-simple-modifiers', rule, { valid: [ '', + '', '', '', @@ -54,6 +55,7 @@ const hbsRuleTester = new RuleTester({ hbsRuleTester.run('template-simple-modifiers', rule, { valid: [ '
', + '
', '
', '
', '
',