Skip to content

Commit

Permalink
fix(compiler-cli): interpolatedSignalNotInvoked diagnostic (angular#5…
Browse files Browse the repository at this point in the history
…3585)

The diagnostic was catching the following case:

```ts
name = signal('Angular');
```

but not the following ones:

```ts
name = signal('Angular').asReadonly();
name = computed(() => 'Angular');
name!: Signal<string>
```

This was not catched in the tests because the type of `Signal` is different than the one actually used in core.
It turns out the real type forces the diagnostic to check both the `symbol.tsType.symbol` and the `symbol.tsType.aliasSymbol`.

PR Close angular#53585
  • Loading branch information
cexbrayat authored and danieljancar committed Jan 26, 2024
1 parent b5c5b4d commit 66c71b8
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 6 deletions.
Expand Up @@ -37,16 +37,19 @@ class InterpolatedSignalCheck extends
}
}

function isSignal(symbol: ts.Symbol|undefined): boolean {
return (symbol?.escapedName === 'WritableSignal' || symbol?.escapedName === 'Signal') &&
(symbol as any).parent.escapedName.includes('@angular/core');
}

function buildDiagnosticForSignal(
ctx: TemplateContext<ErrorCode.INTERPOLATED_SIGNAL_NOT_INVOKED>, node: PropertyRead,
component: ts.ClassDeclaration):
Array<NgTemplateDiagnostic<ErrorCode.INTERPOLATED_SIGNAL_NOT_INVOKED>> {
const symbol = ctx.templateTypeChecker.getSymbolOfNode(node, component);

if (symbol?.kind === SymbolKind.Expression &&
/* can this condition be improved ? */
(symbol.tsType.symbol?.escapedName === 'WritableSignal' ||
symbol.tsType.symbol?.escapedName === 'Signal') &&
(symbol.tsType.symbol as any).parent.escapedName.includes('@angular/core')) {
(isSignal(symbol.tsType.symbol) || isSignal(symbol.tsType.aliasSymbol))) {
const templateMapping =
ctx.templateTypeChecker.getTemplateMappingAtTcbLocation(symbol.tcbLocation)!;

Expand Down
Expand Up @@ -20,11 +20,16 @@ function coreDtsWithSignals() {
return {
fileName: absoluteFrom('/node_modules/@angular/core/index.d.ts'),
source: `
export class Signal<T> {};
export declare const SIGNAL: unique symbol;
export declare type Signal<T> = (() => T) & {
[SIGNAL]: unknown;
};
export declare function signal<T>(initialValue: T): WritableSignal<T>;
export declare function computed<T>(computation: () => T): Signal<T>;
export interface WritableSignal<T> extends Signal<T> {}
export interface WritableSignal<T> extends Signal<T> {
asReadonly(): Signal<T>;
}
`,
templates: {},
};
Expand Down Expand Up @@ -97,6 +102,35 @@ runInEachFileSystem(() => {
expect(getSourceCodeForDiagnostic(diags[1])).toBe(`mySignal2`);
});

it('should produce a warning when a readonly signal isn\'t invoked', () => {
const fileName = absoluteFrom('/main.ts');
const {program, templateTypeChecker} = setup([
coreDtsWithSignals(),
{
fileName,
templates: {
'TestCmp': `<div>{{ count }}</div>`,
},
source: `
import {signal} from '@angular/core';
export class TestCmp {
count = signal(0).asReadonly();
}`,
},
]);
const sf = getSourceFileOrError(program, fileName);
const component = getClass(sf, 'TestCmp');
const extendedTemplateChecker = new ExtendedTemplateCheckerImpl(
templateTypeChecker, program.getTypeChecker(), [interpolatedSignalFactory], {} /* 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.INTERPOLATED_SIGNAL_NOT_INVOKED));
expect(getSourceCodeForDiagnostic(diags[0])).toBe('count');
});

it('should produce a warning when a computed signal isn\'t invoked', () => {
const fileName = absoluteFrom('/main.ts');
const {program, templateTypeChecker} = setup([
Expand Down

0 comments on commit 66c71b8

Please sign in to comment.