Post-merge-review: Fix template-no-link-to-tagname: only flag @tagName, not bare tagName, on angle-bracket <LinkTo>#2652
Conversation
…ing import tracking in strict mode What was broken on master: - The rule matched every `<LinkTo>` tag by bare name. In strict-mode templates (.gjs/.gts) a component named `LinkTo` is only Ember's router link if it is imported from `@ember/routing`. This caused two bugs: 1. False positive: a user-authored `<LinkTo @TagName="button">` component (no import) was flagged even though it has nothing to do with Ember's router LinkTo. 2. False negative: a renamed import such as `import { LinkTo as Link } from '@ember/routing'` used as `<Link @TagName="button">` was not detected because the tag name no longer matches the bare string `LinkTo`. - Additionally: on angle-bracket `<LinkTo>` the rule previously flagged the bare HTML-attribute `tagName` as well as the Ember argument `@tagName`. Bare `tagName` on angle-bracket invocation is just a passthrough HTML attribute and is not deprecated; only `@tagName` is. Fix: - Detect strict mode via `context.filename` (.gjs / .gts). - Add an `ImportDeclaration` visitor (strict mode only) that records every local alias imported as `LinkTo` from `@ember/routing` into `importedLinkComponents`. Mirrors the pattern used in `template-no-invalid-link-text`. - In strict mode the angle-bracket visitor only flags elements whose tag is in the tracked set. In classic HBS it keeps the previous behavior (`LinkTo` / `link-to`), since HBS has no imports. - The curly (`{{link-to}}` / `{{#link-to}}`) visitor is unchanged: there are no imports in HBS curly form. - Only flag `@tagName` (not bare `tagName`) on angle-bracket `<LinkTo>`. Test plan: - Valid: `<LinkTo @TagName="button">` in .gjs and .gts with no `@ember/routing` import (user-authored component must not be flagged). - Invalid: `import { LinkTo } from '@ember/routing';` + `<LinkTo @TagName=...>` in both .gjs and .gts. - Invalid: renamed import `import { LinkTo as Link } from '@ember/routing';` + `<Link @TagName="button">` flags `<Link>` in both .gjs and .gts. - Existing HBS cases still covered (including `<link-to @TagName>` via angle-bracket kebab form in classic templates).
47bb30f to
0338b6c
Compare
The angle-bracket visitor checks `node.tag === 'LinkTo'` by bare name, which causes two GJS/GTS bugs (the same class as PR ember-cli#2652 fixed for template-no-link-to-tagname): - A user-authored `<LinkTo>` from any non-`@ember/routing` source is falsely flagged when childless. - A renamed framework import (`import { LinkTo as Link } from '@ember/routing'`) used as `<Link />` is missed entirely. Adopt the same `@ember/routing` import-tracker pattern: in strict mode, only flag elements whose tag is in the tracked map (covers renamed imports). HBS path uses bare-name match (no imports possible). Curly `{{link-to ...}}` handler is gated to HBS only — `link-to` is not a valid JS identifier, so it cannot be a user binding in strict mode, and the strict-mode compiler would already reject the source. Tests: 21 (was 14) — adds 4 new valid GJS/GTS cases (no-import bare `<LinkTo>` in .gjs and .gts, with-import block-form `<LinkTo>...</LinkTo>`, with-renamed-import block-form `<Link>...</Link>`) and 3 new invalid cases (with-import childless `<LinkTo />` in .gjs and .gts, renamed childless `<Link />`). Docs updated to document the strict-mode behavior.
The angle-bracket visitor checks `node.tag === 'LinkTo'` by bare name, which causes two GJS/GTS bugs (the same class as PR ember-cli#2652 fixed for template-no-link-to-tagname): - A user-authored `<LinkTo>` from any non-`@ember/routing` source is falsely flagged when childless. - A renamed framework import (`import { LinkTo as Link } from '@ember/routing'`) used as `<Link />` is missed entirely. Adopt the same `@ember/routing` import-tracker pattern: in strict mode, only flag elements whose tag is in the tracked map (covers renamed imports). HBS path uses bare-name match (no imports possible). Curly `{{link-to ...}}` handler is gated to HBS only — `link-to` is not a valid JS identifier, so it cannot be a user binding in strict mode, and the strict-mode compiler would already reject the source. Tests: 21 (was 14) — adds 4 new valid GJS/GTS cases (no-import bare `<LinkTo>` in .gjs and .gts, with-import block-form `<LinkTo>...</LinkTo>`, with-renamed-import block-form `<Link>...</Link>`) and 3 new invalid cases (with-import childless `<LinkTo />` in .gjs and .gts, renamed childless `<Link />`). Docs updated to document the strict-mode behavior.
| const filename = context.filename; | ||
| const isStrictMode = filename.endsWith('.gjs') || filename.endsWith('.gts'); | ||
|
|
||
| // In HBS, LinkTo always refers to Ember's router link component. |
There was a problem hiding this comment.
I don't think this is not true -- if a user defines a LinkTo component in their app that will be resolved, i believe
There was a problem hiding this comment.
okay, thanks, yes should maybe soften the comment a bit to "almost always refers to Ember's router link component; a user-defined link-to could shadow it, but detecting that in classic HBS would require resolver-level info the rule doesn't have."?
| { | ||
| code: '<link-to @route="contact" @tagName="div">Contact</link-to>', | ||
| output: null, | ||
| errors: [{ message: 'tagName attribute on LinkTo is deprecated' }], |
There was a problem hiding this comment.
is it still deprecated? even now? did we ever remove it? what version of ember?
There was a problem hiding this comment.
good point. it was removed in Ember 4.0 (Dec 2021).
- Deprecation ID: ember.built-in-components.legacy-arguments
- Deprecated in: 3.27 (June 2021, RFC #671)
- Removed in: 4.0.0 — since 4.0, always renders an
<a>; @TagName is no longer supported at all, as I understand it.
So the error message 'tagName attribute on LinkTo is deprecated' is stale — for Ember 4+ it's not deprecated, it's gone. Options: rename the messageId/text to something like '@TagName on is not supported (removed in Ember 4.0)', and possibly reconsider the category.
There was a problem hiding this comment.
yae a new error message based on the ember-source version would be good
The comment "LinkTo always refers to..." overstated our certainty in classic HBS — a user-defined component could shadow it, though we can't detect that without resolver info. The message said "deprecated" but @TagName was fully removed in Ember 4.0 (deprecation ID: ember.built-in-components.legacy-arguments, deprecated in 3.27, removed in 4.0.0, Dec 2021).
What's broken on master
<LinkTo>tag by bare name. In strict-mode templates (.gjs/.gts) a component namedLinkTois only Ember's router link if it is imported from@ember/routing. This caused two bugs:<LinkTo @tagName="button">component (no import from@ember/routing) was flagged even though it has nothing to do with Ember's routerLinkTo.import { LinkTo as Link } from '@ember/routing'used as<Link @tagName="button">was not detected because the tag name no longer matches the bare stringLinkTo.<LinkTo>the rule previously flagged the bare HTML-attributetagNameas well as the Ember argument@tagName. BaretagNameon angle-bracket invocation is just a passthrough HTML attribute and is not deprecated; only@tagNameis.Fix
context.filename(.gjs/.gts).ImportDeclarationvisitor (strict mode only) that records every local alias imported asLinkTofrom@ember/routingintoimportedLinkComponents. Mirrors the pattern used intemplate-no-invalid-link-text.LinkTo/link-to), since HBS has no imports.{{link-to}}/{{#link-to}}) visitor is unchanged: there are no imports in HBS curly form.@tagName(not baretagName) on angle-bracket<LinkTo>.Test plan
<LinkTo @tagName="button">in.gjsand.gtswith no@ember/routingimport (user-authored component must not be flagged).import { LinkTo } from '@ember/routing';+<LinkTo @tagName=...>in both.gjsand.gts.import { LinkTo as Link } from '@ember/routing';+<Link @tagName="button">flags<Link>in both.gjsand.gts.<link-to @tagName>via angle-bracket kebab form in classic templates).pnpm vitest run tests/lib/rules/template-no-link-to-tagname.js).