Skip to content

Commit 23bfa10

Browse files
crisbetodylhunn
crisbeto
authored and
dylhunn
committed
fix(compiler): add diagnostic for inaccessible deferred trigger (#51922)
If a trigger element can't be accessed from the defer block, we don't generate any instructions for it. These changes add a diagnostic that will surface the error to users. PR Close #51922
1 parent 8413b64 commit 23bfa10

File tree

6 files changed

+138
-11
lines changed

6 files changed

+138
-11
lines changed

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ export enum ErrorCode {
4848
ILLEGAL_FOR_LOOP_TRACK_ACCESS = 8009,
4949
IMPORT_CYCLE_DETECTED = 3003,
5050
IMPORT_GENERATION_FAILURE = 3004,
51+
INACCESSIBLE_DEFERRED_TRIGGER_ELEMENT = 8010,
5152
INJECTABLE_DUPLICATE_PROV = 9001,
5253
INJECTABLE_INHERITS_INVALID_CONSTRUCTOR = 2016,
5354
INLINE_TCB_REQUIRED = 8900,

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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -272,6 +272,20 @@ export enum ErrorCode {
272272
*/
273273
ILLEGAL_FOR_LOOP_TRACK_ACCESS = 8009,
274274

275+
/**
276+
* The trigger of a `defer` block cannot access its trigger element,
277+
* either because it doesn't exist or it's in a different view.
278+
*
279+
* ```
280+
* @defer (on interaction(trigger)) {...}
281+
*
282+
* <ng-template>
283+
* <button #trigger></button>
284+
* </ng-template>
285+
* ```
286+
*/
287+
INACCESSIBLE_DEFERRED_TRIGGER_ELEMENT = 8010,
288+
275289
/**
276290
* A two way binding in a template has an incorrect syntax,
277291
* parentheses outside brackets. For example:

‎packages/compiler-cli/src/ngtsc/typecheck/src/oob.ts‎

Lines changed: 32 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
* found in the LICENSE file at https://angular.io/license
77
*/
88

9-
import {BindingPipe, PropertyRead, PropertyWrite, TmplAstBoundAttribute, TmplAstBoundEvent, TmplAstElement, TmplAstForLoopBlock, TmplAstReference, TmplAstTemplate, TmplAstVariable} from '@angular/compiler';
9+
import {BindingPipe, PropertyRead, PropertyWrite, TmplAstBoundAttribute, TmplAstBoundEvent, TmplAstElement, TmplAstForLoopBlock, TmplAstHoverDeferredTrigger, TmplAstInteractionDeferredTrigger, TmplAstReference, TmplAstTemplate, TmplAstVariable, TmplAstViewportDeferredTrigger} from '@angular/compiler';
1010
import ts from 'typescript';
1111

1212
import {ErrorCode, makeDiagnostic, makeRelatedInformation, ngErrorCode} from '../../diagnostics';
@@ -87,12 +87,19 @@ export interface OutOfBandDiagnosticRecorder {
8787
templateId: TemplateId, element: TmplAstElement|TmplAstTemplate, directiveName: string,
8888
isComponent: boolean, inputAliases: string[]): void;
8989

90-
9190
/**
9291
* Reports accesses of properties that aren't available in a `for` block's tracking expression.
9392
*/
9493
illegalForLoopTrackAccess(
9594
templateId: TemplateId, block: TmplAstForLoopBlock, access: PropertyRead): void;
95+
96+
/**
97+
* Reports deferred triggers that cannot access the element they're referring to.
98+
*/
99+
inaccessibleDeferredTriggerElement(
100+
templateId: TemplateId,
101+
trigger: TmplAstHoverDeferredTrigger|TmplAstInteractionDeferredTrigger|
102+
TmplAstViewportDeferredTrigger): void;
96103
}
97104

98105
export class OutOfBandDiagnosticRecorderImpl implements OutOfBandDiagnosticRecorder {
@@ -310,6 +317,29 @@ export class OutOfBandDiagnosticRecorderImpl implements OutOfBandDiagnosticRecor
310317
ts.DiagnosticCategory.Error, ngErrorCode(ErrorCode.ILLEGAL_FOR_LOOP_TRACK_ACCESS),
311318
message));
312319
}
320+
321+
inaccessibleDeferredTriggerElement(
322+
templateId: TemplateId,
323+
trigger: TmplAstHoverDeferredTrigger|TmplAstInteractionDeferredTrigger|
324+
TmplAstViewportDeferredTrigger): void {
325+
let message: string;
326+
327+
if (trigger.reference === null) {
328+
message = `Trigger cannot find reference. Make sure that the @defer block has a ` +
329+
`@placeholder with at least one root element node.`;
330+
} else {
331+
message =
332+
`Trigger cannot find reference "${trigger.reference}".\nCheck that an element with #${
333+
trigger.reference} exists in the same template and it's accessible from the ` +
334+
`@defer block.\nDeferred blocks can only access triggers in same view, a parent ` +
335+
`embedded view or the root view of the @placeholder block.`;
336+
}
337+
338+
this._diagnostics.push(makeTemplateDiagnostic(
339+
templateId, this.resolver.getSourceMapping(templateId), trigger.sourceSpan,
340+
ts.DiagnosticCategory.Error, ngErrorCode(ErrorCode.INACCESSIBLE_DEFERRED_TRIGGER_ELEMENT),
341+
message));
342+
}
313343
}
314344

