10BC0 Improve Defer Trigger Misconfiguration by SkyZeroZx · Pull Request #67497 · angular/angular · GitHub
[go: up one dir, main page]
Skip to content

Improve Defer Trigger Misconfiguration #67497

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.

Sign up for GitHub

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

Open
SkyZeroZx wants to merge 2 commits into
base: main
Choose a base branch
from
+119 −13
8000
Open

Improve Defer Trigger Misconfiguration #67497

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
58c3406
refactor(compiler-cli): Add diagnostics for idle prefetch triggers
SkyZeroZx Mar 6, 2026
e3e7aac
feat(compiler-cli): Adds warning for prefetch without main defer trigger
SkyZeroZx Mar 6, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 62 additions & 13 deletions .../compiler-cli/src/ngtsc/typecheck/extended/checks/defer_trigger_misconfiguration/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
TmplAstDeferredBlock,
TmplAstDeferredTrigger,
TmplAstHoverDeferredTrigger,
TmplAstIdleDeferredTrigger,
TmplAstImmediateDeferredTrigger,
TmplAstInteractionDeferredTrigger,
TmplAstTimerDeferredTrigger,
Expand Down Expand Up @@ -60,6 +61,26 @@ function areLiteralMapsEqual(a: LiteralMap | null, b: LiteralMap | null): boolea
return true;
}

function getTimedTriggerValue(
trigger: TmplAstTimerDeferredTrigger | TmplAstIdleDeferredTrigger,
): number | null {
if (trigger instanceof TmplAstTimerDeferredTrigger) {
return trigger.delay;
}

return trigger.timeout;
}

function formatTimedTrigger(
trigger: TmplAstTimerDeferredTrigger | TmplAstIdleDeferredTrigger,
): string {
if (trigger instanceof TmplAstTimerDeferredTrigger) {
return `timer(${trigger.delay}ms)`;
}

return trigger.timeout === null ? 'idle' : `idle(${trigger.timeout}ms)`;
}

/**
* This check implements warnings for unreachable or redundant @defer triggers.
* Emits ErrorCode.DEFER_TRIGGER_MISCONFIGURATION with messages matching the project's
Expand Down Expand Up @@ -114,26 +135,54 @@ class DeferTriggerMisconfiguration extends TemplateCheckWithVisitor<ErrorCode.DE
}
}

// `prefetch` without an explicit main trigger defaults the main trigger to `idle`,
if (mains.length === 0 && prefetches.length > 0) {
const msg =
`Define a main trigger when using 'prefetch' triggers. ` +
`Without an explicit main trigger, @defer defaults to 'idle' and prefetch may have no effect.`;
diags.push(
ctx.makeTemplateDiagnostic(
node.sourceSpan,
formatExtendedError(ErrorCode.DEFER_TRIGGER_MISCONFIGURATION, 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 =
// Delay-based pairs (timer/idle): warn when prefetch fires no sooner than main.

const isTimerPair =
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,
formatExtendedError(ErrorCode.DEFER_TRIGGER_MISCONFIGURATION, msg),
),
);

const isIdlePair =
main instanceof TmplAstIdleDeferredTrigger && pre instanceof TmplAstIdleDeferredTrigger;

if (isTimerPair || isIdlePair) {
const mainVal = getTimedTriggerValue(main);
const preVal = getTimedTriggerValue(pre);
const sameUntimedIdle = mainVal == null && preVal == null;
const comparableTimedPair = mainVal != null && preVal != null && preVal >= mainVal;

if (!sameUntimedIdle && !comparableTimedPair) {
continue;
}

const msg =
`The Prefetch '${formatTimedTrigger(pre)}' is not scheduled before the main '${formatTimedTrigger(main)}',` +
` so it won't run prior to rendering. Lower the prefetch timing or remove it.`;

diags.push(
ctx.makeTemplateDiagnostic(
pre.sourceSpan ?? node.sourceSpan,
formatExtendedError(ErrorCode.DEFER_TRIGGER_MISCONFIGURATION, msg),
),
);

continue;
}

// Reference-based triggers (hover/interaction/viewport): warn if both
Expand Down
57 changes: 57 additions & 0 deletions ...xtended/test/checks/defer_trigger_misconfiguration/defer_trigger_misconfiguration_spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,35 @@ runInEachFileSystem(() => {
expect(diags[0].code).toBe(ngErrorCode(ErrorCode.DEFER_TRIGGER_MISCONFIGURATION));
});

it('should emit when prefetch idle timeout >= main idle timeout', () => {
const diags = getDiags(`@defer (on idle(1s); prefetch on idle(2s)) { <div></div> }`);
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 when prefetch idle timeout < main idle timeout', () => {
const diags = getDiags(`@defer (on idle(2s); prefetch on idle(1s)) { <div></div> }`);
expect(diags.length).toBe(0);
});

it('should emit when prefetch idle matches main idle with no timeout', () => {
const diags = getDiags(`@defer (on idle; prefetch on idle) { <div></div> }`);
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 when only main idle has timeout', () => {
const diags = getDiags(`@defer (on idle(1s); prefetch on idle) { <div></div> }`);
expect(diags.length).toBe(0);
});

it('should not emit when only prefetch idle has timeout', () => {
const diags = getDiags(`@defer (on idle; prefetch on idle(1s)) { <div></div> }`);
expect(diags.length).toBe(0);
});

it('should emit when prefetch identical to main viewport/interaction/hover', () => {
const diags = getDiags(
`@defer (on viewport(ref); prefetch on viewport(ref)) { <div></div> }`,
Expand Down Expand Up @@ -151,5 +180,33 @@ runInEachFileSystem(() => {
expect(diags[0].category).toBe(ts.DiagnosticCategory.Warning);
expect(diags[0].code).toBe(ngErrorCode(ErrorCode.DEFER_TRIGGER_MISCONFIGURATION));
});

it('should emit when prefetch trigger is configured without a main trigger', () => {
const diags = getDiags(`@defer (prefetch on viewport(ref)) { <div></div> }`);
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 is configured without a main trigger', () => {
const diags = getDiags(`@defer (prefetch on timer(500ms)) { <div></div> }`);
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 idle is configured without a main trigger', () => {
const diags = getDiags(`@defer (prefetch on idle(500)) { <div></div> }`);
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 when is configured without a main trigger', () => {
const diags = getDiags(`@defer (prefetch when true) { <div></div> }`);
expect(diags.length).toBe(1);
expect(diags[0].category).toBe(ts.DiagnosticCategory.Warning);
expect(diags[0].code).toBe(ngErrorCode(ErrorCode.DEFER_TRIGGER_MISCONFIGURATION));
});
});
});
Loading
0