diff --git a/packages/compiler-cli/src/ngtsc/annotations/component/src/util.ts b/packages/compiler-cli/src/ngtsc/annotations/component/src/util.ts index 1a093b88f3b..086018392c1 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/component/src/util.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/component/src/util.ts @@ -99,10 +99,18 @@ export function validateAndFlattenComponentImports( for (let i = 0; i < imports.length; i++) { const ref = imports[i]; + let refExpr = expr; + if ( + ts.isArrayLiteralExpression(expr) && + expr.elements.length === imports.length && + !expr.elements.some(ts.isSpreadAssignment) + ) { + refExpr = expr.elements[i]; + } if (Array.isArray(ref)) { const {imports: childImports, diagnostics: childDiagnostics} = - validateAndFlattenComponentImports(ref, expr, isDeferred); + validateAndFlattenComponentImports(ref, refExpr, isDeferred); flattened.push(...childImports); diagnostics.push(...childDiagnostics); } else if (ref instanceof Reference) { @@ -138,20 +146,16 @@ export function validateAndFlattenComponentImports( let diagnosticNode: ts.Node; let diagnosticValue: ResolvedValue; - if (ref instanceof DynamicValue) { + // Reporting a diagnostic on the entire array can be noisy, especially if the user has a + // large array. Attempt to determine the most accurate position within the `imports` expression to report the + // diagnostic on. + if (ref instanceof DynamicValue && isWithinExpression(ref.node, expr)) { + // Use the dynamic value position itself if it occurs within the `imports` expression. diagnosticNode = ref.node; diagnosticValue = ref; - } else if ( - ts.isArrayLiteralExpression(expr) && - expr.elements.length === imports.length && - !expr.elements.some(ts.isSpreadAssignment) && - !imports.some(Array.isArray) - ) { - // Reporting a diagnostic on the entire array can be noisy, especially if the user has a - // large array. The most common case is that the array will be static so we can reliably - // trace back a `ResolvedValue` back to its node using its index. This allows us to report - // the exact node that caused the issue. - diagnosticNode = expr.elements[i]; + } else if (refExpr !== expr) { + // The reference comes from a specific element in `expr`, so use that element to report the diagnostic on. + diagnosticNode = refExpr; diagnosticValue = ref; } else { diagnosticNode = expr; @@ -167,6 +171,17 @@ export function validateAndFlattenComponentImports( return {imports: flattened, diagnostics}; } +function isWithinExpression(node: ts.Node, expr: ts.Expression): boolean { + let current: ts.Node | undefined = node; + while (current !== undefined) { + if (current === expr) { + return true; + } + current = current.parent; + } + return false; +} + /** * Inspects `value` to determine if it resembles a `ModuleWithProviders` value. This is an * approximation only suitable for error reporting as any resolved object with an `ngModule` diff --git a/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts b/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts index e8b4cc69ac2..84380680355 100644 --- a/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts +++ b/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts @@ -2589,12 +2589,70 @@ runInEachFileSystem((os: string) => { export class HelloDir {} const someVar = {} as any; + const tuple = [() => {}] as const; @Component({ template: '
', imports: [ someVar, HelloDir, + 'invalid', + tuple, + ] + }) + export class TestCmp {} + `, + ); + + const diags = env.driveDiagnostics(); + expect(diags.length).toBe(3); + { + const message = ts.flattenDiagnosticMessageText(diags[0].messageText, '\n'); + expect(getDiagnosticSourceCode(diags[0])).toBe('someVar'); + expect(message).toContain( + `'imports' must be an array of components, directives, pipes, or NgModules.`, + ); + expect(message).toContain(`Value is of type '{}'`); + } + { + const message = ts.flattenDiagnosticMessageText(diags[1].messageText, '\n'); + expect(getDiagnosticSourceCode(diags[1])).toBe(`'invalid'`); + expect(message).toContain( + `'imports' must be an array of components, directives, pipes, or NgModules.`, + ); + expect(message).toContain(`Value is of type 'string'`); + } + { + const message = ts.flattenDiagnosticMessageText(diags[2].messageText, '\n'); + expect(getDiagnosticSourceCode(diags[2])).toBe('tuple'); + expect(message).toContain( + `'imports' must be an array of components, directives, pipes, or NgModules.`, + ); + expect(message).toContain(`Value is of type '[(not statically analyzable)]'.`); + } + }); + + it('should report imports diagnostic for declaration file in original expression', () => { + env.write( + 'node_modules/external/index.d.ts', + ` + export declare const UNRESOLVED_ITEM: readonly [unresolved]; + `, + ); + env.write( + 'test.ts', + ` + import {Component, Directive} from '@angular/core'; + import {UNRESOLVED_ITEM as Unresolved} from 'external'; + + @Directive({selector: '[hello]'}) + export class HelloDir {} + + @Component({ + template: '', + imports: [ + [Unresolved], + HelloDir, ] }) export class TestCmp {} @@ -2606,11 +2664,12 @@ runInEachFileSystem((os: string) => { ? ts.flattenDiagnosticMessageText(diags[0].messageText, '\n') : ''; expect(diags.length).toBe(1); - expect(getDiagnosticSourceCode(diags[0])).toBe('someVar'); + expect(diags[0].file!.fileName).toContain('test.ts'); + expect(getDiagnosticSourceCode(diags[0])).toBe('Unresolved'); expect(message).toContain( `'imports' must be an array of components, directives, pipes, or NgModules.`, ); - expect(message).toContain(`Value is of type '{}'`); + expect(message).toContain(`Value is of type '[(not statically analyzable)]'.`); }); describe('empty and missing selectors', () => {