315345
function makeInlineDiagnostic(

‎packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts‎

Lines changed: 48 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
* found in the LICENSE file at https://angular.io/license
77
*/
88

9-
import {AST, BindingPipe, BindingType, BoundTarget, Call, DYNAMIC_TYPE, ImplicitReceiver, ParsedEventType, ParseSourceSpan, PropertyRead, PropertyWrite, SafeCall, SafePropertyRead, SchemaMetadata, ThisReceiver, TmplAstBoundAttribute, TmplAstBoundEvent, TmplAstBoundText, TmplAstDeferredBlock, TmplAstElement, TmplAstForLoopBlock, TmplAstIcu, TmplAstIfBlock, TmplAstIfBlockBranch, TmplAstNode, TmplAstReference, TmplAstSwitchBlock, TmplAstTemplate, TmplAstTextAttribute, TmplAstVariable, TransplantedType} from '@angular/compiler';
9+
import {AST, BindingPipe, BindingType, BoundTarget, Call, DYNAMIC_TYPE, ImplicitReceiver, ParsedEventType, ParseSourceSpan, PropertyRead, PropertyWrite, SafeCall, SafePropertyRead, SchemaMetadata, ThisReceiver, TmplAstBoundAttribute, TmplAstBoundEvent, TmplAstBoundText, TmplAstDeferredBlock, TmplAstDeferredBlockTriggers, TmplAstElement, TmplAstForLoopBlock, TmplAstHoverDeferredTrigger, TmplAstIcu, TmplAstIfBlock, TmplAstIfBlockBranch, TmplAstInteractionDeferredTrigger, TmplAstNode, TmplAstReference, TmplAstSwitchBlock, TmplAstTemplate, TmplAstTextAttribute, TmplAstVariable, TmplAstViewportDeferredTrigger, TransplantedType} from '@angular/compiler';
1010
import ts from 'typescript';
1111

1212
import {Reference} from '../../imports';
@@ -1694,14 +1694,7 @@ class Scope {
16941694
}
16951695
this.checkAndAppendReferencesOfNode(node);
16961696
} else if (node instanceof TmplAstDeferredBlock) {
1697-
node.triggers.when !== undefined &&
1698-
this.opQueue.push(new TcbExpressionOp(this.tcb, this, node.triggers.when.value));
1699-
node.prefetchTriggers.when !== undefined &&
1700-
this.opQueue.push(new TcbExpressionOp(this.tcb, this, node.prefetchTriggers.when.value));
1701-
this.appendChildren(node);
1702-
node.placeholder !== null && this.appendChildren(node.placeholder);
1703-
node.loading !== null && this.appendChildren(node.loading);
1704-
node.error !== null && this.appendChildren(node.error);
1697+
this.appendDeferredBlock(node);
17051698
} else if (node instanceof TmplAstIfBlock) {
17061699
this.opQueue.push(new TcbIfOp(this.tcb, this, node));
17071700
} else if (node instanceof TmplAstSwitchBlock) {
@@ -1877,6 +1870,52 @@ class Scope {
18771870
}
18781871
}
18791872
}
1873+
1874+
private appendDeferredBlock(block: TmplAstDeferredBlock): void {
1875+
this.appendDeferredTriggers(block, block.triggers);
1876+
this.appendDeferredTriggers(block, block.prefetchTriggers);
1877+
this.appendChildren(block);
1878+
1879+
if (block.placeholder !== null) {
1880+
this.appendChildren(block.placeholder);
1881+
}
1882+
1883+
if (block.loading !== null) {
1884+
this.appendChildren(block.loading);
1885+
}
1886+
1887+
if (block.error !== null) {
1888+
this.appendChildren(block.error);
1889+
}
1890+
}
1891+
1892+
private appendDeferredTriggers(
1893+
block: TmplAstDeferredBlock, triggers: TmplAstDeferredBlockTriggers): void {
1894+
if (triggers.when !== undefined) {
1895+
this.opQueue.push(new TcbExpressionOp(this.tcb, this, triggers.when.value));
1896+
}
1897+
1898+
if (triggers.hover !== undefined) {
1899+
this.appendReferenceBasedDeferredTrigger(block, triggers.hover);
1900+
}
1901+
1902+
if (triggers.interaction !== undefined) {
1903+
this.appendReferenceBasedDeferredTrigger(block, triggers.interaction);
1904+
}
1905+
1906+
if (triggers.viewport !== undefined) {
1907+
this.appendReferenceBasedDeferredTrigger(block, triggers.viewport);
1908+
}
1909+
}
1910+
1911+
private appendReferenceBasedDeferredTrigger(
1912+
block: TmplAstDeferredBlock,
1913+
trigger: TmplAstHoverDeferredTrigger|TmplAstInteractionDeferredTrigger|
1914+
TmplAstViewportDeferredTrigger): void {
1915+
if (this.tcb.boundTarget.getDeferredTriggerTarget(block, trigger) === null) {
1916+
this.tcb.oobRecorder.inaccessibleDeferredTriggerElement(this.tcb.id, trigger);
1917+
}
1918+
}
18801919
}
18811920

