diff --git a/packages/compiler-cli/src/ngtsc/annotations/component/src/handler.ts b/packages/compiler-cli/src/ngtsc/annotations/component/src/handler.ts index 14e47426317..8ca2b49ec7b 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/component/src/handler.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/component/src/handler.ts @@ -1127,6 +1127,7 @@ export class ComponentDecoratorHandler implements DecoratorHandler< foreignImports: analysis.foreignImports, rawImports: analysis.rawImports, deferredImports: analysis.resolvedDeferredImports, + deferredImportsByBlock: analysis.resolvedDeferredImportsByBlock, animationTriggerNames: analysis.legacyAnimationTriggerNames, schemas: analysis.schemas, decorator: analysis.decorator, @@ -1134,6 +1135,7 @@ export class ComponentDecoratorHandler implements DecoratorHandler< ngContentSelectors: analysis.template.ngContentSelectors, preserveWhitespaces: analysis.template.preserveWhitespaces ?? false, isExplicitlyDeferred: false, + deferredBlocks: null, selectorlessEnabled: analysis.selectorlessEnabled, localReferencedSymbols: analysis.localReferencedSymbols, }); diff --git a/packages/compiler-cli/src/ngtsc/annotations/directive/src/handler.ts b/packages/compiler-cli/src/ngtsc/annotations/directive/src/handler.ts index d7ee73ecc4a..4a570fdfb35 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/directive/src/handler.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/directive/src/handler.ts @@ -328,6 +328,7 @@ export class DirectiveDecoratorHandler implements DecoratorHandler< foreignImports: null, rawImports: null, deferredImports: null, + deferredImportsByBlock: null, schemas: null, ngContentSelectors: null, decorator: analysis.decorator, @@ -336,6 +337,7 @@ export class DirectiveDecoratorHandler implements DecoratorHandler< // Instead, we statically analyze their imports to make a direct determination. assumedToExportProviders: false, isExplicitlyDeferred: false, + deferredBlocks: null, selectorlessEnabled: false, localReferencedSymbols: null, }); diff --git a/packages/compiler-cli/src/ngtsc/annotations/src/pipe.ts b/packages/compiler-cli/src/ngtsc/annotations/src/pipe.ts index c25c18c2ade..687ed55c6df 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/src/pipe.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/src/pipe.ts @@ -247,6 +247,7 @@ export class PipeDecoratorHandler implements DecoratorHandler< isStandalone: analysis.meta.isStandalone, decorator: analysis.decorator, isExplicitlyDeferred: false, + deferredBlocks: null, isPure: analysis.meta.pure, }); diff --git a/packages/compiler-cli/src/ngtsc/metadata/src/api.ts b/packages/compiler-cli/src/ngtsc/metadata/src/api.ts index ff5a7364db6..ae5adcf30ac 100644 --- a/packages/compiler-cli/src/ngtsc/metadata/src/api.ts +++ b/packages/compiler-cli/src/ngtsc/metadata/src/api.ts @@ -275,6 +275,11 @@ export interface DirectiveMeta extends T2DirectiveMeta, DirectiveTypeCheckMeta { */ deferredImports: Reference[] | null; + /** + * Map of block name -> resolved deferred imports when `deferredImports` is an object literal. + */ + deferredImportsByBlock?: Map[]> | null; + /** * For standalone components, the list of schemas declared. */ @@ -301,6 +306,12 @@ export interface DirectiveMeta extends T2DirectiveMeta, DirectiveTypeCheckMeta { */ isExplicitlyDeferred: boolean; + /** + * Block names in `@Component.deferredImports` under which this class was imported, + * if `deferredImports` was configured as an object literal. `null` if flat array. + */ + deferredBlocks?: Set | null; + /** Whether selectorless is enabled for the specific component. */ selectorlessEnabled: boolean; @@ -359,6 +370,7 @@ export interface PipeMeta { isPure: boolean; decorator: ts.Decorator | null; isExplicitlyDeferred: boolean; + deferredBlocks?: Set | null; } /** diff --git a/packages/compiler-cli/src/ngtsc/metadata/src/dts.ts b/packages/compiler-cli/src/ngtsc/metadata/src/dts.ts index 8b2400af9ad..562597438bb 100644 --- a/packages/compiler-cli/src/ngtsc/metadata/src/dts.ts +++ b/packages/compiler-cli/src/ngtsc/metadata/src/dts.ts @@ -243,6 +243,7 @@ export class DtsMetadataReader implements MetadataReader { foreignImports: null, rawImports: null, deferredImports: null, + deferredImportsByBlock: null, // The same goes for schemas. schemas: null, decorator: null, @@ -252,6 +253,7 @@ export class DtsMetadataReader implements MetadataReader { // used to increase the accuracy of a diagnostic. preserveWhitespaces: false, isExplicitlyDeferred: false, + deferredBlocks: null, // We don't need to know if imported components from .d.ts // files are selectorless for type-checking purposes. selectorlessEnabled: false, @@ -301,6 +303,7 @@ export class DtsMetadataReader implements MetadataReader { isPure: null!, // The DTS has no idea about that decorator: null, isExplicitlyDeferred: false, + deferredBlocks: null, }; } } diff --git a/packages/compiler-cli/src/ngtsc/scope/src/standalone.ts b/packages/compiler-cli/src/ngtsc/scope/src/standalone.ts index 2e7ad9ffe3e..64c91e3e1fc 100644 --- a/packages/compiler-cli/src/ngtsc/scope/src/standalone.ts +++ b/packages/compiler-cli/src/ngtsc/scope/src/standalone.ts @@ -104,17 +104,35 @@ export class StandaloneComponentScopeReader implements ComponentScopeReader { } if (clazzMeta.deferredImports !== null) { + const refToBlocks = new Map>(); + if (clazzMeta.deferredImportsByBlock != null) { + for (const [blockName, refs] of clazzMeta.deferredImportsByBlock) { + for (const ref of refs) { + if (!refToBlocks.has(ref.node)) { + refToBlocks.set(ref.node, new Set()); + } + refToBlocks.get(ref.node)!.add(blockName); + } + } + } + for (const ref of clazzMeta.deferredImports) { + const deferredBlocks = refToBlocks.get(ref.node) ?? null; const dirMeta = this.metaReader.getDirectiveMetadata(ref); if (dirMeta !== null) { - deferredDependencies.add({...dirMeta, ref, isExplicitlyDeferred: true}); + deferredDependencies.add({...dirMeta, ref, isExplicitlyDeferred: true, deferredBlocks}); isPoisoned = isPoisoned || dirMeta.isPoisoned || !dirMeta.isStandalone; continue; } const pipeMeta = this.metaReader.getPipeMetadata(ref); if (pipeMeta !== null) { - deferredDependencies.add({...pipeMeta, ref, isExplicitlyDeferred: true}); + deferredDependencies.add({ + ...pipeMeta, + ref, + isExplicitlyDeferred: true, + deferredBlocks, + }); isPoisoned = isPoisoned || !pipeMeta.isStandalone; continue; } diff --git a/packages/compiler-cli/src/ngtsc/scope/src/typecheck.ts b/packages/compiler-cli/src/ngtsc/scope/src/typecheck.ts index 105961c1e34..74f66f1bbb4 100644 --- a/packages/compiler-cli/src/ngtsc/scope/src/typecheck.ts +++ b/packages/compiler-cli/src/ngtsc/scope/src/typecheck.ts @@ -197,8 +197,12 @@ export class TypeCheckScopeRegistry { private applyExplicitlyDeferredFlag( meta: T, isExplicitlyDeferred: boolean, + deferredBlocks?: Set | null, ): T { - return isExplicitlyDeferred === true ? {...meta, isExplicitlyDeferred} : meta; + if (isExplicitlyDeferred === true) { + return {...meta, isExplicitlyDeferred, deferredBlocks: deferredBlocks ?? null}; + } + return meta; } private getSelectorMatcher( @@ -213,8 +217,12 @@ export class TypeCheckScopeRegistry { continue; } - // Carry over the `isExplicitlyDeferred` flag from the dependency info. - const directiveMeta = this.applyExplicitlyDeferredFlag(extMeta, meta.isExplicitlyDeferred); + // Carry over the `isExplicitlyDeferred` flag and `deferredBlocks` from the dependency info. + const directiveMeta = this.applyExplicitlyDeferredFlag( + extMeta, + meta.isExplicitlyDeferred, + meta.deferredBlocks, + ); matcher.addSelectables( CssSelector.parse(meta.selector), this.combineWithHostDirectives(directiveMeta), diff --git a/packages/compiler-cli/src/ngtsc/scope/test/local_spec.ts b/packages/compiler-cli/src/ngtsc/scope/test/local_spec.ts index d157df205f8..488a08f0448 100644 --- a/packages/compiler-cli/src/ngtsc/scope/test/local_spec.ts +++ b/packages/compiler-cli/src/ngtsc/scope/test/local_spec.ts @@ -369,7 +369,9 @@ function fakeDirective(ref: Reference): DirectiveMeta { ngContentSelectors: null, preserveWhitespaces: false, isExplicitlyDeferred: false, + deferredBlocks: null, deferredImports: null, + deferredImportsByBlock: null, inputFieldNamesFromMetadataArray: null, selectorlessEnabled: false, localReferencedSymbols: null, @@ -386,6 +388,7 @@ function fakePipe(ref: Reference): PipeMeta { isStandalone: false, decorator: null, isExplicitlyDeferred: false, + deferredBlocks: null, isPure: false, }; } diff --git a/packages/compiler-cli/src/ngtsc/typecheck/api/api.ts b/packages/compiler-cli/src/ngtsc/typecheck/api/api.ts index b4156a50189..cf1ff444dc7 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/api/api.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/api/api.ts @@ -35,6 +35,7 @@ export interface TypeCheckableDirectiveMeta extends DirectiveMeta, DirectiveType hostDirectives: HostDirectiveMeta[] | null; decorator: ts.Decorator | null; isExplicitlyDeferred: boolean; + deferredBlocks?: Set | null; imports: Reference[] | null; rawImports: ts.Expression | null; } @@ -99,9 +100,7 @@ export interface TypeCheckBlockMetadata { } export type SourceMapping = - | DirectSourceMapping - | IndirectSourceMapping - | ExternalTemplateSourceMapping; + DirectSourceMapping | IndirectSourceMapping | ExternalTemplateSourceMapping; /** * A mapping to a node within the same source file.. diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/oob.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/oob.ts index d06e1fea9ab..418726b458f 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/oob.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/oob.ts @@ -139,17 +139,30 @@ export class OutOfBandDiagnosticRecorderImpl implements OutOfBandDiagnosticRecor this.recordedPipes.add(ast); } - deferredPipeUsedEagerly(id: TypeCheckId, ast: BindingPipe): void { + deferredPipeUsedEagerly( + id: TypeCheckId, + ast: BindingPipe, + currentBlockName: string | null, + declaredBlocks: string[] | null, + ): void { if (this.recordedPipes.has(ast)) { return; } const mapping = this.resolver.getTemplateSourceMapping(id); - const errorMsg = - `Pipe '${ast.name}' was imported via \`@Component.deferredImports\`, ` + - `but was used outside of a \`@defer\` block in a template. To fix this, either ` + - `use the '${ast.name}' pipe inside of a \`@defer\` block or import this dependency ` + - `using the \`@Component.imports\` field.`; + let errorMsg: string; + if (currentBlockName !== null && declaredBlocks !== null && declaredBlocks.length > 0) { + errorMsg = + `Pipe '${ast.name}' was imported via \`@Component.deferredImports\` under block '${declaredBlocks.join(', ')}', ` + + `but is used in a \`@defer\` block configured for '${currentBlockName}'. ` + + `To fix this, add '${ast.name}' to 'deferredImports.${currentBlockName}'.`; + } else { + errorMsg = + `Pipe '${ast.name}' was imported via \`@Component.deferredImports\`, ` + + `but was used outside of a \`@defer\` block in a template. To fix this, either ` + + `use the '${ast.name}' pipe inside of a \`@defer\` block or import this dependency ` + + `using the \`@Component.imports\` field.`; + } const sourceSpan = this.resolver.toTemplateParseSourceSpan(id, ast.nameSpan); if (sourceSpan === null) { @@ -170,21 +183,41 @@ export class OutOfBandDiagnosticRecorderImpl implements OutOfBandDiagnosticRecor this.recordedPipes.add(ast); } - deferredComponentUsedEagerly(id: TypeCheckId, element: TmplAstElement): void { + deferredComponentUsedEagerly( + id: TypeCheckId, + element: TmplAstElement | TmplAstTemplate, + dirMeta: TcbDirectiveMetadata, + currentBlockName: string | null, + declaredBlocks: string[] | null, + ): void { const mapping = this.resolver.getTemplateSourceMapping(id); - const errorMsg = - `Element '${element.name}' contains a component or a directive that ` + - `was imported via \`@Component.deferredImports\`, but the element itself is located ` + - `outside of a \`@defer\` block in a template. To fix this, either ` + - `use the '${element.name}' element inside of a \`@defer\` block or ` + - `import referenced component/directive dependency using the \`@Component.imports\` field.`; + const elementName = + element instanceof TmplAstElement ? element.name : (element.tagName ?? 'ng-template'); + const kind = dirMeta.isComponent ? 'Component' : 'Directive'; + const usage = dirMeta.isComponent + ? `used as element '${elementName}'` + : `used on element '${elementName}'`; + let errorMsg: string; + if (currentBlockName !== null && declaredBlocks !== null && declaredBlocks.length > 0) { + errorMsg = + `${kind} '${dirMeta.name}' (${usage}) was imported via \`@Component.deferredImports\` ` + + `under block '${declaredBlocks.join(', ')}', but is used in a \`@defer\` block configured for '${currentBlockName}'. ` + + `To fix this, add '${dirMeta.name}' to 'deferredImports.${currentBlockName}'.`; + } else { + errorMsg = + `${kind} '${dirMeta.name}' (${usage}) was imported via \`@Component.deferredImports\`, ` + + `but was used outside of a \`@defer\` block in a template. To fix this, either ` + + `use the '${elementName}' element inside of a \`@defer\` block or ` + + `import '${dirMeta.name}' using the \`@Component.imports\` field.`; + } - const {start, end} = element.startSourceSpan; + const startSourceSpan = element.startSourceSpan ?? element.sourceSpan; + const {start, end} = startSourceSpan; const absoluteSourceSpan = new AbsoluteSourceSpan(start.offset, end.offset); const sourceSpan = this.resolver.toTemplateParseSourceSpan(id, absoluteSourceSpan); if (sourceSpan === null) { throw new Error( - `Assertion failure: no SourceLocation found for usage of pipe '${element.name}'.`, + `Assertion failure: no SourceLocation found for usage of ${kind.toLowerCase()} '${dirMeta.name}'.`, ); } this._diagnostics.push( diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/tcb_adapter.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/tcb_adapter.ts index f8231eb83aa..5c5c2727d33 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/tcb_adapter.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/tcb_adapter.ts @@ -130,6 +130,7 @@ export function adaptTypeCheckBlockMetadata( isStructural: dir.isStructural, isStandalone: dir.isStandalone, isExplicitlyDeferred: dir.isExplicitlyDeferred, + deferredBlocks: dir.deferredBlocks, preserveWhitespaces: dir.preserveWhitespaces, ngContentSelectors: dir.ngContentSelectors, animationTriggerNames: dir.animationTriggerNames, @@ -207,6 +208,8 @@ export function adaptTypeCheckBlockMetadata( }, getDeferredTriggerTarget: (b, t) => meta.boundTarget.getDeferredTriggerTarget(b, t), isDeferred: (node) => meta.boundTarget.isDeferred(node), + getDeferBlocksOfNode: (node) => meta.boundTarget.getDeferBlocksOfNode(node), + getDeferBlocksOfPipe: (ast) => meta.boundTarget.getDeferBlocksOfPipe(ast), referencedDirectiveExists: (name) => meta.boundTarget.referencedDirectiveExists(name), getConsumerOfBinding: (binding) => { const consumer = meta.boundTarget.getConsumerOfBinding(binding); @@ -223,8 +226,7 @@ export function adaptTypeCheckBlockMetadata( getDeferBlocks: () => meta.boundTarget.getDeferBlocks(), getConflictingHostDirectiveBindings: (node) => meta.boundTarget.getConflictingHostDirectiveBindings(node) as - | ConflictingHostDirectiveBinding[] - | null, + ConflictingHostDirectiveBinding[] | null, }; const pipes = new Map(); @@ -238,6 +240,7 @@ export function adaptTypeCheckBlockMetadata( name: pipe.name!, ref: extractRef(pipe.ref as Reference), isExplicitlyDeferred: pipe.isExplicitlyDeferred, + deferredBlocks: pipe.deferredBlocks, }); } } diff --git a/packages/compiler-cli/src/ngtsc/typecheck/testing/index.ts b/packages/compiler-cli/src/ngtsc/typecheck/testing/index.ts index 8d160ccb735..c93bf0c4d46 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/testing/index.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/testing/index.ts @@ -22,6 +22,7 @@ import { R3TargetBinder, SelectorlessMatcher, SelectorMatcher, + TcbDirectiveMetadata, TcbGenericContextBehavior, TmplAstBoundAttribute, TmplAstBoundEvent, @@ -31,6 +32,7 @@ import { TmplAstHoverDeferredTrigger, TmplAstInteractionDeferredTrigger, TmplAstLetDeclaration, + TmplAstTemplate, TmplAstTextAttribute, TmplAstViewportDeferredTrigger, TypeCheckId, @@ -857,6 +859,7 @@ function prepareDeclarations( isStandalone: false, decorator: null, isExplicitlyDeferred: false, + deferredBlocks: null, isPure: true, }); } @@ -940,6 +943,7 @@ function getDirectiveMetaFromDeclaration( ngContentSelectors: decl.ngContentSelectors || null, preserveWhitespaces: decl.preserveWhitespaces ?? false, isExplicitlyDeferred: false, + deferredBlocks: null, imports: decl.imports, rawImports: null, matchSource: MatchSource.Selector, @@ -999,12 +1003,14 @@ function makeScope(program: ts.Program, sf: ts.SourceFile, decls: TestDeclaratio foreignImports: null, rawImports: null, deferredImports: null, + deferredImportsByBlock: null, schemas: null, decorator: null, assumedToExportProviders: false, ngContentSelectors: decl.ngContentSelectors || null, preserveWhitespaces: decl.preserveWhitespaces ?? false, isExplicitlyDeferred: false, + deferredBlocks: null, inputFieldNamesFromMetadataArray: null, selectorlessEnabled: false, localReferencedSymbols: null, @@ -1037,6 +1043,7 @@ function makeScope(program: ts.Program, sf: ts.SourceFile, decls: TestDeclaratio isStandalone: false, decorator: null, isExplicitlyDeferred: false, + deferredBlocks: null, isPure: true, }); } @@ -1074,8 +1081,19 @@ export class NoopOobRecorder implements OutOfBandDiagnosticRecorder { expect(diags.length).toBe(1); expect(diags[0].code).toBe(ngErrorCode(ErrorCode.DEFERRED_DIRECTIVE_USED_EAGERLY)); + expect(diags[0].messageText).toContain( + "Component 'DeferredCmpA' (used as element 'deferred-cmp-a') was imported via `@Component.deferredImports`, but was used outside of a `@defer` block in a template", + ); }); it('should produce an error the same component is referenced in both `deferredImports` and `imports`', () => { @@ -2103,5 +2106,447 @@ runInEachFileSystem(() => { ); }); }); + + describe('block-specific deferredImports type checking', () => { + it('should pass type-checking when a deferred component is used in its declared block', () => { + env.write( + 'cmp-a.ts', + ` + import { Component } from '@angular/core'; + @Component({ selector: 'cmp-a', template: 'CmpA!' }) + export class CmpA {} + `, + ); + + env.write( + '/test.ts', + ` + import { Component } from '@angular/core'; + import { CmpA } from './cmp-a'; + + @Component({ + selector: 'test-cmp', + // @ts-ignore + deferredImports: { + blockA: [CmpA], + }, + template: \` + @defer (name blockA) { + + } + \`, + }) + export class TestCmp {} + `, + ); + + const diags = env.driveDiagnostics(); + expect(diags.length).toBe(0); + }); + + it('should report an error when a deferred component is used in a defer block that does not include it', () => { + env.write( + 'cmps.ts', + ` + import { Component } from '@angular/core'; + @Component({ selector: 'cmp-a', template: 'CmpA!' }) + export class CmpA {} + + @Component({ selector: 'cmp-b', template: 'CmpB!' }) + export class CmpB {} + `, + ); + + env.write( + '/test.ts', + ` + import { Component } from '@angular/core'; + import { CmpA, CmpB } from './cmps'; + + @Component({ + selector: 'test-cmp', + // @ts-ignore + deferredImports: { + blockA: [CmpA], + blockB: [CmpB], + }, + template: \` + @defer (name blockA) { + + } + \`, + }) + export class TestCmp {} + `, + ); + + const diags = env.driveDiagnostics(); + expect(diags.length).toBe(1); + expect(diags[0].messageText).toContain( + "Component 'CmpB' (used as element 'cmp-b') was imported via `@Component.deferredImports` under block 'blockB', but is used in a `@defer` block configured for 'blockA'", + ); + }); + + it('should allow nested defer blocks to inherit dependencies from parent defer blocks', () => { + env.write( + 'cmps.ts', + ` + import { Component } from '@angular/core'; + @Component({ selector: 'outer-cmp', template: 'Outer!' }) + export class OuterCmp {} + + @Component({ selector: 'inner-cmp', template: 'Inner!' }) + export class InnerCmp {} + `, + ); + + env.write( + '/test.ts', + ` + import { Component } from '@angular/core'; + import { OuterCmp, InnerCmp } from './cmps'; + + @Component({ + selector: 'test-cmp', + // @ts-ignore + deferredImports: { + outerBlock: [OuterCmp], + innerBlock: [InnerCmp], + }, + template: \` + @defer (name outerBlock) { + + @defer (name innerBlock) { + + + } + } + \`, + }) + export class TestCmp {} + `, + ); + + const diags = env.driveDiagnostics(); + expect(diags.length).toBe(0); + }); + + it('should report an error in nested defer block when using a component from an unassociated block', () => { + env.write( + 'cmps.ts', + ` + import { Component } from '@angular/core'; + @Component({ selector: 'outer-cmp', template: 'Outer!' }) + export class OuterCmp {} + + @Component({ selector: 'inner-cmp', template: 'Inner!' }) + export class InnerCmp {} + + @Component({ selector: 'other-cmp', template: 'Other!' }) + export class OtherCmp {} + `, + ); + + env.write( + '/test.ts', + ` + import { Component } from '@angular/core'; + import { OuterCmp, InnerCmp, OtherCmp } from './cmps'; + + @Component({ + selector: 'test-cmp', + // @ts-ignore + deferredImports: { + outerBlock: [OuterCmp], + innerBlock: [InnerCmp], + otherBlock: [OtherCmp], + }, + template: \` + @defer (name outerBlock) { + @defer (name innerBlock) { + + } + } + \`, + }) + export class TestCmp {} + `, + ); + + const diags = env.driveDiagnostics(); + expect(diags.length).toBe(1); + expect(diags[0].messageText).toContain( + "Component 'OtherCmp' (used as element 'other-cmp') was imported via `@Component.deferredImports` under block 'otherBlock', but is used in a `@defer` block configured for 'innerBlock'", + ); + }); + + it('should report an error when a deferred pipe is used in a defer block that does not include it', () => { + env.write( + 'pipe-b.ts', + ` + import { Pipe, PipeTransform } from '@angular/core'; + @Pipe({ name: 'pipeb' }) + export class PipeB implements PipeTransform { + transform(val: string) { return val; } + } + `, + ); + + env.write( + 'cmp-a.ts', + ` + import { Component } from '@angular/core'; + @Component({ selector: 'cmp-a', template: 'CmpA!' }) + export class CmpA {} + `, + ); + + env.write( + '/test.ts', + ` + import { Component } from '@angular/core'; + import { CmpA } from './cmp-a'; + import { PipeB } from './pipe-b'; + + @Component({ + selector: 'test-cmp', + // @ts-ignore + deferredImports: { + blockA: [CmpA], + blockB: [PipeB], + }, + template: \` + @defer (name blockA) { + + {{ 'hello' | pipeb }} + } + \`, + }) + export class TestCmp {} + `, + ); + + const diags = env.driveDiagnostics(); + expect(diags.length).toBe(1); + expect(diags[0].messageText).toContain( + "Pipe 'pipeb' was imported via `@Component.deferredImports` under block 'blockB', but is used in a `@defer` block configured for 'blockA'", + ); + }); + + it('should pass type-checking when multiple defer blocks use distinct pipes', () => { + env.write( + 'pipes.ts', + ` + import { Pipe, PipeTransform } from '@angular/core'; + @Pipe({ name: 'pipea' }) + export class PipeA implements PipeTransform { + transform(val: string) { return val; } + } + @Pipe({ name: 'pipeb' }) + export class PipeB implements PipeTransform { + transform(val: string) { return val; } + } + `, + ); + + env.write( + '/test.ts', + ` + import { Component } from '@angular/core'; + import { PipeA, PipeB } from './pipes'; + + @Component({ + selector: 'test-cmp', + // @ts-ignore + deferredImports: { + blockA: [PipeA], + blockB: [PipeB], + }, + template: \` + @defer (name blockA) { + {{ 'hello' | pipea }} + } + @defer (name blockB) { + {{ 'world' | pipeb }} + } + \`, + }) + export class TestCmp {} + `, + ); + + const diags = env.driveDiagnostics(); + expect(diags.length).toBe(0); + }); + + it('should report an error when a pipe declared in a keyed block is used in an unnamed defer block', () => { + env.write( + 'pipe-a.ts', + ` + import { Pipe, PipeTransform } from '@angular/core'; + @Pipe({ name: 'pipea' }) + export class PipeA implements PipeTransform { + transform(val: string) { return val; } + } + `, + ); + + env.write( + '/test.ts', + ` + import { Component } from '@angular/core'; + import { PipeA } from './pipe-a'; + + @Component({ + selector: 'test-cmp', + // @ts-ignore + deferredImports: { + blockA: [PipeA], + }, + template: \` + @defer { + {{ 'hello' | pipea }} + } + \`, + }) + export class TestCmp {} + `, + ); + + const diags = env.driveDiagnostics(); + expect(diags.length).toBe(1); + expect(diags[0].messageText).toContain(`@defer block must specify a 'name' parameter`); + }); + + it('should report an error when a structural directive from deferredImports is used in an unassociated defer block', () => { + env.write( + 'dirs.ts', + ` + import { Directive, TemplateRef, ViewContainerRef } from '@angular/core'; + @Directive({ selector: '[dirA]' }) + export class DirA { + constructor(tr: TemplateRef, vcr: ViewContainerRef) {} + } + @Directive({ selector: '[dirB]' }) + export class DirB { + constructor(tr: TemplateRef, vcr: ViewContainerRef) {} + } + `, + ); + + env.write( + '/test.ts', + ` + import { Component } from '@angular/core'; + import { DirA, DirB } from './dirs'; + + @Component({ + selector: 'test-cmp', + // @ts-ignore + deferredImports: { + blockA: [DirA], + blockB: [DirB], + }, + template: \` + @defer (name blockA) { +
+ } + \`, + }) + export class TestCmp {} + `, + ); + + const diags = env.driveDiagnostics(); + expect(diags.length).toBe(1); + expect(diags[0].messageText).toContain( + "Directive 'DirB' (used on element 'div') was imported via `@Component.deferredImports` under block 'blockB', but is used in a `@defer` block configured for 'blockA'", + ); + }); + + it('should pass type-checking when a structural directive from deferredImports is used in its declared defer block', () => { + env.write( + 'dir-a.ts', + ` + import { Directive, TemplateRef, ViewContainerRef } from '@angular/core'; + @Directive({ selector: '[dirA]' }) + export class DirA { + constructor(tr: TemplateRef, vcr: ViewContainerRef) {} + } + `, + ); + + env.write( + '/test.ts', + ` + import { Component } from '@angular/core'; + import { DirA } from './dir-a'; + + @Component({ + selector: 'test-cmp', + // @ts-ignore + deferredImports: { + blockA: [DirA], + }, + template: \` + @defer (name blockA) { +
+ } + \`, + }) + export class TestCmp {} + `, + ); + + const diags = env.driveDiagnostics(); + expect(diags.length).toBe(0); + }); + + it('should report separate diagnostics for multiple deferred directives on the same element', () => { + env.write( + 'dirs.ts', + ` + import { Directive } from '@angular/core'; + @Directive({ selector: '[dirA]' }) + export class DirA {} + @Directive({ selector: '[dirB]' }) + export class DirB {} + `, + ); + + env.write( + '/test.ts', + ` + import { Component } from '@angular/core'; + import { DirA, DirB } from './dirs'; + + @Component({ + selector: 'test-cmp', + // @ts-ignore + deferredImports: { + blockA: [DirA], + blockB: [DirB], + blockC: [], + }, + template: \` + @defer (name blockC) { +
+ } + \`, + }) + export class TestCmp {} + `, + ); + + const diags = env.driveDiagnostics(); + expect(diags.length).toBe(2); + expect(diags[0].messageText).toContain( + "Directive 'DirA' (used on element 'div') was imported via `@Component.deferredImports` under block 'blockA', but is used in a `@defer` block configured for 'blockC'", + ); + expect(diags[1].messageText).toContain( + "Directive 'DirB' (used on element 'div') was imported via `@Component.deferredImports` under block 'blockB', but is used in a `@defer` block configured for 'blockC'", + ); + }); + }); }); }); diff --git a/packages/compiler/src/render3/view/t2_api.ts b/packages/compiler/src/render3/view/t2_api.ts index 5b3cab360a1..d6006195f74 100644 --- a/packages/compiler/src/render3/view/t2_api.ts +++ b/packages/compiler/src/render3/view/t2_api.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.dev/license */ -import {AST} from '../../expression_parser/ast'; +import {AST, BindingPipe} from '../../expression_parser/ast'; import {ClassPropertyMapping} from '../../property_mapping'; import { BoundAttribute, @@ -51,9 +51,7 @@ export type ScopedNode = /** Possible values that a reference can be resolved to. */ export type ReferenceTarget = - | {directive: DirectiveT; node: Exclude} - | Element - | Template; + {directive: DirectiveT; node: Exclude} | Element | Template; /** Entity that is local to the template and defined within the template. */ export type TemplateEntity = Reference | Variable | LetDeclaration; @@ -318,7 +316,17 @@ export interface BoundTarget { /** * Whether a given node is located in a `@defer` block. */ - isDeferred(node: Element): boolean; + isDeferred(node: DirectiveOwner): boolean; + + /** + * Gets the list of `@defer` blocks enclosing a given element, ordered from outermost to innermost. + */ + getDeferBlocksOfNode(node: DirectiveOwner): DeferredBlock[]; + + /** + * Gets the list of `@defer` blocks enclosing a given pipe, ordered from outermost to innermost. + */ + getDeferBlocksOfPipe(ast: BindingPipe): DeferredBlock[]; /** * Checks whether a component/directive that was referenced directly in the template exists. diff --git a/packages/compiler/src/render3/view/t2_binder.ts b/packages/compiler/src/render3/view/t2_binder.ts index e71417427a1..91880c2c151 100644 --- a/packages/compiler/src/render3/view/t2_binder.ts +++ b/packages/compiler/src/render3/view/t2_binder.ts @@ -163,8 +163,7 @@ export function findMatchingDirectivesAndPipes(template: string, directiveSelect /** Object used to match template nodes to directives. */ export type DirectiveMatcher = - | SelectorMatcher - | SelectorlessMatcher; + SelectorMatcher | SelectorlessMatcher; /** * Processes `Target`s with a given set of directives and performs a binding operation, which @@ -199,6 +198,7 @@ export class R3TargetBinder implements TargetB const usedPipes = new Set(); const eagerPipes = new Set(); const deferBlocks: DeferBlockScopes = []; + const pipes = new Map(); const conflictingHostDirectiveBindings = new Map< DirectiveOwner, ConflictingHostDirectiveBinding[] @@ -241,6 +241,7 @@ export class R3TargetBinder implements TargetB usedPipes, eagerPipes, deferBlocks, + pipes, ); } @@ -257,6 +258,7 @@ export class R3TargetBinder implements TargetB usedPipes, eagerPipes, deferBlocks, + pipes, ); } @@ -275,6 +277,7 @@ export class R3TargetBinder implements TargetB usedPipes, eagerPipes, deferBlocks, + pipes, conflictingHostDirectiveBindings, ); } @@ -296,7 +299,7 @@ class Scope implements Visitor { /** * Set of element-like nodes that belong to this scope. */ - readonly elementLikeInScope = new Set(); + readonly elementLikeInScope = new Set(); /** * Child `Scope`s for immediately nested `ScopedNode`s. @@ -369,6 +372,7 @@ class Scope implements Visitor { } visitTemplate(template: Template) { + this.elementLikeInScope.add(template); template.directives.forEach((node) => node.visit(this)); // References on a are defined in the outer scope, so capture them before @@ -454,6 +458,7 @@ class Scope implements Visitor { } visitDirective(directive: Directive) { + this.elementLikeInScope.add(directive); directive.references.forEach((current) => this.visitReference(current)); } @@ -474,6 +479,21 @@ class Scope implements Visitor { this.elementLikeInScope.add(node); } + /** + * Returns all enclosing `DeferredBlock`s for this scope, ordered from outermost to innermost. + */ + getEnclosingDeferBlocks(): DeferredBlock[] { + const blocks: DeferredBlock[] = []; + let current: Scope | null = this; + while (current !== null) { + if (current.rootNode instanceof DeferredBlock) { + blocks.push(current.rootNode); + } + current = current.parentScope; + } + return blocks.reverse(); + } + private maybeDeclare(thing: TemplateEntity) { // Declare something with a name, as long as that name isn't taken. if (!this.namedEntities.has(thing.name)) { @@ -972,6 +992,7 @@ class TemplateBinder extends CombinedRecursiveAstVisitor { private scope: Scope, private rootNode: ScopedNode | null, private level: number, + private pipes: Map, ) { super(); } @@ -997,6 +1018,7 @@ class TemplateBinder extends CombinedRecursiveAstVisitor { usedPipes: Set, eagerPipes: Set, deferBlocks: DeferBlockScopes, + pipes: Map, ): void { const template = nodeOrNodes instanceof Template ? nodeOrNodes : null; // The top-level template has nesting level 0. @@ -1010,6 +1032,7 @@ class TemplateBinder extends CombinedRecursiveAstVisitor { scope, template, 0, + pipes, ); binder.ingest(nodeOrNodes); } @@ -1162,6 +1185,7 @@ class TemplateBinder extends CombinedRecursiveAstVisitor { if (!this.scope.isDeferred) { this.eagerPipes.add(ast.name); } + this.pipes.set(ast, this.scope.getEnclosingDeferBlocks()); return super.visitPipe(ast, context); } @@ -1190,6 +1214,7 @@ class TemplateBinder extends CombinedRecursiveAstVisitor { childScope, node, this.level + 1, + this.pipes, ); binder.ingest(node); } @@ -1237,6 +1262,7 @@ class R3BoundTarget implements BoundTarget, private eagerPipes: Set, rawDeferred: DeferBlockScopes, + private pipes: Map, private conflictingHostDirectiveBindings: Map< DirectiveOwner, ConflictingHostDirectiveBinding[] @@ -1368,7 +1394,12 @@ class R3BoundTarget implements BoundTarget 0; + } + + getDeferBlocksOfNode(node: DirectiveOwner): DeferredBlock[] { + const blocks: DeferredBlock[] = []; for (const block of this.deferredBlocks) { if (!this.deferredScopes.has(block)) { continue; @@ -1379,15 +1410,20 @@ class R3BoundTarget implements BoundTarget 0) { const current = stack.pop()!; - if (current.elementLikeInScope.has(element)) { - return true; + if (current.elementLikeInScope.has(node)) { + blocks.push(block); + break; } stack.push(...current.childScopes.values()); } } - return false; + return blocks; + } + + getDeferBlocksOfPipe(ast: BindingPipe): DeferredBlock[] { + return this.pipes.get(ast) ?? []; } referencedDirectiveExists(name: string): boolean { diff --git a/packages/compiler/src/typecheck/api.ts b/packages/compiler/src/typecheck/api.ts index 3c3e51f0e1c..17826870634 100644 --- a/packages/compiler/src/typecheck/api.ts +++ b/packages/compiler/src/typecheck/api.ts @@ -79,6 +79,7 @@ export interface TcbPipeMetadata { name: string; ref: TcbReferenceMetadata; isExplicitlyDeferred: boolean; + deferredBlocks?: Set | null; } /** @@ -109,6 +110,7 @@ export interface TcbDirectiveMetadata { isStructural: boolean; isStandalone: boolean; isExplicitlyDeferred: boolean; + deferredBlocks?: Set | null; preserveWhitespaces: boolean; exportAs: string[] | null; matchSource: MatchSource; diff --git a/packages/compiler/src/typecheck/oob.ts b/packages/compiler/src/typecheck/oob.ts index 39e743971f2..9687cc60053 100644 --- a/packages/compiler/src/typecheck/oob.ts +++ b/packages/compiler/src/typecheck/oob.ts @@ -74,7 +74,12 @@ export interface OutOfBandDiagnosticRecorder { * @param id the type-checking ID of the template which contains the unknown pipe. * @param ast the `BindingPipe` invocation of the pipe which could not be found. */ - deferredPipeUsedEagerly(id: TypeCheckId, ast: BindingPipe): void; + deferredPipeUsedEagerly( + id: TypeCheckId, + ast: BindingPipe, + currentBlockName: string | null, + declaredBlocks: string[] | null, + ): void; /** * Reports usage of a component/directive imported via `@Component.deferredImports` outside @@ -83,7 +88,13 @@ export interface OutOfBandDiagnosticRecorder { * @param id the type-checking ID of the template which contains the unknown pipe. * @param element the element which hosts a component that was defer-loaded. */ - deferredComponentUsedEagerly(id: TypeCheckId, element: Element): void; + deferredComponentUsedEagerly( + id: TypeCheckId, + element: Element | Template, + dirMeta: TcbDirectiveMetadata, + currentBlockName: string | null, + declaredBlocks: string[] | null, + ): void; /** * Reports a duplicate declaration of a template variable. diff --git a/packages/compiler/src/typecheck/ops/expression.ts b/packages/compiler/src/typecheck/ops/expression.ts index 9484cb1c45b..0c469f71ace 100644 --- a/packages/compiler/src/typecheck/ops/expression.ts +++ b/packages/compiler/src/typecheck/ops/expression.ts @@ -186,16 +186,33 @@ export class TcbExpressionTranslator { // Use an 'any' value to at least allow the rest of the expression to be checked. pipe = new TcbExpr('(0 as any)'); - } else if ( - pipeMeta.isExplicitlyDeferred && - this.tcb.boundTarget.getEagerlyUsedPipes().includes(ast.name) - ) { - // This pipe was defer-loaded (included into `@Component.deferredImports`), - // but was used outside of a `@defer` block, which is the error. - this.tcb.oobRecorder.deferredPipeUsedEagerly(this.tcb.id, ast); - - // Use an 'any' value to at least allow the rest of the expression to be checked. - pipe = new TcbExpr('(0 as any)'); + } else if (pipeMeta.isExplicitlyDeferred) { + const enclosingBlocks = this.tcb.boundTarget.getDeferBlocksOfPipe(ast); + const isDeferred = enclosingBlocks.length > 0; + if (!isDeferred) { + this.tcb.oobRecorder.deferredPipeUsedEagerly(this.tcb.id, ast, null, null); + pipe = new TcbExpr('(0 as any)'); + } else if (pipeMeta.deferredBlocks != null) { + const isAllowedInBlock = enclosingBlocks.some( + (b) => b.definedName !== null && pipeMeta.deferredBlocks!.has(b.definedName), + ); + if (!isAllowedInBlock) { + const currentBlockName = + enclosingBlocks[enclosingBlocks.length - 1].definedName ?? 'unnamed'; + const declaredBlocks = Array.from(pipeMeta.deferredBlocks); + this.tcb.oobRecorder.deferredPipeUsedEagerly( + this.tcb.id, + ast, + currentBlockName, + declaredBlocks, + ); + pipe = new TcbExpr('(0 as any)'); + } else { + pipe = this.tcb.env.pipeInst(pipeMeta); + } + } else { + pipe = this.tcb.env.pipeInst(pipeMeta); + } } else { // Use a variable declared as the pipe's type. pipe = this.tcb.env.pipeInst(pipeMeta); diff --git a/packages/compiler/src/typecheck/ops/scope.ts b/packages/compiler/src/typecheck/ops/scope.ts index 4f418e60132..1eb610a6fd9 100644 --- a/packages/compiler/src/typecheck/ops/scope.ts +++ b/packages/compiler/src/typecheck/ops/scope.ts @@ -579,13 +579,37 @@ export class Scope { this.reportConflictingBindings(node); - if (node instanceof Element) { - const isDeferred = this.tcb.boundTarget.isDeferred(node); - if (!isDeferred && directives.some((dirMeta) => dirMeta.isExplicitlyDeferred)) { - // This node has directives/components that were defer-loaded (included into - // `@Component.deferredImports`), but the node itself was used outside of a - // `@defer` block, which is the error. - this.tcb.oobRecorder.deferredComponentUsedEagerly(this.tcb.id, node); + if (node instanceof Element || node instanceof Template) { + const enclosingBlocks = this.tcb.boundTarget.getDeferBlocksOfNode(node); + const isDeferred = enclosingBlocks.length > 0; + + for (const dirMeta of directives) { + if (!dirMeta.isExplicitlyDeferred) { + continue; + } + + if (!isDeferred) { + // This node has directives/components that were defer-loaded (included into + // `@Component.deferredImports`), but the node itself was used outside of a + // `@defer` block, which is the error. + this.tcb.oobRecorder.deferredComponentUsedEagerly(this.tcb.id, node, dirMeta, null, null); + } else if (dirMeta.deferredBlocks != null) { + const isAllowedInBlock = enclosingBlocks.some( + (b) => b.definedName !== null && dirMeta.deferredBlocks!.has(b.definedName), + ); + if (!isAllowedInBlock) { + const currentBlockName = + enclosingBlocks[enclosingBlocks.length - 1].definedName ?? 'unnamed'; + const declaredBlocks = Array.from(dirMeta.deferredBlocks); + this.tcb.oobRecorder.deferredComponentUsedEagerly( + this.tcb.id, + node, + dirMeta, + currentBlockName, + declaredBlocks, + ); + } + } } } diff --git a/packages/compiler/test/render3/view/binding_spec.ts b/packages/compiler/test/render3/view/binding_spec.ts index 91105436eb5..dc2450e4811 100644 --- a/packages/compiler/test/render3/view/binding_spec.ts +++ b/packages/compiler/test/render3/view/binding_spec.ts @@ -669,6 +669,80 @@ describe('t2 binding', () => { expect(eagerDirs).toEqual([]); }); + it('should track enclosing defer blocks for pipes', () => { + const template = parseTemplate( + ` + {{ 'outside' | pipeA }} + @defer (name blockA) { + {{ 'in A' | pipeB }} + @defer (name blockB) { + {{ 'in B' | pipeC }} + } + } + `, + '', + ); + const binder = new R3TargetBinder(makeSelectorMatcher()); + const bound = binder.bind({template: template.nodes}); + const deferBlocks = bound.getDeferBlocks(); + expect(deferBlocks.length).toBe(2); + + const pipes: e.BindingPipe[] = []; + class AstVisitor extends e.RecursiveAstVisitor { + override visitPipe(ast: e.BindingPipe, context: any) { + pipes.push(ast); + super.visitPipe(ast, context); + } + } + const astVisitor = new AstVisitor(); + class TemplateVisitor extends a.RecursiveVisitor { + override visitBoundText(text: a.BoundText) { + text.value.visit(astVisitor); + } + } + a.visitAll(new TemplateVisitor(), template.nodes); + + const pipeA = pipes.find((p) => p.name === 'pipeA')!; + const pipeB = pipes.find((p) => p.name === 'pipeB')!; + const pipeC = pipes.find((p) => p.name === 'pipeC')!; + + expect(bound.getDeferBlocksOfPipe(pipeA)).toEqual([]); + expect(bound.getDeferBlocksOfPipe(pipeB)).toEqual([deferBlocks[0]]); + expect(bound.getDeferBlocksOfPipe(pipeC)).toEqual([deferBlocks[0], deferBlocks[1]]); + }); + + it('should track enclosing defer blocks for element and template nodes', () => { + const template = parseTemplate( + ` +
+ @defer (name blockA) { +
+
+ @defer (name blockB) { +
+ } + } + `, + '', + ); + const binder = new R3TargetBinder(makeSelectorMatcher()); + const bound = binder.bind({template: template.nodes}); + const deferBlocks = bound.getDeferBlocks(); + expect(deferBlocks.length).toBe(2); + + const divOutside = template.nodes[0] as a.Element; + const blockA = template.nodes[1] as a.DeferredBlock; + const divInsideA = blockA.children[0] as a.Element; + const templateInsideA = blockA.children[1] as a.Template; + const blockB = blockA.children[2] as a.DeferredBlock; + const divInsideB = blockB.children[0] as a.Element; + + expect(bound.getDeferBlocksOfNode(divOutside)).toEqual([]); + expect(bound.getDeferBlocksOfNode(divInsideA)).toEqual([deferBlocks[0]]); + expect(bound.getDeferBlocksOfNode(templateInsideA)).toEqual([deferBlocks[0]]); + expect(bound.getDeferBlocksOfNode(divInsideB)).toEqual([deferBlocks[0], deferBlocks[1]]); + }); + it('should identify a trigger element that is a parent of the deferred block', () => { const template = parseTemplate( ` diff --git a/packages/core/src/metadata/directives.ts b/packages/core/src/metadata/directives.ts index ab1f8cd50b9..da0ad2812ff 100644 --- a/packages/core/src/metadata/directives.ts +++ b/packages/core/src/metadata/directives.ts @@ -659,7 +659,8 @@ export interface Component extends Directive { * Note: this is an internal-only field, use regular `@Component.imports` field instead. * @internal // 3p-only */ - deferredImports?: (Type | ReadonlyArray)[]; + deferredImports?: + (Type | ReadonlyArray)[] | {[blockName: string]: (Type | ReadonlyArray)[]}; /** * The set of schemas that declare elements to be allowed in a standalone component. Elements and