Skip to content

Commit 9b35787

Browse files
alxhubalxhub
committed
refactor(compiler-cli): unify tracked template scope dependencies (#45672)
Previously, the compiler tracked directives and pipes in template scopes separately. This commit refactors the scope system to unify them into a single data structure, disambiguated by a `kind` field. PR Close #45672
1 parent 1527e8f commit 9b35787

File tree

23 files changed

+227
-233
lines changed

23 files changed

+227
-233
lines changed

‎packages/compiler-cli/src/ngtsc/annotations/component/src/handler.ts‎

Lines changed: 94 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ import {assertSuccessfulReferenceEmit, ImportedFile, ModuleResolver, Reference,
1616
import {DependencyTracker} from '../../../incremental/api';
1717
import {extractSemanticTypeParameters, SemanticDepGraphUpdater} from '../../../incremental/semantic_graph';
1818
import {IndexingContext} from '../../../indexer';
19-
import {DirectiveMeta, extractDirectiveTypeCheckMeta, InjectableClassRegistry, MetadataReader, MetadataRegistry, MetaType, ResourceRegistry} from '../../../metadata';
19+
import {DirectiveMeta, extractDirectiveTypeCheckMeta, InjectableClassRegistry, MetadataReader, MetadataRegistry, MetaKind, PipeMeta, ResourceRegistry} from '../../../metadata';
2020
import {PartialEvaluator} from '../../../partial_evaluator';
2121
import {PerfEvent, PerfRecorder} from '../../../perf';
2222
import {ClassDeclaration, DeclarationNode, Decorator, ReflectionHost, reflectObjectLiteral} from '../../../reflection';
@@ -446,7 +446,7 @@ export class ComponentDecoratorHandler implements
446446
// the information about the component is available during the compile() phase.
447447
const ref = new Reference(node);
448448
this.metaRegistry.registerDirectiveMetadata({
449-
type: MetaType.Directive,
449+
kind: MetaKind.Directive,
450450
ref,
451451
name: node.name.text,
452452
selector: analysis.meta.selector,
@@ -482,9 +482,9 @@ export class ComponentDecoratorHandler implements
482482
return null;
483483
}
484484

485-
for (const directive of scope.compilation.directives) {
486-
if (directive.selector !== null) {
487-
matcher.addSelectables(CssSelector.parse(directive.selector), directive);
485+
for (const dep of scope.compilation.dependencies) {
486+
if (dep.kind === MetaKind.Directive && dep.selector !== null) {
487+
matcher.addSelectables(CssSelector.parse(dep.selector), dep);
488488
}
489489
}
490490
}
@@ -584,15 +584,15 @@ export class ComponentDecoratorHandler implements
584584
type MatchedDirective = DirectiveMeta&{selector: string};
585585
const matcher = new SelectorMatcher<MatchedDirective>();
586586

587-
for (const dir of scope.directives) {
588-
if (dir.selector !== null) {
589-
matcher.addSelectables(CssSelector.parse(dir.selector), dir as MatchedDirective);
587+
const pipes = new Map<string, PipeMeta>();
588+
589+
for (const dep of scope.dependencies) {
590+
if (dep.kind === MetaKind.Directive && dep.selector !== null) {
591+
matcher.addSelectables(CssSelector.parse(dep.selector), dep as MatchedDirective);
592+
} else if (dep.kind === MetaKind.Pipe) {
593+
pipes.set(dep.name, dep);
590594
}
591595
}
592-
const pipes = new Map<string, Reference<ClassDeclaration>>();
593-
for (const pipe of scope.pipes) {
594-
pipes.set(pipe.name, pipe.ref);
595-
}
596596

597597
// Next, the component template AST is bound using the R3TargetBinder. This produces a
598598
// BoundTarget, which is similar to a ts.TypeChecker.
@@ -602,91 +602,113 @@ export class ComponentDecoratorHandler implements
602602
// The BoundTarget knows which directives and pipes matched the template.
603603
type UsedDirective = R3DirectiveDependencyMetadata&
604604
{ref: Reference<ClassDeclaration>, importedFile: ImportedFile};
605-
const usedDirectives: UsedDirective[] = bound.getUsedDirectives().map(directive => {
606-
const type = this.refEmitter.emit(directive.ref, context);
607-
assertSuccessfulReferenceEmit(
608-
type, node.name, directive.isComponent ? 'component' : 'directive');
609-
return {
610-
kind: R3TemplateDependencyKind.Directive,
611-
ref: directive.ref,
612-
type: type.expression,
613-
importedFile: type.importedFile,
614-
selector: directive.selector,
615-
inputs: directive.inputs.propertyNames,
616-
outputs: directive.outputs.propertyNames,
617-
exportAs: directive.exportAs,
618-
isComponent: directive.isComponent,
619-
};
620-
});
605+
606+
const used = new Set<ClassDeclaration>();
607+
for (const dir of bound.getUsedDirectives()) {
608+
used.add(dir.ref.node);
609+
}
610+
for (const name of bound.getUsedPipes()) {
611+
if (!pipes.has(name)) {
612+
continue;
613+
}
614+
used.add(pipes.get(name)!.ref.node);
615+
}
621616

622617
type UsedPipe = R3PipeDependencyMetadata&{
623618
ref: Reference<ClassDeclaration>,
624619
importedFile: ImportedFile,
625620
};
626-
const usedPipes: UsedPipe[] = [];
627-
for (const pipeName of bound.getUsedPipes()) {
628-
if (!pipes.has(pipeName)) {
629-
continue;
621+
622+
const declarations: (UsedPipe|UsedDirective)[] = [];
623+
624+
// Transform the dependencies list, filtering out unused dependencies.
625+
for (const dep of scope.dependencies) {
626+
switch (dep.kind) {
627+
case MetaKind.Directive:
628+
if (!used.has(dep.ref.node)) {
629+
continue;
630+
}
631+
const dirType = this.refEmitter.emit(dep.ref, context);
632+
assertSuccessfulReferenceEmit(
633+
dirType, node.name, dep.isComponent ? 'component' : 'directive');
634+
635+
declarations.push({
636+
kind: R3TemplateDependencyKind.Directive,
637+
ref: dep.ref,
638+
type: dirType.expression,
639+
importedFile: dirType.importedFile,
640+
selector: dep.selector!,
641+
inputs: dep.inputs.propertyNames,
642+
outputs: dep.outputs.propertyNames,
643+
exportAs: dep.exportAs,
644+
isComponent: dep.isComponent,
645+
});
646+
break;
647+
case MetaKind.Pipe:
648+
if (!used.has(dep.ref.node)) {
649+
continue;
650+
}
651+
652+
const pipeType = this.refEmitter.emit(dep.ref, context);
653+
assertSuccessfulReferenceEmit(pipeType, node.name, 'pipe');
654+
655+
declarations.push({
656+
kind: R3TemplateDependencyKind.Pipe,
657+
type: pipeType.expression,
658+
name: dep.name,
659+
ref: dep.ref,
660+
importedFile: pipeType.importedFile,
661+
});
662+
break;
630663
}
631-
const pipe = pipes.get(pipeName)!;
632-
const type = this.refEmitter.emit(pipe, context);
633-
assertSuccessfulReferenceEmit(type, node.name, 'pipe');
634-
usedPipes.push({
635-
kind: R3TemplateDependencyKind.Pipe,
636-
type: type.expression,
637-
name: pipeName,
638-
ref: pipe,
639-
importedFile: type.importedFile,
640-
});
641664
}
665+
666+
const isUsedDirective = (decl: UsedDirective|UsedPipe): decl is UsedDirective =>
667+
decl.kind === R3TemplateDependencyKind.Directive;
668+
const isUsedPipe = (decl: UsedDirective|UsedPipe): decl is UsedPipe =>
669+
decl.kind === R3TemplateDependencyKind.Pipe;
670+
671+
const getSemanticReference = (decl: UsedDirective|UsedPipe) =>
672+
this.semanticDepGraphUpdater!.getSemanticReference(decl.ref.node, decl.type);
673+
642674
if (this.semanticDepGraphUpdater !== null) {
643-
symbol.usedDirectives = usedDirectives.map(
644-
dir => this.semanticDepGraphUpdater!.getSemanticReference(dir.ref.node, dir.type));
645-
symbol.usedPipes = usedPipes.map(
646-
pipe => this.semanticDepGraphUpdater!.getSemanticReference(pipe.ref.node, pipe.type));
675+
symbol.usedDirectives = declarations.filter(isUsedDirective).map(getSemanticReference);
676+
symbol.usedPipes = declarations.filter(isUsedPipe).map(getSemanticReference);
647677
}
648678

649679
// Scan through the directives/pipes actually used in the template and check whether any
650680
// import which needs to be generated would create a cycle.
651681
const cyclesFromDirectives = new Map<UsedDirective, Cycle>();
652-
for (const usedDirective of usedDirectives) {
653-
const cycle =
654-
this._checkForCyclicImport(usedDirective.importedFile, usedDirective.type, context);
655-
if (cycle !== null) {
656-
cyclesFromDirectives.set(usedDirective, cycle);
657-
}
658-
}
659682
const cyclesFromPipes = new Map<UsedPipe, Cycle>();
660-
for (const usedPipe of usedPipes) {
661-
const cycle = this._checkForCyclicImport(usedPipe.importedFile, usedPipe.type, context);
683+
for (const usedDep of declarations) {
684+
const cycle = this._checkForCyclicImport(usedDep.importedFile, usedDep.type, context);
662685
if (cycle !== null) {
663-
cyclesFromPipes.set(usedPipe, cycle);
686+
switch (usedDep.kind) {
687+
case R3TemplateDependencyKind.Directive:
688+
cyclesFromDirectives.set(usedDep, cycle);
689+
break;
690+
case R3TemplateDependencyKind.Pipe:
691+
cyclesFromPipes.set(usedDep, cycle);
692+
break;
693+
}
664694
}
665695
}
666696

667697
const cycleDetected = cyclesFromDirectives.size !== 0 || cyclesFromPipes.size !== 0;
668698
if (!cycleDetected) {
669699
// No cycle was detected. Record the imports that need to be created in the cycle detector
670700
// so that future cyclic import checks consider their production.
671-
for (const {type, importedFile} of usedDirectives) {
672-
this._recordSyntheticImport(importedFile, type, context);
673-
}
674-
for (const {type, importedFile} of usedPipes) {
701+
for (const {type, importedFile} of declarations) {
675702
this._recordSyntheticImport(importedFile, type, context);
676703
}
677704

678-
// Check whether the directive/pipe arrays in ɵcmp need to be wrapped in closures.
679-
// This is required if any directive/pipe reference is to a declaration in the same file
705+
// Check whether the dependencies arrays in ɵcmp need to be wrapped in a closure.
706+
// This is required if any dependency reference is to a declaration in the same file
680707
// but declared after this component.
681708
const wrapDirectivesAndPipesInClosure =
682-
usedDirectives.some(
683-
dir => isExpressionForwardReference(dir.type, node.name, context)) ||
684-
usedPipes.some(pipe => isExpressionForwardReference(pipe.type, node.name, context));
685-
686-
data.declarations = [
687-
...usedDirectives,
688-
...usedPipes,
689-
];
709+
declarations.some(decl => isExpressionForwardReference(decl.type, node.name, context));
710+
711+
data.declarations = declarations;
690712
data.declarationListEmitMode = wrapDirectivesAndPipesInClosure ?
691713
DeclarationListEmitMode.Closure :
692714
DeclarationListEmitMode.Direct;
@@ -696,7 +718,8 @@ export class ComponentDecoratorHandler implements
696718
// create a cycle. Instead, mark this component as requiring remote scoping, so that the
697719
// NgModule file will take care of setting the directives for the component.
698720
this.scopeRegistry.setComponentRemoteScope(
699-
node, usedDirectives.map(dir => dir.ref), usedPipes.map(pipe => pipe.ref));
721+
node, declarations.filter(isUsedDirective).map(dir => dir.ref),
722+
declarations.filter(isUsedPipe).map(pipe => pipe.ref));
700723
symbol.isRemotelyScoped = true;
701724

702725
// If a semantic graph is being tracked, record the fact that this component is remotely

‎packages/compiler-cli/src/ngtsc/annotations/component/src/scope.ts‎

Lines changed: 19 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,10 @@ import {ComponentScopeReader, DtsModuleScopeResolver, ExportScope, LocalModuleSc
1515
import {ComponentAnalysisData} from './metadata';
1616

1717

18+
export type DependencyMeta = DirectiveMeta|PipeMeta;
19+
1820
export interface ScopeTemplateResult {
19-
directives: DirectiveMeta[];
20-
pipes: PipeMeta[];
21+
dependencies: DependencyMeta[];
2122
diagnostics: ts.Diagnostic[];
2223
ngModule: ClassDeclaration|null;
2324
}
@@ -43,8 +44,7 @@ export function scopeTemplate(
4344
if (scope !== null) {
4445
// This is an NgModule-ful component, so use scope information coming from the NgModule.
4546
return {
46-
directives: scope.compilation.directives,
47-
pipes: scope.compilation.pipes,
47+
dependencies: scope.compilation.dependencies,
4848
ngModule: scope.ngModule,
4949
diagnostics: [],
5050
};
@@ -53,8 +53,7 @@ export function scopeTemplate(
5353
if (analysis.imports === null) {
5454
// Early exit for standalone components that don't declare imports (empty scope).
5555
return {
56-
directives: [],
57-
pipes: [],
56+
dependencies: [],
5857
ngModule: null,
5958
diagnostics: [],
6059
};
@@ -64,10 +63,16 @@ export function scopeTemplate(
6463

6564
// We need to deduplicate directives/pipes in `imports`, as any given directive/pipe may be
6665
// present in the export scope of more than one NgModule (or just listed more than once).
67-
const directives = new Map<ClassDeclaration, DirectiveMeta>();
68-
const pipes = new Map<ClassDeclaration, PipeMeta>();
66+
const seen = new Set<ClassDeclaration>();
67+
const dependencies: DependencyMeta[] = [];
6968

7069
for (const ref of analysis.imports.resolved) {
70+
if (seen.has(ref.node)) {
71+
// This imported type has already been processed, so don't process it a second time.
72+
continue;
73+
}
74+
seen.add(ref.node);
75+
7176
// Determine if this import is a component/directive/pipe/NgModule.
7277
const dirMeta = metaReader.getDirectiveMetadata(ref);
7378
if (dirMeta !== null) {
@@ -79,9 +84,7 @@ export function scopeTemplate(
7984
continue;
8085
}
8186

82-
if (!directives.has(ref.node)) {
83-
directives.set(ref.node, dirMeta);
84-
}
87+
dependencies.push(dirMeta);
8588
continue;
8689
}
8790

@@ -114,33 +117,21 @@ export function scopeTemplate(
114117
return null;
115118
}
116119

117-
for (const dir of scope.exported.directives) {
118-
if (!directives.has(dir.ref.node)) {
119-
directives.set(dir.ref.node, dir);
120-
}
121-
}
122-
123-
for (const pipe of scope.exported.pipes) {
124-
if (!pipes.has(pipe.ref.node)) {
125-
pipes.set(pipe.ref.node, pipe);
126-
}
127-
}
120+
dependencies.push(...scope.exported.dependencies);
128121
}
129122
}
130123

131124
return {
132-
directives: Array.from(directives.values()),
133-
pipes: Array.from(pipes.values()),
134-
diagnostics,
125+
dependencies,
135126
ngModule: null,
127+
diagnostics,
136128
};
137129
} else {
138130
// This is a "free" component, and is neither standalone nor declared in an NgModule.
139131
// This should probably be an error now that we have standalone components, but that would be a
140-
// breaking change. For now, preserve the old behavior (treat is as having an empty scope).
132+
// breaking change. For now, preserve the old behavior (treat it as having an empty scope).
141133
return {
142-
directives: [],
143-
pipes: [],
134+
dependencies: [],
144135
ngModule: null,
145136
diagnostics: [],
146137
};

‎packages/compiler-cli/src/ngtsc/annotations/directive/src/handler.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import ts from 'typescript';
1111

1212
import {Reference} from '../../../imports';
1313
import {extractSemanticTypeParameters, SemanticDepGraphUpdater} from '../../../incremental/semantic_graph';
14-
import {ClassPropertyMapping, DirectiveTypeCheckMeta, extractDirectiveTypeCheckMeta, InjectableClassRegistry, MetadataReader, MetadataRegistry, MetaType} from '../../../metadata';
14+
import {ClassPropertyMapping, DirectiveTypeCheckMeta, extractDirectiveTypeCheckMeta, InjectableClassRegistry, MetadataReader, MetadataRegistry, MetaKind} from '../../../metadata';
1515
import {PartialEvaluator} from '../../../partial_evaluator';
1616
import {PerfEvent, PerfRecorder} from '../../../perf';
1717
import {ClassDeclaration, ClassMember, ClassMemberKind, Decorator, ReflectionHost} from '../../../reflection';
@@ -132,7 +132,7 @@ export class DirectiveDecoratorHandler implements
132132
// the information about the directive is available during the compile() phase.
133133
const ref = new Reference(node);
134134
this.metaRegistry.registerDirectiveMetadata({
135-
type: MetaType.Directive,
135+
kind: MetaKind.Directive,
136136
ref,
137137
name: node.name.text,
138138
selector: analysis.meta.selector,

‎packages/compiler-cli/src/ngtsc/annotations/ng_module/src/handler.ts‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import ts from 'typescript';
1212
import {ErrorCode, FatalDiagnosticError, makeDiagnostic, makeRelatedInformation} from '../../../diagnostics';
1313
import {assertSuccessfulReferenceEmit, Reference, ReferenceEmitter} from '../../../imports';
1414
import {isArrayEqual, isReferenceEqual, isSymbolEqual, SemanticReference, SemanticSymbol} from '../../../incremental/semantic_graph';
15-
import {InjectableClassRegistry, MetadataReader, MetadataRegistry} from '../../../metadata';
15+
import {InjectableClassRegistry, MetadataReader, MetadataRegistry, MetaKind} from '../../../metadata';
1616
import {PartialEvaluator, ResolvedValue} from '../../../partial_evaluator';
1717
import {PerfEvent, PerfRecorder} from '../../../perf';
1818
import {ClassDeclaration, Decorator, isNamedClassDeclaration, ReflectionHost, reflectObjectLiteral, typeNodeToValueExpr} from '../../../reflection';
@@ -738,8 +738,7 @@ export class NgModuleDecoratorHandler implements
738738
}
739739

740740
function isNgModule(node: ClassDeclaration, compilation: ScopeData): boolean {
741-
return !compilation.directives.some(directive => directive.ref.node === node) &&
742-
!compilation.pipes.some(pipe => pipe.ref.node === node);
741+
return !compilation.dependencies.some(dep => dep.ref.node === node);
743742
}
744743

745744
/**

‎packages/compiler-cli/src/ngtsc/annotations/src/pipe.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import ts from 'typescript';
1212
import {ErrorCode, FatalDiagnosticError} from '../../diagnostics';
1313
import {Reference} from '../../imports';
1414
import {SemanticSymbol} from '../../incremental/semantic_graph';
15-
import {InjectableClassRegistry, MetadataRegistry, MetaType} from '../../metadata';
15+
import {InjectableClassRegistry, MetadataRegistry, MetaKind} from '../../metadata';
1616
import {PartialEvaluator} from '../../partial_evaluator';
1717
import {PerfEvent, PerfRecorder} from '../../perf';
1818
import {ClassDeclaration, Decorator, ReflectionHost, reflectObjectLiteral} from '../../reflection';
@@ -154,7 +154,7 @@ export class PipeDecoratorHandler implements
154154
register(node: ClassDeclaration, analysis: Readonly<PipeHandlerData>): void {
155155
const ref = new Reference(node);
156156
this.metaRegistry.registerPipeMetadata({
157-
type: MetaType.Pipe,
157+
kind: MetaKind.Pipe,
158158
ref,
159159
name: analysis.meta.pipeName,
160160
nameExpr: analysis.pipeNameExpr,

0 commit comments

Comments
 (0)