mirror of
https://github.com/angular/angular.git
synced 2026-09-14 13:54:52 +08:00
feat(compiler-cli): Adds diagnostic for misconfigured @defer triggers (#64069)
Warns when @defer blocks define unreachable or redundant triggers, such as multiple main triggers, ineffective prefetches, or timer delays not scheduled before rendering. PR Close #64069
This commit is contained in:
committed by
Andrew Kushnir
parent
eee8eab4b6
commit
563dbd998c
@@ -423,6 +423,13 @@ export enum ErrorCode {
|
||||
*/
|
||||
DEFER_IMPLICIT_TRIGGER_INVALID_PLACEHOLDER = 8020,
|
||||
|
||||
/**
|
||||
* Raised when an `@defer` block defines unreachable or redundant triggers.
|
||||
* Examples: multiple main triggers, 'on immediate' together with other mains or any prefetch,
|
||||
* prefetch timer delay that is not earlier than the main timer, or an identical prefetch
|
||||
*/
|
||||
DEFER_TRIGGER_MISCONFIGURATION = 8021,
|
||||
|
||||
/**
|
||||
* A two way binding in a template has an incorrect syntax,
|
||||
* parentheses outside brackets. For example:
|
||||
|
||||
@@ -33,4 +33,5 @@ export enum ExtendedTemplateDiagnosticName {
|
||||
UNUSED_STANDALONE_IMPORTS = 'unusedStandaloneImports',
|
||||
UNPARENTHESIZED_NULLISH_COALESCING = 'unparenthesizedNullishCoalescing',
|
||||
UNINVOKED_FUNCTION_IN_TEXT_INTERPOLATION = 'uninvokedFunctionInTextInterpolation',
|
||||
DEFER_TRIGGER_MISCONFIGURATION = 'deferTriggerMisconfiguration',
|
||||
}
|
||||
|
||||
@@ -13,6 +13,7 @@ ts_project(
|
||||
"//packages/compiler-cli/src/ngtsc/diagnostics",
|
||||
"//packages/compiler-cli/src/ngtsc/typecheck/api",
|
||||
"//packages/compiler-cli/src/ngtsc/typecheck/extended/api",
|
||||
"//packages/compiler-cli/src/ngtsc/typecheck/extended/checks/defer_trigger_misconfiguration",
|
||||
"//packages/compiler-cli/src/ngtsc/typecheck/extended/checks/interpolated_signal_not_invoked",
|
||||
"//packages/compiler-cli/src/ngtsc/typecheck/extended/checks/invalid_banana_in_box",
|
||||
"//packages/compiler-cli/src/ngtsc/typecheck/extended/checks/missing_control_flow_directive",
|
||||
|
||||
+17
@@ -0,0 +1,17 @@
|
||||
load("//tools:defaults.bzl", "ts_project")
|
||||
|
||||
ts_project(
|
||||
name = "defer_trigger_misconfiguration",
|
||||
srcs = ["index.ts"],
|
||||
visibility = [
|
||||
"//packages/compiler-cli/src/ngtsc:__subpackages__",
|
||||
"//packages/compiler-cli/test/ngtsc:__pkg__",
|
||||
],
|
||||
deps = [
|
||||
"//:node_modules/typescript",
|
||||
"//packages/compiler",
|
||||
"//packages/compiler-cli/src/ngtsc/diagnostics",
|
||||
"//packages/compiler-cli/src/ngtsc/typecheck/api",
|
||||
"//packages/compiler-cli/src/ngtsc/typecheck/extended/api",
|
||||
],
|
||||
)
|
||||
+140
@@ -0,0 +1,140 @@
|
||||
/**
|
||||
* @license
|
||||
* Copyright Google LLC All Rights Reserved.
|
||||
*
|
||||
* Use of this source code is governed by an MIT-style license that can be
|
||||
* found in the LICENSE file at https://angular.dev/license
|
||||
*/
|
||||
|
||||
import ts from 'typescript';
|
||||
|
||||
import {
|
||||
TmplAstDeferredBlock,
|
||||
TmplAstDeferredTrigger,
|
||||
TmplAstHoverDeferredTrigger,
|
||||
TmplAstImmediateDeferredTrigger,
|
||||
TmplAstInteractionDeferredTrigger,
|
||||
TmplAstTimerDeferredTrigger,
|
||||
TmplAstViewportDeferredTrigger,
|
||||
} from '@angular/compiler';
|
||||
|
||||
import {ErrorCode, ExtendedTemplateDiagnosticName} from '../../../../diagnostics';
|
||||
import {NgTemplateDiagnostic} from '../../../api';
|
||||
import {TemplateCheckFactory, TemplateCheckWithVisitor, TemplateContext} from '../../api';
|
||||
|
||||
/**
|
||||
* This check implements warnings for unreachable or redundant @defer triggers.
|
||||
* Emits ErrorCode.DEFER_TRIGGER_MISCONFIGURATION with messages matching the project's
|
||||
* expected text.
|
||||
*/
|
||||
class DeferTriggerMisconfiguration extends TemplateCheckWithVisitor<ErrorCode.DEFER_TRIGGER_MISCONFIGURATION> {
|
||||
override code = ErrorCode.DEFER_TRIGGER_MISCONFIGURATION as const;
|
||||
|
||||
override visitNode(
|
||||
ctx: TemplateContext<ErrorCode.DEFER_TRIGGER_MISCONFIGURATION>,
|
||||
component: ts.ClassDeclaration,
|
||||
node: any,
|
||||
): NgTemplateDiagnostic<ErrorCode.DEFER_TRIGGER_MISCONFIGURATION>[] {
|
||||
if (!(node instanceof TmplAstDeferredBlock)) return [];
|
||||
|
||||
const mainKeys = Object.keys(node.triggers) as Array<keyof typeof node.triggers>;
|
||||
const prefetchKeys = Object.keys(node.prefetchTriggers) as Array<
|
||||
keyof typeof node.prefetchTriggers
|
||||
>;
|
||||
|
||||
// Gather actual trigger objects for mains and prefetch (only defined ones)
|
||||
const mains = mainKeys
|
||||
.map((k) => node.triggers[k])
|
||||
.filter((t): t is TmplAstDeferredTrigger => t !== undefined && t !== null);
|
||||
|
||||
const prefetches = prefetchKeys
|
||||
.map((k) => node.prefetchTriggers[k])
|
||||
.filter((t): t is TmplAstDeferredTrigger => t !== undefined && t !== null);
|
||||
|
||||
const diags: NgTemplateDiagnostic<ErrorCode.DEFER_TRIGGER_MISCONFIGURATION>[] = [];
|
||||
|
||||
// 'on immediate' dominance
|
||||
const hasImmediateMain = mains.some((t) => t instanceof TmplAstImmediateDeferredTrigger);
|
||||
if (hasImmediateMain) {
|
||||
if (mains.length > 1) {
|
||||
const msg = `The 'immediate' trigger makes additional triggers redundant.`;
|
||||
diags.push(ctx.makeTemplateDiagnostic(node.sourceSpan, msg));
|
||||
}
|
||||
if (prefetches.length > 0) {
|
||||
const msg = `Prefetch triggers have no effect because 'immediate' executes earlier.`;
|
||||
diags.push(ctx.makeTemplateDiagnostic(node.sourceSpan, msg));
|
||||
}
|
||||
}
|
||||
|
||||
// If there is exactly one main and at least one prefetch, compare them.
|
||||
if (mains.length === 1 && prefetches.length > 0) {
|
||||
const main = mains[0];
|
||||
|
||||
for (const pre of prefetches) {
|
||||
// Timer vs Timer: warn when prefetch delay >= main delay
|
||||
const isTimerTriggger =
|
||||
main instanceof TmplAstTimerDeferredTrigger && pre instanceof TmplAstTimerDeferredTrigger;
|
||||
if (isTimerTriggger) {
|
||||
const mainDelay = main.delay;
|
||||
const preDelay = pre.delay;
|
||||
if (preDelay >= mainDelay) {
|
||||
const msg = `The Prefetch 'timer(${preDelay}ms)' is not scheduled before the main 'timer(${mainDelay}ms)', so it won’t run prior to rendering. Lower the prefetch delay or remove it.`;
|
||||
diags.push(ctx.makeTemplateDiagnostic(pre.sourceSpan ?? node.sourceSpan, msg));
|
||||
}
|
||||
}
|
||||
|
||||
// Reference-based triggers (hover/interaction/viewport): only warn if both
|
||||
// have a reference and the references are identical. If references differ
|
||||
// (or one is missing), the prefetch targets a different element and
|
||||
// provides potential value.
|
||||
|
||||
const isHoverTrigger =
|
||||
main instanceof TmplAstHoverDeferredTrigger && pre instanceof TmplAstHoverDeferredTrigger;
|
||||
|
||||
const isInteractionTrigger =
|
||||
main instanceof TmplAstInteractionDeferredTrigger &&
|
||||
pre instanceof TmplAstInteractionDeferredTrigger;
|
||||
|
||||
const isViewportTrigger =
|
||||
main instanceof TmplAstViewportDeferredTrigger &&
|
||||
pre instanceof TmplAstViewportDeferredTrigger;
|
||||
|
||||
if (isHoverTrigger || isInteractionTrigger || isViewportTrigger) {
|
||||
const mainRef = main.reference;
|
||||
const preRef = pre.reference;
|
||||
if (mainRef && preRef && mainRef === preRef) {
|
||||
const kindName = main.constructor.name.replace('DeferredTrigger', '').toLowerCase();
|
||||
const msg = `Prefetch '${kindName}' matches the main trigger and provides no benefit. Remove the prefetch modifier.`;
|
||||
diags.push(ctx.makeTemplateDiagnostic(pre.sourceSpan ?? node.sourceSpan, msg));
|
||||
}
|
||||
// otherwise, different references or missing reference => no warning
|
||||
continue;
|
||||
}
|
||||
|
||||
// Syntactic identical: same class for immediate/idle/never etc. (timers handled above)
|
||||
if (
|
||||
main.constructor === pre.constructor &&
|
||||
!(main instanceof TmplAstTimerDeferredTrigger)
|
||||
) {
|
||||
const kind =
|
||||
main instanceof TmplAstImmediateDeferredTrigger
|
||||
? 'immediate'
|
||||
: main.constructor.name.replace('DeferredTrigger', '').toLowerCase();
|
||||
const msg = `Prefetch '${kind}' matches the main trigger and provides no benefit. Remove the prefetch modifier.`;
|
||||
diags.push(ctx.makeTemplateDiagnostic(pre.sourceSpan ?? node.sourceSpan, msg));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
return diags;
|
||||
}
|
||||
}
|
||||
|
||||
export const factory: TemplateCheckFactory<
|
||||
ErrorCode.DEFER_TRIGGER_MISCONFIGURATION,
|
||||
ExtendedTemplateDiagnosticName.DEFER_TRIGGER_MISCONFIGURATION
|
||||
> = {
|
||||
code: ErrorCode.DEFER_TRIGGER_MISCONFIGURATION,
|
||||
name: ExtendedTemplateDiagnosticName.DEFER_TRIGGER_MISCONFIGURATION,
|
||||
create: () => new DeferTriggerMisconfiguration(),
|
||||
};
|
||||
@@ -24,6 +24,7 @@ import {factory as unparenthesizedNullishCoalescingFactory} from './checks/unpar
|
||||
import {factory as unusedLetDeclarationFactory} from './checks/unused_let_declaration';
|
||||
import {factory as uninvokedTrackFunctionFactory} from './checks/uninvoked_track_function';
|
||||
import {factory as uninvokedFunctionInTextInterpolationFactory} from './checks/uninvoked_function_in_text_interpolation';
|
||||
import {factory as deferTriggerMisconfigurationFactory} from './checks/defer_trigger_misconfiguration';
|
||||
|
||||
export {ExtendedTemplateCheckerImpl} from './src/extended_template_checker';
|
||||
|
||||
@@ -46,6 +47,7 @@ export const ALL_DIAGNOSTIC_FACTORIES: readonly TemplateCheckFactory<
|
||||
unparenthesizedNullishCoalescingFactory,
|
||||
uninvokedTrackFunctionFactory,
|
||||
uninvokedFunctionInTextInterpolationFactory,
|
||||
deferTriggerMisconfigurationFactory,
|
||||
];
|
||||
|
||||
export const SUPPORTED_DIAGNOSTIC_NAMES = new Set<string>([
|
||||
|
||||
+27
@@ -0,0 +1,27 @@
|
||||
load("//tools:defaults.bzl", "jasmine_test", "ts_project")
|
||||
|
||||
ts_project(
|
||||
name = "test_lib",
|
||||
testonly = True,
|
||||
srcs = ["defer_trigger_misconfiguration_spec.ts"],
|
||||
deps = [
|
||||
"//:node_modules/typescript",
|
||||
"//packages/compiler",
|
||||
"//packages/compiler-cli/src/ngtsc/core:api",
|
||||
"//packages/compiler-cli/src/ngtsc/diagnostics",
|
||||
"//packages/compiler-cli/src/ngtsc/file_system",
|
||||
"//packages/compiler-cli/src/ngtsc/file_system/testing",
|
||||
"//packages/compiler-cli/src/ngtsc/testing",
|
||||
"//packages/compiler-cli/src/ngtsc/typecheck/extended",
|
||||
"//packages/compiler-cli/src/ngtsc/typecheck/extended/checks/defer_trigger_misconfiguration",
|
||||
"//packages/compiler-cli/src/ngtsc/typecheck/testing",
|
||||
],
|
||||
)
|
||||
|
||||
jasmine_test(
|
||||
name = "test",
|
||||
data = [
|
||||
":test_lib",
|
||||
"//packages/core:npm_package",
|
||||
],
|
||||
)
|
||||
+197
@@ -0,0 +1,197 @@
|
||||
/**
|
||||
* @license
|
||||
* Copyright Google LLC All Rights Reserved.
|
||||
*
|
||||
* Use of this source code is governed by an MIT-style license that can be
|
||||
* found in the LICENSE file at https://angular.dev/license
|
||||
*/
|
||||
|
||||
import ts from 'typescript';
|
||||
import {ErrorCode, ExtendedTemplateDiagnosticName, ngErrorCode} from '../../../../../diagnostics';
|
||||
import {absoluteFrom, getSourceFileOrError} from '../../../../../file_system';
|
||||
import {runInEachFileSystem} from '../../../../../file_system/testing';
|
||||
import {getClass, setup} from '../../../../testing';
|
||||
import {factory as deferTriggerMisconfigurationFactory} from '../../../checks/defer_trigger_misconfiguration';
|
||||
import {ExtendedTemplateCheckerImpl} from '../../../src/extended_template_checker';
|
||||
|
||||
runInEachFileSystem(() => {
|
||||
describe('DeferTriggerMisconfiguration', () => {
|
||||
it('binds the error code to its extended template diagnostic name', () => {
|
||||
expect(deferTriggerMisconfigurationFactory.code).toBe(
|
||||
ErrorCode.DEFER_TRIGGER_MISCONFIGURATION,
|
||||
);
|
||||
expect(deferTriggerMisconfigurationFactory.name).toBe(
|
||||
ExtendedTemplateDiagnosticName.DEFER_TRIGGER_MISCONFIGURATION,
|
||||
);
|
||||
});
|
||||
|
||||
it('should emit when on immediate coexists with other mains', () => {
|
||||
const fileName = absoluteFrom('/main.ts');
|
||||
const {program, templateTypeChecker} = setup([
|
||||
{
|
||||
fileName,
|
||||
templates: {
|
||||
'TestCmp': `@defer (on immediate; on timer(100ms) ; on viewport(ref) ) { <div></div> }`,
|
||||
},
|
||||
source: 'export class TestCmp { }',
|
||||
},
|
||||
]);
|
||||
const sf = getSourceFileOrError(program, fileName);
|
||||
const component = getClass(sf, 'TestCmp');
|
||||
const extendedTemplateChecker = new ExtendedTemplateCheckerImpl(
|
||||
templateTypeChecker,
|
||||
program.getTypeChecker(),
|
||||
[deferTriggerMisconfigurationFactory],
|
||||
{} /* options */,
|
||||
);
|
||||
const diags = extendedTemplateChecker.getDiagnosticsForComponent(component);
|
||||
expect(diags.length).toBe(1);
|
||||
expect(diags[0].category).toBe(ts.DiagnosticCategory.Warning);
|
||||
expect(diags[0].code).toBe(ngErrorCode(ErrorCode.DEFER_TRIGGER_MISCONFIGURATION));
|
||||
});
|
||||
|
||||
it('should emit when on immediate coexists with prefetch', () => {
|
||||
const fileName = absoluteFrom('/main.ts');
|
||||
const {program, templateTypeChecker} = setup([
|
||||
{
|
||||
fileName,
|
||||
templates: {
|
||||
'TestCmp': `@defer (on immediate; prefetch on viewport ) { <div></div> }`,
|
||||
},
|
||||
source: 'export class TestCmp { }',
|
||||
},
|
||||
]);
|
||||
const sf = getSourceFileOrError(program, fileName);
|
||||
const component = getClass(sf, 'TestCmp');
|
||||
const extendedTemplateChecker = new ExtendedTemplateCheckerImpl(
|
||||
templateTypeChecker,
|
||||
program.getTypeChecker(),
|
||||
[deferTriggerMisconfigurationFactory],
|
||||
{} /* options */,
|
||||
);
|
||||
const diags = extendedTemplateChecker.getDiagnosticsForComponent(component);
|
||||
expect(diags.length).toBe(1);
|
||||
expect(diags[0].category).toBe(ts.DiagnosticCategory.Warning);
|
||||
expect(diags[0].code).toBe(ngErrorCode(ErrorCode.DEFER_TRIGGER_MISCONFIGURATION));
|
||||
});
|
||||
|
||||
it('should emit when prefetch timer >= main timer', () => {
|
||||
const fileName = absoluteFrom('/main.ts');
|
||||
const {program, templateTypeChecker} = setup([
|
||||
{
|
||||
fileName,
|
||||
templates: {
|
||||
'TestCmp': `@defer (on timer(1s); prefetch on timer(2000ms)) { <div></div> }`,
|
||||
},
|
||||
source: 'export class TestCmp { }',
|
||||
},
|
||||
]);
|
||||
const sf = getSourceFileOrError(program, fileName);
|
||||
const component = getClass(sf, 'TestCmp');
|
||||
const extendedTemplateChecker = new ExtendedTemplateCheckerImpl(
|
||||
templateTypeChecker,
|
||||
program.getTypeChecker(),
|
||||
[deferTriggerMisconfigurationFactory],
|
||||
{} /* options */,
|
||||
);
|
||||
const diags = extendedTemplateChecker.getDiagnosticsForComponent(component);
|
||||
expect(diags.length).toBe(1);
|
||||
expect(diags[0].category).toBe(ts.DiagnosticCategory.Warning);
|
||||
expect(diags[0].code).toBe(ngErrorCode(ErrorCode.DEFER_TRIGGER_MISCONFIGURATION));
|
||||
});
|
||||
|
||||
it('should emit when prefetch identical to main viewport/interaction/hover', () => {
|
||||
const fileName = absoluteFrom('/main.ts');
|
||||
const {program, templateTypeChecker} = setup([
|
||||
{
|
||||
fileName,
|
||||
templates: {
|
||||
'TestCmp': `@defer (on viewport(ref); prefetch on viewport(ref)) { <div></div> }`,
|
||||
},
|
||||
source: 'export class TestCmp { }',
|
||||
},
|
||||
]);
|
||||
const sf = getSourceFileOrError(program, fileName);
|
||||
const component = getClass(sf, 'TestCmp');
|
||||
const extendedTemplateChecker = new ExtendedTemplateCheckerImpl(
|
||||
templateTypeChecker,
|
||||
program.getTypeChecker(),
|
||||
[deferTriggerMisconfigurationFactory],
|
||||
{} /* options */,
|
||||
);
|
||||
const diags = extendedTemplateChecker.getDiagnosticsForComponent(component);
|
||||
expect(diags.length).toBe(1);
|
||||
expect(diags[0].category).toBe(ts.DiagnosticCategory.Warning);
|
||||
expect(diags[0].code).toBe(ngErrorCode(ErrorCode.DEFER_TRIGGER_MISCONFIGURATION));
|
||||
});
|
||||
|
||||
it('should not emit for valid prefetch earlier than main', () => {
|
||||
const fileName = absoluteFrom('/main.ts');
|
||||
const {program, templateTypeChecker} = setup([
|
||||
{
|
||||
fileName,
|
||||
templates: {
|
||||
'TestCmp': `@defer (on timer(1s); prefetch on timer(500ms)) { <div></div> }`,
|
||||
},
|
||||
source: 'export class TestCmp { }',
|
||||
},
|
||||
]);
|
||||
const sf = getSourceFileOrError(program, fileName);
|
||||
const component = getClass(sf, 'TestCmp');
|
||||
const extendedTemplateChecker = new ExtendedTemplateCheckerImpl(
|
||||
templateTypeChecker,
|
||||
program.getTypeChecker(),
|
||||
[deferTriggerMisconfigurationFactory],
|
||||
{} /* options */,
|
||||
);
|
||||
const diags = extendedTemplateChecker.getDiagnosticsForComponent(component);
|
||||
expect(diags.length).toBe(0);
|
||||
});
|
||||
|
||||
it('should not emit when main is viewport(ref) and prefetch is viewport without reference', () => {
|
||||
const fileName = absoluteFrom('/main.ts');
|
||||
const {program, templateTypeChecker} = setup([
|
||||
{
|
||||
fileName,
|
||||
templates: {
|
||||
'TestCmp': `@defer (on viewport(ref); prefetch on viewport) { <div></div> } @placeholder { <div></div> }`,
|
||||
},
|
||||
source: 'export class TestCmp { }',
|
||||
},
|
||||
]);
|
||||
const sf = getSourceFileOrError(program, fileName);
|
||||
const component = getClass(sf, 'TestCmp');
|
||||
const extendedTemplateChecker = new ExtendedTemplateCheckerImpl(
|
||||
templateTypeChecker,
|
||||
program.getTypeChecker(),
|
||||
[deferTriggerMisconfigurationFactory],
|
||||
{} /* options */,
|
||||
);
|
||||
const diags = extendedTemplateChecker.getDiagnosticsForComponent(component);
|
||||
expect(diags.length).toBe(0);
|
||||
});
|
||||
|
||||
it('should not emit when main is interaction(refA) and prefetch is interaction(refB)', () => {
|
||||
const fileName = absoluteFrom('/main.ts');
|
||||
const {program, templateTypeChecker} = setup([
|
||||
{
|
||||
fileName,
|
||||
templates: {
|
||||
'TestCmp': `@defer (on interaction(refA); prefetch on interaction(refB)) { <div></div> } @placeholder { <div></div> }`,
|
||||
},
|
||||
source: 'export class TestCmp { }',
|
||||
},
|
||||
]);
|
||||
const sf = getSourceFileOrError(program, fileName);
|
||||
const component = getClass(sf, 'TestCmp');
|
||||
const extendedTemplateChecker = new ExtendedTemplateCheckerImpl(
|
||||
templateTypeChecker,
|
||||
program.getTypeChecker(),
|
||||
[deferTriggerMisconfigurationFactory],
|
||||
{} /* options */,
|
||||
);
|
||||
const diags = extendedTemplateChecker.getDiagnosticsForComponent(component);
|
||||
expect(diags.length).toBe(0);
|
||||
});
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user