Skip to content

Commit 03d1d00

Browse files
JeanMecheatscott
JeanMeche
authored and
atscott
committed
feat(compiler-cli): Add an extended diagnostic for nSkipHydration (#49512)
This diagnostic ensures that the special attribute `ngSkipHydration` is not a binding and has no other value than `"true"` or an empty value. Fixes #49501 PR Close #49512
1 parent d43367b commit 03d1d00

File tree

10 files changed

+287
-0
lines changed

10 files changed

+287
-0
lines changed

‎aio/content/extended-diagnostics/NG8108.md‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
@name ngSkipHydration should be a static attribute
2+
3+
@description
4+
5+
The `ngSkipHydration` is a special attribute that indicates to Angular that a particular component should be
6+
opted-out of hydration. This diagnostic ensures that this attribute `ngSkipHydration` is set statically and the
7+
value is either set to `"true"` or an empty value.
8+
9+
10+
## What should I do instead?
11+
12+
When using the `ngSkipHydration`, ensure that it's set as a static attribute (i.e. you do not use the Angular template binding syntax).
13+
14+
<code-example format="html" language="html">
15+
&lt;my-cmp ngSkipHydration /&gt;
16+
&lt;!-- or --&gt;
17+
&lt;my-cmp ngSkipHydration="true" /&gt;
18+
</code-example>
19+
20+
See [extended diagnostic configuration](extended-diagnostics#configuration) for more info.
21+
22+
<!-- links -->
23+
24+
<!-- external links -->
25+
26+
<!-- end links -->
27+
28+
@reviewed 2023-03-21

‎goldens/public-api/compiler-cli/error_code.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,7 @@ export enum ErrorCode {
7373
PIPE_MISSING_NAME = 2002,
7474
SCHEMA_INVALID_ATTRIBUTE = 8002,
7575
SCHEMA_INVALID_ELEMENT = 8001,
76+
SKIP_HYDRATION_NOT_STATIC = 8108,
7677
SPLIT_TWO_WAY_BINDING = 8007,
7778
SUFFIX_NOT_SUPPORTED = 8106,
7879
SUGGEST_STRICT_TEMPLATES = 10001,

‎goldens/public-api/compiler-cli/extended_template_diagnostic_name.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@ export enum ExtendedTemplateDiagnosticName {
1717
// (undocumented)
1818
OPTIONAL_CHAIN_NOT_NULLABLE = "optionalChainNotNullable",
1919
// (undocumented)
20+
SKIP_HYDRATION_NOT_STATIC = "skipHydrationNotStatic",
21+
// (undocumented)
2022
SUFFIX_NOT_SUPPORTED = "suffixNotSupported",
2123
// (undocumented)
2224
TEXT_ATTRIBUTE_NOT_BINDING = "textAttributeNotBinding"

‎packages/compiler-cli/src/ngtsc/diagnostics/src/error_code.ts‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -329,6 +329,20 @@ export enum ErrorCode {
329329
*/
330330
OPTIONAL_CHAIN_NOT_NULLABLE = 8107,
331331

332+
333+
/**
334+
* `ngSkipHydration` should not be a binding (it should be a static attribute).
335+
*
336+
* For example:
337+
* ```
338+
* <my-cmp [ngSkipHydration]="someTruthyVar" />
339+
* ```
340+
*
341+
* `ngSkipHydration` cannot be a binding and can not have values other than "true" or an empty
342+
* value
343+
*/
344+
SKIP_HYDRATION_NOT_STATIC = 8108,
345+
332346
/**
333347
* The template type-checking engine would need to generate an inline type check block for a
334348
* component, but the current type-checking environment doesn't support it.

‎packages/compiler-cli/src/ngtsc/diagnostics/src/extended_template_diagnostic_name.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,4 +23,5 @@ export enum ExtendedTemplateDiagnosticName {
2323
TEXT_ATTRIBUTE_NOT_BINDING = 'textAttributeNotBinding',
2424
MISSING_NGFOROF_LET = 'missingNgForOfLet',
2525
SUFFIX_NOT_SUPPORTED = 'suffixNotSupported',
26+
SKIP_HYDRATION_NOT_STATIC = 'skipHydrationNotStatic',
2627
}

‎packages/compiler-cli/src/ngtsc/typecheck/extended/BUILD.bazel‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ ts_library(
1717
"//packages/compiler-cli/src/ngtsc/typecheck/extended/checks/missing_ngforof_let",
1818
"//packages/compiler-cli/src/ngtsc/typecheck/extended/checks/nullish_coalescing_not_nullable",
1919
"//packages/compiler-cli/src/ngtsc/typecheck/extended/checks/optional_chain_not_nullable",
20+
"//packages/compiler-cli/src/ngtsc/typecheck/extended/checks/skip_hydration_not_static",
2021
"//packages/compiler-cli/src/ngtsc/typecheck/extended/checks/suffix_not_supported",
2122
"//packages/compiler-cli/src/ngtsc/typecheck/extended/checks/text_attribute_not_binding",
2223
"@npm//typescript",

‎packages/compiler-cli/src/ngtsc/typecheck/extended/checks/skip_hydration_not_static/BUILD.bazel‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
load("//tools:defaults.bzl", "ts_library")
2+
3+
ts_library(
4+
name = "skip_hydration_not_static",
5+
srcs = ["index.ts"],
6+
visibility = [
7+
"//packages/compiler-cli/src/ngtsc:__subpackages__",
8+
"//packages/compiler-cli/test/ngtsc:__pkg__",
9+
],
10+
deps = [
11+
"//packages/compiler",
12+
"//packages/compiler-cli/src/ngtsc/diagnostics",
13+
"//packages/compiler-cli/src/ngtsc/typecheck/api",
14+
"//packages/compiler-cli/src/ngtsc/typecheck/extended/api",
15+
"@npm//typescript",
16+
],
17+
)

‎packages/compiler-cli/src/ngtsc/typecheck/extended/checks/skip_hydration_not_static/index.ts‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
/**
2+
* @license
3+
* Copyright Google LLC All Rights Reserved.
4+
*
5+
* Use of this source code is governed by an MIT-style license that can be
6+
* found in the LICENSE file at https://angular.io/license
7+
*/
8+
9+
import {AST, TmplAstBoundAttribute, TmplAstNode, TmplAstTextAttribute} from '@angular/compiler';
10+
import ts from 'typescript';
11+
12+
import {ErrorCode, ExtendedTemplateDiagnosticName} from '../../../../diagnostics';
13+
import {NgTemplateDiagnostic} from '../../../api';
14+
import {TemplateCheckFactory, TemplateCheckWithVisitor, TemplateContext} from '../../api';
15+
16+
const NG_SKIP_HYDRATION_ATTR_NAME = 'ngSkipHydration';
17+
18+
/**
19+
* Ensures that the special attribute `ngSkipHydration` is not a binding and has no other
20+
* value than `"true"` or an empty value.
21+
*/
22+
class NgSkipHydrationSpec extends TemplateCheckWithVisitor<ErrorCode.SKIP_HYDRATION_NOT_STATIC> {
23+
override code = ErrorCode.SKIP_HYDRATION_NOT_STATIC as const;
24+
25+
override visitNode(
26+
ctx: TemplateContext<ErrorCode.SKIP_HYDRATION_NOT_STATIC>, component: ts.ClassDeclaration,
27+
node: TmplAstNode|AST): NgTemplateDiagnostic<ErrorCode.SKIP_HYDRATION_NOT_STATIC>[] {
28+
/** Binding should always error */
29+
if (node instanceof TmplAstBoundAttribute && node.name === NG_SKIP_HYDRATION_ATTR_NAME) {
30+
const errorString = `ngSkipHydration should not be used as a binding.`;
31+
const diagnostic = ctx.makeTemplateDiagnostic(node.sourceSpan, errorString);
32+
return [diagnostic];
33+
}
34+
35+
/** No value, empty string or `"true"` are the only valid values */
36+
const acceptedValues = ['true', '' /* empty string */];
37+
if (node instanceof TmplAstTextAttribute && node.name === NG_SKIP_HYDRATION_ATTR_NAME &&
38+
!acceptedValues.includes(node.value) && node.value !== undefined) {
39+
const errorString =
40+
`ngSkipHydration only accepts "true" or "" as value or no value at all. For example 'ngSkipHydration="true"' or 'ngSkipHydration'`;
41+
const diagnostic = ctx.makeTemplateDiagnostic(node.sourceSpan, errorString);
42+
return [diagnostic];
43+
}
44+
45+
return [];
46+
}
47+
}
48+
49+
export const factory: TemplateCheckFactory<
50+
ErrorCode.SKIP_HYDRATION_NOT_STATIC, ExtendedTemplateDiagnosticName.SKIP_HYDRATION_NOT_STATIC> =
51+
{
52+
code: ErrorCode.SKIP_HYDRATION_NOT_STATIC,
53+
name: ExtendedTemplateDiagnosticName.SKIP_HYDRATION_NOT_STATIC,
54+
create: () => new NgSkipHydrationSpec(),
55+
};

‎packages/compiler-cli/src/ngtsc/typecheck/extended/test/checks/skip_hydration_not_static/BUILD.bazel‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
load("//tools:defaults.bzl", "jasmine_node_test", "ts_library")
2+
3+
ts_library(
4+
name = "test_lib",
5+
testonly = True,
6+
srcs = ["skip_hydration_not_static_spec.ts"],
7+
deps = [
8+
"//packages/compiler",
9+
"//packages/compiler-cli/src/ngtsc/core:api",
10+
"//packages/compiler-cli/src/ngtsc/diagnostics",
11+
"//packages/compiler-cli/src/ngtsc/file_system",
12+
"//packages/compiler-cli/src/ngtsc/file_system/testing",
13+
"//packages/compiler-cli/src/ngtsc/testing",
14+
"//packages/compiler-cli/src/ngtsc/typecheck/extended",
15+
"//packages/compiler-cli/src/ngtsc/typecheck/extended/checks/skip_hydration_not_static",
16+
"//packages/compiler-cli/src/ngtsc/typecheck/testing",
17+
"@npm//typescript",
18+
],
19+
)
20+
21+
jasmine_node_test(
22+
name = "test",
23+
bootstrap = ["//tools/testing:node_no_angular"],
24+
deps = [
25+
":test_lib",
26+
],
27+
)

‎packages/compiler-cli/src/ngtsc/typecheck/extended/test/checks/skip_hydration_not_static/skip_hydration_not_static_spec.ts‎

Lines changed: 141 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,141 @@
1+
/**
2+
* @license
3+
* Copyright Google LLC All Rights Reserved.
4+
*
5+
* Use of this source code is governed by an MIT-style license that can be
6+
* found in the LICENSE file at https://angular.io/license
7+
*/
8+
9+
import ts from 'typescript';
10+
11+
import {ErrorCode, ExtendedTemplateDiagnosticName, ngErrorCode} from '../../../../../diagnostics';
12+
import {absoluteFrom, getSourceFileOrError} from '../../../../../file_system';
13+
import {runInEachFileSystem} from '../../../../../file_system/testing';
14+
import {getSourceCodeForDiagnostic} from '../../../../../testing';
15+
import {getClass, setup} from '../../../../testing';
16+
import {factory as skipHydrationNotStaticFactory} from '../../../checks/skip_hydration_not_static';
17+
import {ExtendedTemplateCheckerImpl} from '../../../src/extended_template_checker';
18+
19+
runInEachFileSystem(() => {
20+
describe('SkipHydrationNotStatic', () => {
21+
it('binds the error code to its extended template diagnostic name', () => {
22+
expect(skipHydrationNotStaticFactory.code).toBe(ErrorCode.SKIP_HYDRATION_NOT_STATIC);
23+
expect(skipHydrationNotStaticFactory.name)
24+
.toBe(ExtendedTemplateDiagnosticName.SKIP_HYDRATION_NOT_STATIC);
25+
});
26+
27+
it('should produce class binding warning', () => {
28+
const fileName = absoluteFrom('/main.ts');
29+
const {program, templateTypeChecker} = setup([
30+
{
31+
fileName,
32+
templates: {
33+
'TestCmp': `<div [ngSkipHydration]="true"></div>`,
34+
},
35+
source: 'export class TestCmp { }',
36+
},
37+
]);
38+
const sf = getSourceFileOrError(program, fileName);
39+
const component = getClass(sf, 'TestCmp');
40+
const extendedTemplateChecker = new ExtendedTemplateCheckerImpl(
41+
templateTypeChecker, program.getTypeChecker(), [skipHydrationNotStaticFactory], {}
42+
/* options */
43+
);
44+
const diags = extendedTemplateChecker.getDiagnosticsForComponent(component);
45+
expect(diags.length).toBe(1);
46+
expect(diags[0].category).toBe(ts.DiagnosticCategory.Warning);
47+
expect(diags[0].code).toBe(ngErrorCode(ErrorCode.SKIP_HYDRATION_NOT_STATIC));
48+
expect(getSourceCodeForDiagnostic(diags[0])).toBe(`[ngSkipHydration]="true"`);
49+
});
50+
51+
it('should produce an attribute binding warning', () => {
52+
const fileName = absoluteFrom('/main.ts');
53+
const {program, templateTypeChecker} = setup([
54+
{
55+
fileName,
56+
templates: {
57+
'TestCmp': `<div [ngSkipHydration]="''"></div>`,
58+
},
59+
source: 'export class TestCmp { }',
60+
},
61+
]);
62+
const sf = getSourceFileOrError(program, fileName);
63+
const component = getClass(sf, 'TestCmp');
64+
const extendedTemplateChecker = new ExtendedTemplateCheckerImpl(
65+
templateTypeChecker, program.getTypeChecker(), [skipHydrationNotStaticFactory], {}
66+
/* options */
67+
);
68+
const diags = extendedTemplateChecker.getDiagnosticsForComponent(component);
69+
expect(diags.length).toBe(1);
70+
expect(diags[0].category).toBe(ts.DiagnosticCategory.Warning);
71+
expect(diags[0].code).toBe(ngErrorCode(ErrorCode.SKIP_HYDRATION_NOT_STATIC));
72+
expect(getSourceCodeForDiagnostic(diags[0])).toBe(`[ngSkipHydration]="''"`);
73+
});
74+
75+
it('should produce a wrong value warning', () => {
76+
const fileName = absoluteFrom('/main.ts');
77+
const {program, templateTypeChecker} = setup([
78+
{
79+
fileName,
80+
templates: {
81+
'TestCmp': `<div ngSkipHydration="XXX"></div>`,
82+
},
83+
source: 'export class TestCmp { }',
84+
},
85+
]);
86+
const sf = getSourceFileOrError(program, fileName);
87+
const component = getClass(sf, 'TestCmp');
88+
const extendedTemplateChecker = new ExtendedTemplateCheckerImpl(
89+
templateTypeChecker, program.getTypeChecker(), [skipHydrationNotStaticFactory], {}
90+
/* options */
91+
);
92+
const diags = extendedTemplateChecker.getDiagnosticsForComponent(component);
93+
expect(diags.length).toBe(1);
94+
expect(diags[0].category).toBe(ts.DiagnosticCategory.Warning);
95+
expect(diags[0].code).toBe(ngErrorCode(ErrorCode.SKIP_HYDRATION_NOT_STATIC));
96+
expect(getSourceCodeForDiagnostic(diags[0])).toBe(`ngSkipHydration="XXX"`);
97+
});
98+
99+
it('should not produce a warning when there is no value', () => {
100+
const fileName = absoluteFrom('/main.ts');
101+
const {program, templateTypeChecker} = setup([
102+
{
103+
fileName,
104+
templates: {
105+
'TestCmp': `<div ngSkipHydration></div>`,
106+
},
107+
source: 'export class TestCmp { }',
108+
},
109+
]);
110+
const sf = getSourceFileOrError(program, fileName);
111+
const component = getClass(sf, 'TestCmp');
112+
const extendedTemplateChecker = new ExtendedTemplateCheckerImpl(
113+
templateTypeChecker, program.getTypeChecker(), [skipHydrationNotStaticFactory], {}
114+
/* options */
115+
);
116+
const diags = extendedTemplateChecker.getDiagnosticsForComponent(component);
117+
expect(diags.length).toBe(0);
118+
});
119+
120+
it('should not produce a warning with a correct value ', () => {
121+
const fileName = absoluteFrom('/main.ts');
122+
const {program, templateTypeChecker} = setup([
123+
{
124+
fileName,
125+
templates: {
126+
'TestCmp': `<div ngSkipHydration="true"></div>`,
127+
},
128+
source: 'export class TestCmp { }',
129+
},
130+
]);
131+
const sf = getSourceFileOrError(program, fileName);
132+
const component = getClass(sf, 'TestCmp');
133+
const extendedTemplateChecker = new ExtendedTemplateCheckerImpl(
134+
templateTypeChecker, program.getTypeChecker(), [skipHydrationNotStaticFactory], {}
135+
/* options */
136+
);
137+
const diags = extendedTemplateChecker.getDiagnosticsForComponent(component);
138+
expect(diags.length).toBe(0);
139+
});
140+
});
141+
});

0 commit comments

Comments
 (0)