-
Notifications
You must be signed in to change notification settings - Fork 27k
feat(language-service): support to report the deprecated API in the t… #62054
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
Conversation
packages/compiler-cli/src/ngtsc/typecheck/diagnostics/src/diagnostic.ts
Outdated
Show resolved
Hide resolved
| /** | ||
| * Connect the TCB node to the template node and generate the template diagnostics. | ||
| * | ||
| * How to generate the template diagnostics: | ||
| * | ||
| * 1. For each diagnostic, find the TCB node that is reported. | ||
| * 2. Build a map called `nodeToDiag` that the key is the type node and value is the diagnostic. | ||
| * For example: | ||
| * ``` | ||
| * var _t1 = null! as TestDir; | ||
| * ^^^^^^^------ This is diagnostic node that is reported by the ts. | ||
| * ``` | ||
| * The key is the class component of TestDir. | ||
| * 3. Find the all directive nodes in the TCB. | ||
| * For example: | ||
| * In the above example, the directive node is `_t1`, get the type of `_t1` which is the | ||
| * class component of `TestDir`. Check if there is a diagnostic in the `nodeToDiag` map | ||
| * that matches the class component of `TestDir`. | ||
| * If there is a match, it means that the diagnostic is reported for the directive node | ||
| * 4. Generate the template diagnostic and return the template diagnostics. | ||
| */ | ||
| function getTheElementTagDeprecatedSuggestionDiagnostics( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So we ran into this issue internally as well for the indexing that powers code search: A directive's span applies to the whole element that it is active for (correctly). However, this makes it appear like the element is deprecated when in reality it's just [directive] here. This is pretty problematic. As an example NgFor and NgIf have this problem and when used, make it so the element appears deprecated. Components really have this problem too but it's generally not something that comes up because their selectors are element tag selectors 99% of the time.
I don't think we have any information about the span for the selectors of directives/components. For quick info, we did something like this:
| export function getDirectiveMatchesForElementTag<T extends {selector: string | null}>( |
| export function getDirectiveMatchesForAttribute( |
We remove an attribute and see which directives no longer apply and from that information can determine that the selector is necessary for the directive matching. Maybe you can do something similar here? Otherwise I might recommend omitting the directives from this feature and rely on the inputs hopefully being deprecated as well. We did that for the control flow inputs as a workaround for this issue (8885811)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
Now, the getDirectiveMatchesForElementTag and visitor are located in the language service, so I pass a class to the type checker.
c89aa6d to
cb893ad
Compare
| /** | ||
| * Connect the TCB node to the template node and generate the template diagnostics. | ||
| * | ||
| * How to generate the template diagnostics: | ||
| * | ||
| * 1. For each diagnostic, find the TCB node that is reported. | ||
| * 2. Build a map called `nodeToDiag` that the key is the type node and value is the diagnostic. | ||
| * For example: | ||
| * ``` | ||
| * var _t1 = null! as TestDir; | ||
| * ^^^^^^^------ This is diagnostic node that is reported by the ts. | ||
| * ``` | ||
| * The key is the class component of TestDir. | ||
| * 3. Find the all directive nodes in the TCB. | ||
| * For example: | ||
| * In the above example, the directive node is `_t1`, get the type of `_t1` which is the | ||
| * class component of `TestDir`. Check if there is a diagnostic in the `nodeToDiag` map | ||
| * that matches the class component of `TestDir`. | ||
| * If there is a match, it means that the diagnostic is reported for the directive node | ||
| * 4. Generate the template diagnostic and return the template diagnostics. | ||
| */ | ||
| function getTheElementTagDeprecatedSuggestionDiagnostics( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So we ran into this issue internally as well for the indexing that powers code search: A directive's span applies to the whole element that it is active for (correctly). However, this makes it appear like the element is deprecated when in reality it's just [directive] here. This is pretty problematic. As an example NgFor and NgIf have this problem and when used, make it so the element appears deprecated. Components really have this problem too but it's generally not something that comes up because their selectors are element tag selectors 99% of the time.
I don't think we have any information about the span for the selectors of directives/components. For quick info, we did something like this:
| export function getDirectiveMatchesForElementTag<T extends {selector: string | null}>( |
| export function getDirectiveMatchesForAttribute( |
We remove an attribute and see which directives no longer apply and from that information can determine that the selector is necessary for the directive matching. Maybe you can do something similar here? Otherwise I might recommend omitting the directives from this feature and rely on the inputs hopefully being deprecated as well. We did that for the control flow inputs as a workaround for this issue (8885811)
| } | ||
|
|
||
| const directiveForDiagnostic = templateTypeChecker.getDirectiveMetadata(decl); | ||
| if (directiveForDiagnostic === null || !directiveForDiagnostic.isComponent) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only the component will report the deprecated diagnostic here.
1056e1f to
fcc3f85
Compare
fcc3f85 to
fbb1f30
Compare
fbb1f30 to
491d0d7
Compare
…emplate In the Typescript Language Service, these diagnostics are reported as suggestion diagnostics. This will report the deprecated `Component`, `Directive`, etc. Fixes angular#59343
491d0d7 to
9a29a7c
Compare
|
This PR was merged into the repository by commit d64dd27. The changes were merged into the following branches: main |
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |

…emplate
In the Typescript Language Service, these diagnostics are reported as suggestion diagnostics. This will report the deprecated
Component,Directive, etc.Fixes #59343
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information