diff --git a/packages/compiler-cli/src/ngtsc/annotations/src/component.ts b/packages/compiler-cli/src/ngtsc/annotations/src/component.ts index 07225e8546..24679125e3 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/src/component.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/src/component.ts @@ -510,16 +510,18 @@ export class ComponentDecoratorHandler implements return { selector: directive.selector, expression: this.refEmitter.emit(directive.ref, context), + ref: directive.ref, }; }); - const usedPipes: {pipeName: string, expression: Expression}[] = []; + const usedPipes: {ref: Reference, pipeName: string, expression: Expression}[] = []; for (const pipeName of bound.getUsedPipes()) { if (!pipes.has(pipeName)) { continue; } const pipe = pipes.get(pipeName)!; usedPipes.push({ + ref: pipe, pipeName, expression: this.refEmitter.emit(pipe, context), }); @@ -557,7 +559,8 @@ export class ComponentDecoratorHandler implements // Declaring the directiveDefs/pipeDefs arrays directly would require imports that would // create a cycle. Instead, mark this component as requiring remote scoping, so that the // NgModule file will take care of setting the directives for the component. - this.scopeRegistry.setComponentAsRequiringRemoteScoping(node); + this.scopeRegistry.setComponentRemoteScope( + node, usedDirectives.map(dir => dir.ref), usedPipes.map(pipe => pipe.ref)); } } diff --git a/packages/compiler-cli/src/ngtsc/annotations/src/ng_module.ts b/packages/compiler-cli/src/ngtsc/annotations/src/ng_module.ts index 37afa3b8fc..67ad6373dd 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/src/ng_module.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/src/ng_module.ts @@ -387,15 +387,11 @@ export class NgModuleDecoratorHandler implements } const context = getSourceFile(node); for (const decl of analysis.declarations) { - if (this.scopeRegistry.getRequiresRemoteScope(decl.node)) { - const scope = this.scopeRegistry.getScopeOfModule(ts.getOriginalNode(node) as typeof node); - if (scope === null || scope === 'error') { - continue; - } - - const directives = scope.compilation.directives.map( - directive => this.refEmitter.emit(directive.ref, context)); - const pipes = scope.compilation.pipes.map(pipe => this.refEmitter.emit(pipe.ref, context)); + const remoteScope = this.scopeRegistry.getRemoteScope(decl.node); + if (remoteScope !== null) { + const directives = + remoteScope.directives.map(directive => this.refEmitter.emit(directive, context)); + const pipes = remoteScope.pipes.map(pipe => this.refEmitter.emit(pipe, context)); const directiveArray = new LiteralArrayExpr(directives); const pipesArray = new LiteralArrayExpr(pipes); const declExpr = this.refEmitter.emit(decl, context)!; diff --git a/packages/compiler-cli/src/ngtsc/scope/src/api.ts b/packages/compiler-cli/src/ngtsc/scope/src/api.ts index 73c59fd9a1..e293acda1c 100644 --- a/packages/compiler-cli/src/ngtsc/scope/src/api.ts +++ b/packages/compiler-cli/src/ngtsc/scope/src/api.ts @@ -6,6 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ +import {Reference} from '../../imports'; import {DirectiveMeta, PipeMeta} from '../../metadata'; import {ClassDeclaration} from '../../reflection'; @@ -40,3 +41,19 @@ export interface ExportScope { */ exported: ScopeData; } + +/** + * A resolved scope for a given component that cannot be set locally in the component definition, + * and must be set via remote scoping call in the component's NgModule file. + */ +export interface RemoteScope { + /** + * Those directives used by the component that requires this scope to be set remotely. + */ + directives: Reference[]; + + /** + * Those pipes used by the component that requires this scope to be set remotely. + */ + pipes: Reference[]; +} \ No newline at end of file diff --git a/packages/compiler-cli/src/ngtsc/scope/src/component_scope.ts b/packages/compiler-cli/src/ngtsc/scope/src/component_scope.ts index 7daef581f8..0f4d63913f 100644 --- a/packages/compiler-cli/src/ngtsc/scope/src/component_scope.ts +++ b/packages/compiler-cli/src/ngtsc/scope/src/component_scope.ts @@ -6,6 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ import {ClassDeclaration} from '../../reflection'; +import {RemoteScope} from './api'; import {LocalModuleScope} from './local'; /** @@ -13,7 +14,14 @@ import {LocalModuleScope} from './local'; */ export interface ComponentScopeReader { getScopeForComponent(clazz: ClassDeclaration): LocalModuleScope|null|'error'; - getRequiresRemoteScope(clazz: ClassDeclaration): boolean|null; + + /** + * Get the `RemoteScope` required for this component, if any. + * + * If the component requires remote scoping, then retrieve the directives/pipes registered for + * that component. If remote scoping is not required (the common case), returns `null`. + */ + getRemoteScope(clazz: ClassDeclaration): RemoteScope|null; } /** @@ -36,11 +44,11 @@ export class CompoundComponentScopeReader implements ComponentScopeReader { return null; } - getRequiresRemoteScope(clazz: ClassDeclaration): boolean|null { + getRemoteScope(clazz: ClassDeclaration): RemoteScope|null { for (const reader of this.readers) { - const requiredScoping = reader.getRequiresRemoteScope(clazz); - if (requiredScoping !== null) { - return requiredScoping; + const remoteScope = reader.getRemoteScope(clazz); + if (remoteScope !== null) { + return remoteScope; } } return null; diff --git a/packages/compiler-cli/src/ngtsc/scope/src/local.ts b/packages/compiler-cli/src/ngtsc/scope/src/local.ts index 42990227ab..30c3a1ce26 100644 --- a/packages/compiler-cli/src/ngtsc/scope/src/local.ts +++ b/packages/compiler-cli/src/ngtsc/scope/src/local.ts @@ -15,7 +15,7 @@ import {DirectiveMeta, MetadataReader, MetadataRegistry, NgModuleMeta, PipeMeta} import {ClassDeclaration, DeclarationNode} from '../../reflection'; import {identifierOfNode, nodeNameForError} from '../../util/src/typescript'; -import {ExportScope, ScopeData} from './api'; +import {ExportScope, RemoteScope, ScopeData} from './api'; import {ComponentScopeReader} from './component_scope'; import {DtsModuleScopeResolver} from './dependency'; @@ -96,14 +96,14 @@ export class LocalModuleScopeRegistry implements MetadataRegistry, ComponentScop private cache = new Map(); /** - * Tracks whether a given component requires "remote scoping". + * Tracks the `RemoteScope` for components requiring "remote scoping". * * Remote scoping is when the set of directives which apply to a given component is set in the * NgModule's file instead of directly on the component def (which is sometimes needed to get * around cyclic import issues). This is not used in calculation of `LocalModuleScope`s, but is * tracked here for convenience. */ - private remoteScoping = new Set(); + private remoteScoping = new Map(); /** * Tracks errors accumulated in the processing of scopes for each module declaration. @@ -452,15 +452,17 @@ export class LocalModuleScopeRegistry implements MetadataRegistry, ComponentScop /** * Check whether a component requires remote scoping. */ - getRequiresRemoteScope(node: ClassDeclaration): boolean { - return this.remoteScoping.has(node); + getRemoteScope(node: ClassDeclaration): RemoteScope|null { + return this.remoteScoping.has(node) ? this.remoteScoping.get(node)! : null; } /** - * Set a component as requiring remote scoping. + * Set a component as requiring remote scoping, with the given directives and pipes to be + * registered remotely. */ - setComponentAsRequiringRemoteScoping(node: ClassDeclaration): void { - this.remoteScoping.add(node); + setComponentRemoteScope(node: ClassDeclaration, directives: Reference[], pipes: Reference[]): + void { + this.remoteScoping.set(node, {directives, pipes}); } /** diff --git a/packages/compiler-cli/src/ngtsc/typecheck/test/test_utils.ts b/packages/compiler-cli/src/ngtsc/typecheck/test/test_utils.ts index 096875c852..07bd93cb96 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/test/test_utils.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/test/test_utils.ts @@ -418,7 +418,7 @@ export function setup(targets: TypeCheckingTarget[], overrides: { } const fakeScopeReader: ComponentScopeReader = { - getRequiresRemoteScope() { + getRemoteScope(): null { return null; }, // If there is a module with [className] + 'Module' in the same source file, that will be diff --git a/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts b/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts index 2642e233be..cbf673384f 100644 --- a/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts +++ b/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts @@ -4597,7 +4597,7 @@ runInEachFileSystem(os => { const jsContents = env.getContents('test.js'); expect(jsContents) .toMatch( - /i\d\.ɵɵsetComponentScope\(NormalComponent,\s+\[NormalComponent,\s+CyclicComponent\],\s+\[\]\)/); + /i\d\.ɵɵsetComponentScope\(NormalComponent,\s+\[CyclicComponent\],\s+\[\]\)/); expect(jsContents).not.toContain('/*__PURE__*/ i0.ɵɵsetComponentScope'); }); @@ -4666,6 +4666,49 @@ runInEachFileSystem(os => { const jsContents = env.getContents('test.js'); expect(jsContents).not.toContain('setComponentScope'); }); + + it('should only pass components actually used to setComponentScope', () => { + env.write('test.ts', ` + import {Component, NgModule} from '@angular/core'; + import {NormalComponent} from './cyclic'; + import {OtherComponent} from './other'; + + @Component({ + selector: 'cyclic-component', + template: 'Importing this causes a cycle', + }) + export class CyclicComponent {} + + @NgModule({ + declarations: [NormalComponent, CyclicComponent, OtherComponent], + }) + export class Module {} + `); + + env.write('cyclic.ts', ` + import {Component} from '@angular/core'; + + @Component({ + selector: 'normal-component', + template: '', + }) + export class NormalComponent {} + `); + + env.write('other.ts', ` + import {Component} from '@angular/core'; + + @Component({ + selector: 'other-component', + template: 'An unused other component', + }) + export class OtherComponent {} + `); + + env.driveMain(); + const jsContents = env.getContents('test.js'); + expect(jsContents).not.toMatch(/i\d\.ɵɵsetComponentScope\([^)]*OtherComponent[^)]*\)/); + }); }); describe('local refs', () => {