mirror of
https://github.com/angular/angular
synced 2026-05-24 09:28:37 +00:00
fix(compiler-cli): detect uninvoked functions in defer trigger expressions
Wrap `@defer` trigger expressions (`when`, `prefetch when`, `hydrate when`) in a conditional context within the TCB to enable TypeScript's TS2774 diagnostic for detecting functions used without invocation. Previously, signals and functions passed to `when` triggers without parentheses would silently evaluate to truthy, causing unexpected behavior. Now the compiler reports an error when a function is used as a condition without being called.
This commit is contained in:
parent
57edfaf435
commit
f90e5565e0
4 changed files with 94 additions and 6 deletions
|
|
@ -75,6 +75,34 @@ export class TcbExpressionOp extends TcbOp {
|
|||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* A `TcbOp` which renders an Angular expression inside a conditional context.
|
||||
* This is used for `@defer` triggers (`when`, `prefetch when`, `hydrate when`)
|
||||
* to enable TypeScript's TS2774 diagnostic for uninvoked functions/signals.
|
||||
*
|
||||
* Executing this operation returns nothing.
|
||||
*/
|
||||
export class TcbConditionOp extends TcbOp {
|
||||
constructor(
|
||||
private tcb: Context,
|
||||
private scope: Scope,
|
||||
private expression: AST,
|
||||
) {
|
||||
super();
|
||||
}
|
||||
|
||||
override get optional() {
|
||||
return false;
|
||||
}
|
||||
|
||||
override execute(): null {
|
||||
const expr = tcbExpression(this.expression, this.tcb, this.scope);
|
||||
// Wrap in an if-statement to enable TS2774 for uninvoked signals/functions.
|
||||
this.scope.addStatement(ts.factory.createIfStatement(expr, ts.factory.createBlock([])));
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
export class TcbExpressionTranslator {
|
||||
constructor(
|
||||
protected tcb: Context,
|
||||
|
|
|
|||
|
|
@ -40,7 +40,7 @@ import {Context} from './context';
|
|||
import {TcbTemplateBodyOp, TcbTemplateContextOp} from './template';
|
||||
import {TcbElementOp} from './element';
|
||||
import {addParseSpanInfo} from '../diagnostics';
|
||||
import {tcbExpression, TcbExpressionOp} from './expression';
|
||||
import {tcbExpression, TcbConditionOp, TcbExpressionOp} from './expression';
|
||||
import {TcbBlockImplicitVariableOp, TcbBlockVariableOp, TcbTemplateVariableOp} from './variables';
|
||||
import {TcbComponentContextCompletionOp} from './completions';
|
||||
import {LocalSymbol, TcbInvalidReferenceOp, TcbReferenceOp} from './references';
|
||||
|
|
@ -945,7 +945,7 @@ export class Scope {
|
|||
|
||||
// Only the `when` hydration trigger needs to be checked.
|
||||
if (block.hydrateTriggers.when) {
|
||||
this.opQueue.push(new TcbExpressionOp(this.tcb, this, block.hydrateTriggers.when.value));
|
||||
this.opQueue.push(new TcbConditionOp(this.tcb, this, block.hydrateTriggers.when.value));
|
||||
}
|
||||
|
||||
this.appendChildren(block);
|
||||
|
|
@ -968,7 +968,7 @@ export class Scope {
|
|||
triggers: TmplAstDeferredBlockTriggers,
|
||||
): void {
|
||||
if (triggers.when !== undefined) {
|
||||
this.opQueue.push(new TcbExpressionOp(this.tcb, this, triggers.when.value));
|
||||
this.opQueue.push(new TcbConditionOp(this.tcb, this, triggers.when.value));
|
||||
}
|
||||
|
||||
if (triggers.viewport !== undefined && triggers.viewport.options !== null) {
|
||||
|
|
|
|||
|
|
@ -1843,7 +1843,7 @@ describe('type check blocks', () => {
|
|||
}
|
||||
`;
|
||||
|
||||
expect(tcb(TEMPLATE)).toContain('((this).shouldShow()) && (((this).isVisible));');
|
||||
expect(tcb(TEMPLATE)).toContain('if (((this).shouldShow()) && (((this).isVisible))) { }');
|
||||
});
|
||||
|
||||
it('should generate `prefetch when` trigger', () => {
|
||||
|
|
@ -1853,7 +1853,7 @@ describe('type check blocks', () => {
|
|||
}
|
||||
`;
|
||||
|
||||
expect(tcb(TEMPLATE)).toContain('((this).shouldShow()) && (((this).isVisible));');
|
||||
expect(tcb(TEMPLATE)).toContain('if (((this).shouldShow()) && (((this).isVisible))) { }');
|
||||
});
|
||||
|
||||
it('should generate `hydrate when` trigger', () => {
|
||||
|
|
@ -1863,7 +1863,7 @@ describe('type check blocks', () => {
|
|||
}
|
||||
`;
|
||||
|
||||
expect(tcb(TEMPLATE)).toContain('((this).shouldShow()) && (((this).isVisible));');
|
||||
expect(tcb(TEMPLATE)).toContain('if (((this).shouldShow()) && (((this).isVisible))) { }');
|
||||
});
|
||||
|
||||
it('should generate options for `viewport` trigger', () => {
|
||||
|
|
|
|||
|
|
@ -4957,6 +4957,66 @@ suppress
|
|||
]);
|
||||
});
|
||||
|
||||
it('should check that functions are invoked in `when` trigger', () => {
|
||||
env.write(
|
||||
'test.ts',
|
||||
`
|
||||
import {Component, signal} from '@angular/core';
|
||||
|
||||
@Component({
|
||||
template: \`@defer (when flag) {Hello}\`,
|
||||
})
|
||||
export class Main {
|
||||
flag = signal(false);
|
||||
}
|
||||
`,
|
||||
);
|
||||
|
||||
const diags = env.driveDiagnostics();
|
||||
expect(diags.length).toBe(1);
|
||||
expect(diags[0].messageText).toContain('always return true');
|
||||
});
|
||||
|
||||
it('should check that functions are invoked in `prefetch when` trigger', () => {
|
||||
env.write(
|
||||
'test.ts',
|
||||
`
|
||||
import {Component, signal} from '@angular/core';
|
||||
|
||||
@Component({
|
||||
template: \`@defer (prefetch when flag) {Hello}\`,
|
||||
})
|
||||
export class Main {
|
||||
flag = signal(false);
|
||||
}
|
||||
`,
|
||||
);
|
||||
|
||||
const diags = env.driveDiagnostics();
|
||||
expect(diags.length).toBe(1);
|
||||
expect(diags[0].messageText).toContain('always return true');
|
||||
});
|
||||
|
||||
it('should check that functions are invoked in `hydrate when` trigger', () => {
|
||||
env.write(
|
||||
'test.ts',
|
||||
`
|
||||
import {Component, signal} from '@angular/core';
|
||||
|
||||
@Component({
|
||||
template: \`@defer (hydrate when flag) {Hello}\`,
|
||||
})
|
||||
export class Main {
|
||||
flag = signal(false);
|
||||
}
|
||||
`,
|
||||
);
|
||||
|
||||
const diags = env.driveDiagnostics();
|
||||
expect(diags.length).toBe(1);
|
||||
expect(diags[0].messageText).toContain('always return true');
|
||||
});
|
||||
|
||||
it('should report if a deferred trigger reference does not exist', () => {
|
||||
env.write(
|
||||
'test.ts',
|
||||
|
|
|
|||
Loading…
Reference in a new issue