-
-
Notifications
You must be signed in to change notification settings - Fork 210
Add rule: ember/template-no-deprecated #2448
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 1 commit
a80c119
726ec0b
a6cd56a
a810190
4fbd7e5
ce31d4e
79e1127
d64c881
111f9e4
5f4dd35
40d3c76
e76e5f9
c410eb6
2df429d
58d3dfd
9c73644
7d0e5f0
4ee1869
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,79 @@ | ||
| # Future Work: `@arg` Deprecation in Templates | ||
|
|
||
| This document tracks the unimplemented case for `ember/template-no-deprecated`: | ||
| detecting `@deprecated` on component arguments passed in template syntax. | ||
|
|
||
| ## The Problem | ||
|
|
||
| `<MyComponent @deprecatedArg={{x}}>` — `@deprecatedArg` appears as a `GlimmerAttrNode`. | ||
| It is never scope-registered (the parser skips args, see `ember-eslint-parser/src/parser/transforms.js` | ||
| around line 504). The scope-based approach used in `template-no-deprecated` does not apply here. | ||
|
|
||
| ## The Analogous Solved Problem | ||
|
|
||
| `@typescript-eslint/no-deprecated` handles JSX attributes via `getJSXAttributeDeprecation` | ||
| (`no-deprecated.js:266–271`): | ||
|
|
||
| ```js | ||
| function getJSXAttributeDeprecation(openingElement, propertyName) { | ||
| const tsNode = services.esTreeNodeToTSNodeMap.get(openingElement.name); | ||
| const contextualType = checker.getContextualType(tsNode); | ||
| const symbol = contextualType.getProperty(propertyName); | ||
| return getJsDocDeprecation(symbol); | ||
| } | ||
| ``` | ||
|
|
||
| This works because `JSXOpeningElement.name` is a standard TS AST node, so | ||
| `esTreeNodeToTSNodeMap` has it, and TypeScript's JSX support provides `getContextualType` | ||
| for JSX element names (which gives the props type directly). | ||
|
|
||
| ## Why This Doesn't Directly Apply to Glimmer | ||
|
|
||
| `GlimmerElementNode.parts[0]` (`GlimmerElementNodePart`) is a **synthetic node** invented by | ||
| `ember-eslint-parser`. It is not in `esTreeNodeToTSNodeMap`. Therefore: | ||
|
|
||
| - `services.esTreeNodeToTSNodeMap.get(parts[0])` → `undefined` | ||
| - `checker.getContextualType(undefined)` → crash | ||
|
|
||
| ## Options | ||
|
|
||
| ### Option A: Navigate the Type Manually (no Glint required) | ||
|
|
||
| 1. Visit `GlimmerAttrNode` where `node.name.startsWith('@')` | ||
| 2. Get parent `GlimmerElementNode`, resolve its import binding (same as `template-no-deprecated`) | ||
| 3. `services.esTreeNodeToTSNodeMap.get(importDefNode)` → TS import node | ||
| 4. `checker.getTypeAtLocation(tsImportNode)` → component's constructor type | ||
| 5. Navigate type parameters to extract the `Args` type: | ||
| - Get base type `Component<{Args: A}>` → extract first type argument → get `A` | ||
| - Requires walking `type.typeArguments` and the `Args` property | ||
| 6. `argsType.getProperty(argName.slice(1))` → property symbol | ||
| 7. `getJsDocDeprecation(symbol)` → check for `@deprecated` | ||
|
|
||
| Self-contained (no Glint needed) but TypeScript type navigation is brittle — | ||
| it depends on the specific generic signature of `@glimmer/component`. | ||
|
|
||
| ### Option B: Register `@arg` Nodes in Scope (requires ember-eslint-parser change) | ||
|
|
||
| Modify `ember-eslint-parser/src/parser/transforms.js` to register `GlimmerAttrNode` names | ||
| starting with `@` as references, using a virtual identifier that IS in `esTreeNodeToTSNodeMap`. | ||
| Then the JSX-attribute pattern would work directly. | ||
|
|
||
| This is an upstream change to `ember-eslint-parser`, not this plugin. | ||
|
|
||
| ### Option C: Use Glint v2 Virtual TypeScript Files (future) | ||
|
|
||
| Glint v2 (Volar) operates as a tsserver TS plugin. ESLint's `@typescript-eslint/parser` | ||
| creates a separate TypeScript program that does **not** load tsserver plugins by default, | ||
| so Glint's template type information is not available in `parserServices.program`. | ||
|
|
||
| To bridge this: either (1) `ember-eslint-parser` would need to integrate with Glint's | ||
| virtual file infrastructure and re-expose the resulting program, or (2) Glint v2 would need | ||
| to provide a TypeScript compiler transform (not just a language server plugin) that | ||
| `@typescript-eslint/parser` picks up automatically. Neither exists today. File this as a | ||
| Glint project concern. | ||
|
|
||
| ## Recommended Path | ||
|
|
||
| Option A is the most tractable near-term approach — no upstream changes required. | ||
| The main risk is brittleness against non-standard component base classes. | ||
| Option B is architecturally cleaner but requires coordination with `ember-eslint-parser` maintainers. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,62 @@ | ||
| # ember/template-no-deprecated | ||
|
|
||
| 💼 This rule is enabled in the  `recommended-gts` [config](https://github.com/ember-cli/eslint-plugin-ember#-configurations). | ||
|
|
||
| <!-- end auto-generated rule header --> | ||
|
|
||
| Disallows using Glimmer components, helpers, or modifiers that are marked `@deprecated` in their JSDoc. | ||
NullVoxPopuli marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
| This rule requires TypeScript (`parserServices.program` must be present). It is a no-op in plain `.gjs` files because cross-file import deprecations require type information. | ||
|
|
||
| ## Rule Details | ||
|
|
||
| The rule resolves template references through ESLint's scope analysis: a `<Component>` or `{{helper}}` reference is traced back to its import declaration, then the TypeScript type checker inspects the exported symbol's JSDoc tags for `@deprecated`. | ||
|
||
|
|
||
| **Covered syntax:** | ||
|
|
||
| | Template syntax | Example | | ||
| |---|---| | ||
| | Component element | `<DeprecatedComponent />` | | ||
| | Helper / value mustache | `{{deprecatedHelper}}` | | ||
| | Block component | `{{#DeprecatedBlock}}…{{/DeprecatedBlock}}` | | ||
| | Modifier | `<div {{deprecatedModifier}}>` | | ||
|
|
||
| **Not covered (see future work):** `<MyComp @deprecatedArg={{x}}>` — argument names are not scope-registered by the parser. | ||
wagenet marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
| ## Examples | ||
|
|
||
| Given a module: | ||
|
|
||
| ```ts | ||
| // deprecated-component.ts | ||
| /** @deprecated use NewComponent instead */ | ||
| export default class DeprecatedComponent {} | ||
| ``` | ||
|
|
||
| Examples of **incorrect** code for this rule: | ||
|
|
||
| ```gts | ||
| import DeprecatedComponent from './deprecated-component'; | ||
|
|
||
| <template> | ||
| <DeprecatedComponent /> | ||
| </template> | ||
| ``` | ||
|
|
||
| ```gts | ||
| import { deprecatedHelper } from './deprecated-helper'; | ||
|
|
||
| <template> | ||
| {{deprecatedHelper}} | ||
| </template> | ||
| ``` | ||
|
|
||
| Examples of **correct** code for this rule: | ||
|
|
||
| ```gts | ||
| import NewComponent from './new-component'; | ||
|
|
||
| <template> | ||
| <NewComponent /> | ||
| </template> | ||
| ``` | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. PR needs cleanup |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,126 @@ | ||
| 'use strict'; | ||
|
|
||
| const tsutils = require('ts-api-utils'); | ||
| const ts = require('typescript'); | ||
|
|
||
| /** @type {import('eslint').Rule.RuleModule} */ | ||
| module.exports = { | ||
| meta: { | ||
| type: 'problem', | ||
| docs: { | ||
| description: | ||
| 'disallow using deprecated Glimmer components, helpers, and modifiers in templates', | ||
| category: 'Ember Octane', | ||
| recommendedGts: true, | ||
| url: 'https://github.com/ember-cli/eslint-plugin-ember/tree/master/docs/rules/template-no-deprecated.md', | ||
| }, | ||
| requiresTypeChecking: true, | ||
| schema: [], | ||
| messages: { | ||
| deprecated: '`{{name}}` is deprecated.', | ||
| deprecatedWithReason: '`{{name}}` is deprecated. {{reason}}', | ||
| }, | ||
| }, | ||
|
|
||
| create(context) { | ||
| const services = context.sourceCode.parserServices ?? context.parserServices; | ||
| if (!services?.program) { | ||
| return {}; | ||
| } | ||
|
|
||
| const checker = services.program.getTypeChecker(); | ||
| const sourceCode = context.sourceCode; | ||
|
|
||
| function getJsDocDeprecation(symbol) { | ||
| let jsDocTags; | ||
| try { | ||
| jsDocTags = symbol?.getJsDocTags(checker); | ||
| } catch { | ||
| // workaround for https://github.com/microsoft/TypeScript/issues/60024 | ||
| return undefined; | ||
| } | ||
| const tag = jsDocTags?.find((t) => t.name === 'deprecated'); | ||
| if (!tag) { | ||
| return undefined; | ||
| } | ||
| const displayParts = tag.text; | ||
| return displayParts ? ts.displayPartsToString(displayParts) : ''; | ||
| } | ||
|
|
||
| function searchForDeprecationInAliasesChain(symbol, checkAliasedSymbol) { | ||
| if (!symbol || !tsutils.isSymbolFlagSet(symbol, ts.SymbolFlags.Alias)) { | ||
| return checkAliasedSymbol ? getJsDocDeprecation(symbol) : undefined; | ||
| } | ||
| const targetSymbol = checker.getAliasedSymbol(symbol); | ||
| while (tsutils.isSymbolFlagSet(symbol, ts.SymbolFlags.Alias)) { | ||
| const reason = getJsDocDeprecation(symbol); | ||
| if (reason != null) { | ||
| return reason; | ||
| } | ||
| const immediateAliasedSymbol = | ||
| symbol.getDeclarations() && checker.getImmediateAliasedSymbol(symbol); | ||
| if (!immediateAliasedSymbol) { | ||
| break; | ||
| } | ||
| symbol = immediateAliasedSymbol; | ||
| if (checkAliasedSymbol && symbol === targetSymbol) { | ||
| return getJsDocDeprecation(symbol); | ||
| } | ||
| } | ||
| return undefined; | ||
| } | ||
|
|
||
| function checkDeprecatedIdentifier(identifierNode, scope) { | ||
| const ref = scope.references.find((v) => v.identifier === identifierNode); | ||
| const variable = ref?.resolved; | ||
| const def = variable?.defs[0]; | ||
|
|
||
| if (!def || def.type !== 'ImportBinding') { | ||
| return; | ||
| } | ||
|
|
||
| const tsNode = services.esTreeNodeToTSNodeMap.get(def.node); | ||
| if (!tsNode) { | ||
| return; | ||
| } | ||
|
|
||
| // ImportClause and ImportSpecifier require .name for getSymbolAtLocation | ||
| const tsIdentifier = tsNode.name ?? tsNode; | ||
| const symbol = checker.getSymbolAtLocation(tsIdentifier); | ||
| if (!symbol) { | ||
| return; | ||
| } | ||
|
|
||
| const reason = searchForDeprecationInAliasesChain(symbol, true); | ||
| if (reason == null) { | ||
| return; | ||
| } | ||
|
|
||
| if (reason === '') { | ||
| context.report({ | ||
| node: identifierNode, | ||
| messageId: 'deprecated', | ||
| data: { name: identifierNode.name }, | ||
| }); | ||
| } else { | ||
| context.report({ | ||
| node: identifierNode, | ||
| messageId: 'deprecatedWithReason', | ||
| data: { name: identifierNode.name, reason }, | ||
| }); | ||
| } | ||
| } | ||
|
|
||
| return { | ||
| GlimmerPathExpression(node) { | ||
| checkDeprecatedIdentifier(node.head, sourceCode.getScope(node)); | ||
| }, | ||
|
|
||
| GlimmerElementNode(node) { | ||
| // GlimmerElementNode is in its own scope; get the outer scope | ||
| const scope = sourceCode.getScope(node.parent); | ||
| checkDeprecatedIdentifier(node.parts[0], scope); | ||
| }, | ||
| }; | ||
| }, | ||
| }; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why's this here? |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| export default class CurrentComponent {} |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| /** @deprecated use NewComponent instead */ | ||
| export default class DeprecatedComponent {} |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| /** @deprecated */ | ||
| export function deprecatedHelper(): string { | ||
| return 'deprecated'; | ||
| } |
NullVoxPopuli marked this conversation as resolved.
Show resolved
Hide resolved
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| // Placeholder file — actual code is provided inline by tests. | ||
| // Its presence lets TypeScript include this path in the program. |
Uh oh!
There was an error while loading. Please reload this page.