18821921
interface TcbBoundAttribute {

‎packages/compiler-cli/src/ngtsc/typecheck/testing/index.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -799,4 +799,5 @@ export class NoopOobRecorder implements OutOfBandDiagnosticRecorder {
799799
splitTwoWayBinding(): void {}
800800
missingRequiredInputs(): void {}
801801
illegalForLoopTrackAccess(): void {}
802+
inaccessibleDeferredTriggerElement(): void {}
802803
}

‎packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3704,6 +3704,48 @@ suppress
37043704
`Property 'does_not_exist' does not exist on type 'Main'.`,
37053705
]);
37063706
});
3707+
3708+
it('should report if a deferred trigger reference does not exist', () => {
3709+
env.write('test.ts', `
3710+
import {Component} from '@angular/core';
3711+
3712+
@Component({
3713+
template: \`
3714+
@defer (on viewport(does_not_exist)) {Hello}
3715+
\`,
3716+
standalone: true,
3717+
})
3718+
export class Main {}
3719+
`);
3720+
3721+
const diags = env.driveDiagnostics();
3722+
expect(diags.length).toBe(1);
3723+
expect(ts.flattenDiagnosticMessageText(diags[0].messageText, ''))
3724+
.toContain('Trigger cannot find reference "does_not_exist".');
3725+
});
3726+
3727+
it('should report if a deferred trigger reference is in a different embedded view', () => {
3728+
env.write('test.ts', `
3729+
import {Component} from '@angular/core';
3730+
3731+
@Component({
3732+
template: \`
3733+
@defer (on viewport(trigger)) {Hello}
3734+
3735+
<ng-template>
3736+
<button #trigger></button>
3737+
</ng-template>
3738+
\`,
3739+
standalone: true,
3740+
})
3741+
export class Main {}
3742+
`);
3743+
3744+
const diags = env.driveDiagnostics();
3745+
expect(diags.length).toBe(1);
3746+
expect(ts.flattenDiagnosticMessageText(diags[0].messageText, ''))
3747+
.toContain('Trigger cannot find reference "trigger".');
3748+
});
37073749
});
37083750

37093751
describe('conditional blocks', () => {

0 commit comments

Comments
 (0)