diff --git a/packages/compiler-cli/src/ngtsc/typecheck/extended/checks/uninvoked_track_function/index.ts b/packages/compiler-cli/src/ngtsc/typecheck/extended/checks/uninvoked_track_function/index.ts index 311f306d3bc8..144d079505db 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/extended/checks/uninvoked_track_function/index.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/extended/checks/uninvoked_track_function/index.ts @@ -59,18 +59,27 @@ class UninvokedTrackFunctionCheck extends TemplateCheckWithVisitor 0) { - const fullExpressionText = generateStringFromExpression( - node.trackBy.ast, - node.trackBy.source || '', - ); - - const errorString = formatExtendedError( - ErrorCode.UNINVOKED_TRACK_FUNCTION, - `The track function in the @for block should be invoked: ${fullExpressionText}(/* arguments */)`, - ); - - return [ctx.makeTemplateDiagnostic(node.sourceSpan, errorString)]; + if (type) { + const callSignatures = type.getCallSignatures(); + if (callSignatures.length > 0) { + const hasParameters = callSignatures.some((sig) => sig.parameters.length > 0); + const tsSymbol = ctx.templateTypeChecker.getTsSymbolOfSymbol(symbol); + const isMethod = isMethodSymbol(tsSymbol, callSignatures); + + if (hasParameters || isMethod) { + const fullExpressionText = generateStringFromExpression( + node.trackBy.ast, + node.trackBy.source || '', + ); + + const errorString = formatExtendedError( + ErrorCode.UNINVOKED_TRACK_FUNCTION, + `The track function in the @for block should be invoked: ${fullExpressionText}(/* arguments */)`, + ); + + return [ctx.makeTemplateDiagnostic(node.sourceSpan, errorString)]; + } + } } } @@ -78,6 +87,29 @@ class UninvokedTrackFunctionCheck extends TemplateCheckWithVisitor ts.isMethodDeclaration(decl) || ts.isMethodSignature(decl))) { + return true; + } + } + } + + return callSignatures.some( + (sig) => + sig.declaration !== undefined && + (ts.isMethodDeclaration(sig.declaration) || ts.isMethodSignature(sig.declaration)), + ); +} + function generateStringFromExpression(expression: AST, source: string): string { return source.substring(expression.span.start, expression.span.end); } diff --git a/packages/compiler-cli/src/ngtsc/typecheck/extended/test/checks/uninvoked_track_function/BUILD.bazel b/packages/compiler-cli/src/ngtsc/typecheck/extended/test/checks/uninvoked_track_function/BUILD.bazel index 4283d105759a..054485eee329 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/extended/test/checks/uninvoked_track_function/BUILD.bazel +++ b/packages/compiler-cli/src/ngtsc/typecheck/extended/test/checks/uninvoked_track_function/BUILD.bazel @@ -23,5 +23,6 @@ jasmine_test( data = [ ":test_lib", "//packages/core:npm_package", + "//packages/forms:npm_package", ], ) diff --git a/packages/compiler-cli/src/ngtsc/typecheck/extended/test/checks/uninvoked_track_function/uninvoked_track_function.spec.ts b/packages/compiler-cli/src/ngtsc/typecheck/extended/test/checks/uninvoked_track_function/uninvoked_track_function.spec.ts index 734e8e21d6f8..670740542984 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/extended/test/checks/uninvoked_track_function/uninvoked_track_function.spec.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/extended/test/checks/uninvoked_track_function/uninvoked_track_function.spec.ts @@ -8,6 +8,7 @@ import ts from 'typescript'; +import {formatExtendedError} from '@angular/compiler-cli/src/ngtsc/typecheck/extended/api'; import {ErrorCode, ExtendedTemplateDiagnosticName, ngErrorCode} from '../../../../../diagnostics'; import {absoluteFrom, getSourceFileOrError} from '../../../../../file_system'; import {runInEachFileSystem} from '../../../../../file_system/testing'; @@ -15,7 +16,6 @@ import {getSourceCodeForDiagnostic} from '../../../../../testing'; import {getClass, setup} from '../../../../testing'; import {factory as uninvokedTrackFunctionCheckFactory} from '../../../checks/uninvoked_track_function'; import {ExtendedTemplateCheckerImpl} from '../../../src/extended_template_checker'; -import {formatExtendedError} from '@angular/compiler-cli/src/ngtsc/typecheck/extended/api'; runInEachFileSystem(() => { describe('UninvokedTrackFunctionCheck', () => { @@ -75,23 +75,163 @@ runInEachFileSystem(() => { expect(diags.length).toBe(0); }); + + it('should not produce a warning when track is a FieldTree property', () => { + const diags = diagnoseTestComponent( + ` + @for (row of rows; track row.field) {} + `, + `rows!: {field: FieldTree}[];`, + `import type {FieldTree} from '@angular/forms/signals';`, + ); + + expect(diags.length).toBe(0); + }); + + it('should not produce a warning when track is a ReadonlyFieldTree property', () => { + const diags = diagnoseTestComponent( + ` + @for (row of rows; track row.field) {} + `, + `rows!: {field: ReadonlyFieldTree}[];`, + `import type {ReadonlyFieldTree} from '@angular/forms/signals';`, + ); + + expect(diags.length).toBe(0); + }); + + it('should not produce a warning when track is a Field property', () => { + const diags = diagnoseTestComponent( + ` + @for (row of rows; track row.field) {} + `, + `rows!: {field: Field}[];`, + `import type {Field} from '@angular/forms/signals';`, + ); + + expect(diags.length).toBe(0); + }); + + it('should not produce a warning when track is a nested FieldTree property', () => { + const diags = diagnoseTestComponent( + ` + @for (row of rows; track row.field.subField) {} + `, + `rows!: {field: FieldTree<{subField: string}>}[];`, + `import type {FieldTree} from '@angular/forms/signals';`, + ); + + expect(diags.length).toBe(0); + }); + + it('should produce a warning when track is a regular function on an object', () => { + const diags = diagnoseTestComponent( + ` + @for (row of rows; track row.trackFn) {} + `, + `rows!: {trackFn: (item: any) => string}[];`, + ); + + expect(diags.length).toBe(1); + expect(diags[0].category).toBe(ts.DiagnosticCategory.Warning); + expect(diags[0].code).toBe(ngErrorCode(ErrorCode.UNINVOKED_TRACK_FUNCTION)); + expect(getSourceCodeForDiagnostic(diags[0])).toBe(`@for (row of rows; track row.trackFn) {}`); + expect(diags[0].messageText).toBe(generateDiagnosticText('row.trackFn')); + }); + + it('should not produce a warning when track is a simple callable object with no parameters', () => { + const diags = diagnoseTestComponent( + ` + @for (item of callableItems; track item) {} + @for (row of rows; track row.item) {} + `, + ` + callableItems!: (((() => string) & {id: number})[]); + rows!: {item: (() => string) & {id: number}}[]; + `, + ); + + expect(diags.length).toBe(0); + }); + + it('should produce a warning when track is a method on an item', () => { + const diags = diagnoseTestComponent( + ` + @for (item of itemsWithMethod; track item.getId) {} + `, + `itemsWithMethod!: {getId(): string}[];`, + ); + + expect(diags.length).toBe(1); + expect(diags[0].category).toBe(ts.DiagnosticCategory.Warning); + expect(diags[0].code).toBe(ngErrorCode(ErrorCode.UNINVOKED_TRACK_FUNCTION)); + expect(getSourceCodeForDiagnostic(diags[0])).toBe( + `@for (item of itemsWithMethod; track item.getId) {}`, + ); + expect(diags[0].messageText).toBe(generateDiagnosticText('item.getId')); + }); + + it('should produce a warning when track is an arrow function property on component', () => { + const diags = diagnoseTestComponent( + ` + @for (item of items; track trackFn) {} + `, + `trackFn = (item: any) => item.name;`, + ); + + expect(diags.length).toBe(1); + expect(diags[0].category).toBe(ts.DiagnosticCategory.Warning); + expect(diags[0].code).toBe(ngErrorCode(ErrorCode.UNINVOKED_TRACK_FUNCTION)); + expect(getSourceCodeForDiagnostic(diags[0])).toBe(`@for (item of items; track trackFn) {}`); + expect(diags[0].messageText).toBe(generateDiagnosticText('trackFn')); + }); + + it('should not produce a warning with signal forms field tracking patterns', () => { + const diags = diagnoseTestComponent( + ` + @for (email of emailsForm.emails; track email) {} + @for (row of rows(); track row.field) {} + `, + ` + readonly model = signal({ + emails: ['john.doe@mail.com', 'max.musterman@mail.com'], + }); + readonly emailsForm = form(this.model); + + readonly rows = computed(() => + this.model().emails.map((_, index) => ({ + index, + field: this.emailsForm.emails[index], + })) + ); + `, + `import {computed, signal} from '@angular/core';\nimport {form} from '@angular/forms/signals';`, + ); + + expect(diags.length).toBe(0); + }); }); }); -function diagnoseTestComponent(template: string, classField: string) { +function diagnoseTestComponent(template: string, classField: string, imports: string = '') { const fileName = absoluteFrom('/main.ts'); - const {program, templateTypeChecker} = setup([ - { - fileName, - templates: {'TestCmp': template}, - source: ` + const {program, templateTypeChecker} = setup( + [ + { + fileName, + templates: {'TestCmp': template}, + source: ` + ${imports} export class TestCmp { items = [{name: 'a'}, {name: 'b'}]; signalItems = [{name: signal('a')}, {name: signal('b')}]; ${classField} }`, - }, - ]); + }, + ], + {}, + {forms: true}, + ); const sf = getSourceFileOrError(program, fileName); const component = getClass(sf, 'TestCmp'); const extendedTemplateChecker = new ExtendedTemplateCheckerImpl( diff --git a/packages/compiler-cli/src/ngtsc/typecheck/testing/index.ts b/packages/compiler-cli/src/ngtsc/typecheck/testing/index.ts index 8d160ccb7352..ff31690e958b 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/testing/index.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/testing/index.ts @@ -180,6 +180,21 @@ export function angularCoreDtsFiles(): TestFile[] { }))); } +let _angularFormsDts: TestFile[] | null = null; +export function angularFormsDtsFiles(): TestFile[] { + if (_angularFormsDts !== null) { + return _angularFormsDts; + } + + const directory = resolveFromRunfiles('_main/packages/forms/npm_package'); + const dtsFiles = globSync('**/*.d.ts', {cwd: directory}); + + return (_angularFormsDts = ['package.json', ...dtsFiles].map((fileName) => ({ + name: absoluteFrom(`/node_modules/@angular/forms/${fileName}`), + contents: readFileSync(path.join(directory, fileName), 'utf8'), + }))); +} + export function angularAnimationsDts(): TestFile { return { name: absoluteFrom('/node_modules/@angular/animations/index.d.ts'), @@ -533,12 +548,20 @@ export function setup( parseOptions?: ParseTemplateOptions; referenceEmitter?: ReferenceEmitter; } = {}, + load: { + forms?: boolean; + } = {}, ): { templateTypeChecker: TemplateTypeChecker; program: ts.Program; programStrategy: TsCreateProgramDriver; } { - const files = [typescriptLibDts(), ...angularCoreDtsFiles(), angularAnimationsDts()]; + const files = [ + typescriptLibDts(), + ...angularCoreDtsFiles(), + angularAnimationsDts(), + ...(load.forms ? angularFormsDtsFiles() : []), + ]; const fakeMetadataRegistry = new Map(); const shims = new Map();