Skip to content

Commit d98cc32

Browse files
committed
fix(compiler-cli): allow passing uninvoked signals as foreign component props
Avoid triggering the `interpolated_signal_not_invoked` diagnostic when a signal is passed directly as a property binding to a foreign component. Foreign components may accept signals directly, so they should not be flagged as uninvoked in this context. To support testing this, the typecheck testing infrastructure was updated to allow defining mock foreign components in the test setup.
1 parent 5836f98 commit d98cc32

3 files changed

Lines changed: 80 additions & 6 deletions

File tree

packages/compiler-cli/src/ngtsc/typecheck/extended/checks/interpolated_signal_not_invoked/index.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,10 @@ class InterpolatedSignalCheck extends TemplateCheckWithVisitor<ErrorCode.INTERPO
6161
}
6262
// check bound inputs like `[prop]="mySignal"` on an element or inline template
6363
else if (node instanceof TmplAstElement && node.inputs.length > 0) {
64+
// Allow signals to be passed directly to foreign components, without invocation.
65+
if (ctx.templateTypeChecker.getForeignComponent(component, node) !== null) {
66+
return [];
67+
}
6468
const directivesOfElement = ctx.templateTypeChecker.getDirectivesOfNode(component, node);
6569
return node.inputs.flatMap((input) =>
6670
checkBoundAttribute(ctx, component, directivesOfElement, input),

packages/compiler-cli/src/ngtsc/typecheck/extended/test/checks/interpolated_signal_not_invoked/interpolated_signal_not_invoked_spec.ts

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -353,6 +353,38 @@ runInEachFileSystem(() => {
353353
expect(diags.length).toBe(0);
354354
});
355355

356+
it('should not produce a warning when a signal is not invoked in a property binding on a foreign component', () => {
357+
const fileName = absoluteFrom('/main.ts');
358+
const {program, templateTypeChecker} = setup([
359+
{
360+
fileName,
361+
templates: {
362+
'TestCmp': `<FancyButton [disabled]="mySignal"></FancyButton>`,
363+
},
364+
source: `
365+
import {signal} from '@angular/core';
366+
367+
export function FancyButton() {}
368+
369+
export class TestCmp {
370+
mySignal = signal(false);
371+
}`,
372+
foreignComponents: ['FancyButton'],
373+
},
374+
]);
375+
const sf = getSourceFileOrError(program, fileName);
376+
const component = getClass(sf, 'TestCmp');
377+
const extendedTemplateChecker = new ExtendedTemplateCheckerImpl(
378+
templateTypeChecker,
379+
program.getTypeChecker(),
380+
[interpolatedSignalFactory],
381+
{},
382+
/* options */
383+
);
384+
const diags = extendedTemplateChecker.getDiagnosticsForComponent(component);
385+
expect(diags.length).toBe(0);
386+
});
387+
356388
it('should produce a warning when a signal in a nested property read is not invoked', () => {
357389
const fileName = absoluteFrom('/main.ts');
358390
const {program, templateTypeChecker} = setup([

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

Lines changed: 44 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import {
1212
ClassPropertyMapping,
1313
CssSelector,
1414
DomSchemaChecker,
15+
ForeignComponentMeta,
1516
MatchSource,
1617
OutOfBandDiagnosticRecorder,
1718
ParseSourceFile,
@@ -78,6 +79,7 @@ import {
7879
AmbientImport,
7980
ClassDeclaration,
8081
isNamedClassDeclaration,
82+
isNamedFunctionDeclaration,
8183
TypeScriptReflectionHost,
8284
} from '../../reflection';
8385
import {
@@ -362,6 +364,7 @@ export function tcb(
362364
config?: Partial<TypeCheckingConfig>,
363365
options?: {emitSpans?: boolean},
364366
templateParserOptions?: ParseTemplateOptions,
367+
foreignComponents: string[] = [],
365368
): string {
366369
const codeLines = [
367370
'declare const ɵNgFieldDirective: unique symbol;',
@@ -398,13 +401,15 @@ export function tcb(
398401
throw new Error('Template parse errors: \n' + errors.join('\n'));
399402
}
400403

401-
const {matcher, pipes} = prepareDeclarations(
404+
const {matcher, pipes, foreignMatcher} = prepareDeclarations(
402405
declarations,
403406
(decl) => getClass(sf, decl.name),
404407
new Map(),
405408
selectorlessEnabled,
409+
foreignComponents,
410+
(name) => getFunction(sf, name),
406411
);
407-
const binder = new R3TargetBinder<DirectiveMeta>(matcher);
412+
const binder = new R3TargetBinder<DirectiveMeta>(matcher, foreignMatcher);
408413
const boundTarget = binder.bind({template: nodes});
409414

410415
const id = 'tcb' as TypeCheckId;
@@ -503,6 +508,11 @@ export interface TypeCheckingTarget {
503508
* components in this file.
504509
*/
505510
declarations?: TestDeclaration[];
511+
512+
/**
513+
* Names of foreign components that are available in the template scope.
514+
*/
515+
foreignComponents?: string[];
506516
}
507517

508518
/**
@@ -622,6 +632,7 @@ export function setup(
622632
}
623633

624634
const declarations = target.declarations ?? [];
635+
const foreignComponents = target.foreignComponents ?? [];
625636

626637
for (const className of Object.keys(target.templates)) {
627638
const classDecl = getClass(sf, className);
@@ -633,7 +644,7 @@ export function setup(
633644
throw new Error('Template parse errors: \n' + errors.join('\n'));
634645
}
635646

636-
const {matcher, pipes} = prepareDeclarations(
647+
const {matcher, pipes, foreignMatcher} = prepareDeclarations(
637648
declarations,
638649
(decl) => {
639650
let declFile = sf;
@@ -647,8 +658,10 @@ export function setup(
647658
},
648659
fakeMetadataRegistry,
649660
overrides.parseOptions?.enableSelectorless ?? false,
661+
foreignComponents,
662+
(name) => getFunction(sf, name),
650663
);
651-
const binder = new R3TargetBinder<DirectiveMeta>(matcher);
664+
const binder = new R3TargetBinder<DirectiveMeta>(matcher, foreignMatcher);
652665
const classRef = new Reference(classDecl);
653666
const templateContext: TemplateContext = {
654667
nodes,
@@ -820,6 +833,8 @@ function prepareDeclarations(
820833
resolveDeclaration: DeclarationResolver,
821834
metadataRegistry: Map<string, TypeCheckableDirectiveMeta>,
822835
selectorlessEnabled: boolean,
836+
foreignComponentNames: string[] = [],
837+
resolveForeignComponent: (name: string) => ClassDeclaration,
823838
) {
824839
const pipes = new Map<string, PipeMeta>();
825840
const hostDirectiveResolder = new HostDirectivesResolver(
@@ -850,6 +865,17 @@ function prepareDeclarations(
850865
}
851866
}
852867

868+
const foreignRegistry = new Map<string, ForeignComponentMeta[]>();
869+
for (const name of foreignComponentNames) {
870+
foreignRegistry.set(name, [
871+
{
872+
name,
873+
ref: new Reference(resolveForeignComponent(name)),
874+
},
875+
]);
876+
}
877+
const foreignMatcher = new SelectorlessMatcher<ForeignComponentMeta>(foreignRegistry);
878+
853879
// We need to make two passes over the directives so that all declarations
854880
// have been registered by the time we resolve the host directives.
855881

@@ -858,7 +884,7 @@ function prepareDeclarations(
858884
for (const meta of directives) {
859885
registry.set(meta.name, [meta, ...hostDirectiveResolder.resolve(meta)]);
860886
}
861-
return {matcher: new SelectorlessMatcher<DirectiveMeta>(registry), pipes};
887+
return {matcher: new SelectorlessMatcher<DirectiveMeta>(registry), pipes, foreignMatcher};
862888
} else {
863889
const matcher = new SelectorMatcher<DirectiveMeta[]>();
864890
for (const meta of directives) {
@@ -867,7 +893,7 @@ function prepareDeclarations(
867893
matcher.addSelectables(selector, matches);
868894
}
869895

870-
return {matcher, pipes};
896+
return {matcher, pipes, foreignMatcher};
871897
}
872898
}
873899

@@ -880,6 +906,18 @@ export function getClass(sf: ts.SourceFile, name: string): ClassDeclaration<ts.C
880906
throw new Error(`Class ${name} not found in file: ${sf.fileName}: ${sf.text}`);
881907
}
882908

909+
export function getFunction(
910+
sf: ts.SourceFile,
911+
name: string,
912+
): ClassDeclaration<ts.FunctionDeclaration> {
913+
for (const stmt of sf.statements) {
914+
if (isNamedFunctionDeclaration(stmt) && stmt.name.text === name) {
915+
return stmt;
916+
}
917+
}
918+
throw new Error(`Function ${name} not found in file: ${sf.fileName}`);
919+
}
920+
883921
function getDirectiveMetaFromDeclaration(
884922
decl: TestDirective,
885923
resolveDeclaration: DeclarationResolver,

0 commit comments

Comments
 (0)