Post-merge-review: Fix template-no-curly-component-invocation: preserve this./@/local names in suggestions, and skip JS scope bindings#2657
Open
johanrd wants to merge 1 commit intoember-cli:masterfrom
Conversation
…s, local names, and JS scope bindings
Three false positives in the port:
1. transformTagName PascalCased this./@/local names, producing wrong
suggestions (e.g. {{#this.foo-bar}} → <This.fooBar> instead of
<this.foo-bar>). Matches upstream transformTagName(name, isLocal).
2. In GJS/GTS, explicit JS scope bindings (imports, const, function
params) used as curly invocations were flagged as needing angle-
bracket conversion. The conversion would break: converting
{{fooBar}} to <FooBar> references an unbound name. Added
isJsScopeBinding() using sourceCode.getScope() (pattern borrowed
from template-no-restricted-invocations / template-no-redundant-fn).
Applies to both single-word and named-args mustache forms, and to
block invocations.
3. Block params already had transformTagName preservation wired up;
the fix extends that to JS scope bindings too.
Upstream uses this.isLocal(path) via its own scope tracker for the same
purpose; this implementation uses ESLint's sourceCode.getScope().
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
What's broken on
masterTwo related false positives:
Autofix PascalCases path prefixes.
transformTagNameunconditionally PascalCases the curly path when generating the suggested angle-bracket form, corrupting three categories of names:GJS/GTS JS scope bindings flagged. The port has no JS scope tracker, so valid imported/const identifiers used in curly form are flagged as needing angle-bracket conversion. The "fix" would break:
import fooBar from './foo-bar'; <template>{{fooBar}}</template>would be rewritten to<FooBar />which references an unbound name.Fix
isLocaltotransformTagNameand return the name verbatim when it starts with@, starts withthis., or is a local (block-param) binding. Matches upstream exactly (curly-component-invocation.jsL1–2).isJsScopeBinding(node)usingsourceCode.getScope(). When the mustache/block path head resolves to a JS binding, skip reporting entirely. Pattern already used bytemplate-no-redundant-fnandtemplate-no-restricted-invocations(isJsScopeVariable). Upstream ember-template-lint usesthis.isLocal(path)via its own scope tracker for the same purpose.Test plan
import fooBar from '…'; {{fooBar}},{{fooBar arg=1}},{{#fooBar}}…{{/fooBar}},const someHelper = …; {{someHelper}}) all fail on master.Co-written by Claude.