Skip to content

Commit

Permalink
refactor(compiler): extra diagnostics for @defer in local compilati…
Browse files Browse the repository at this point in the history
…on mode

This commit adds extra logic to produce a diagnostic in case `@Component.deferredImports` contain types from imports that also bring eager symbols. This would result in retaining a regular import and generating a dynamic import, which would not allow to defer-load dependencies.
  • Loading branch information
AndrewKushnir committed Jan 12, 2024
1 parent 1f8c53c commit 50ac488
Show file tree
Hide file tree
Showing 15 changed files with 350 additions and 107 deletions.
103 changes: 71 additions & 32 deletions packages/compiler-cli/src/ngtsc/annotations/component/src/handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
* found in the LICENSE file at https://angular.io/license
*/

import {AnimationTriggerNames, BoundTarget, compileClassDebugInfo, compileComponentClassMetadata, compileComponentFromMetadata, compileDeclareClassMetadata, compileDeclareComponentFromMetadata, ConstantPool, CssSelector, DeclarationListEmitMode, DeclareComponentTemplateInfo, DEFAULT_INTERPOLATION_CONFIG, DomElementSchemaRegistry, Expression, FactoryTarget, makeBindingParser, R3ComponentMetadata, R3DeferBlockMetadata, R3DeferBlockTemplateDependency, R3DirectiveDependencyMetadata, R3NgModuleDependencyMetadata, R3PipeDependencyMetadata, R3TargetBinder, R3TemplateDependency, R3TemplateDependencyKind, R3TemplateDependencyMetadata, SchemaMetadata, SelectorMatcher, TmplAstDeferredBlock, TmplAstDeferredBlockTriggers, TmplAstDeferredTrigger, TmplAstElement, ViewEncapsulation, WrappedNodeExpr} from '@angular/compiler';
import {AnimationTriggerNames, BoundTarget, compileClassDebugInfo, compileComponentClassMetadata, compileComponentFromMetadata, compileDeclareClassMetadata, compileDeclareComponentFromMetadata, ConstantPool, CssSelector, DeclarationListEmitMode, DeclareComponentTemplateInfo, DEFAULT_INTERPOLATION_CONFIG, DeferBlockDepsEmitMode, DomElementSchemaRegistry, Expression, FactoryTarget, makeBindingParser, R3ComponentMetadata, R3DeferBlockMetadata, R3DeferBlockTemplateDependency, R3DirectiveDependencyMetadata, R3NgModuleDependencyMetadata, R3PipeDependencyMetadata, R3TargetBinder, R3TemplateDependency, R3TemplateDependencyKind, R3TemplateDependencyMetadata, SchemaMetadata, SelectorMatcher, TmplAstDeferredBlock, TmplAstDeferredBlockTriggers, TmplAstDeferredTrigger, TmplAstElement, ViewEncapsulation, WrappedNodeExpr} from '@angular/compiler';
import ts from 'typescript';

import {Cycle, CycleAnalyzer, CycleHandlingStrategy} from '../../../cycles';
Expand Down Expand Up @@ -477,6 +477,15 @@ export class ComponentDecoratorHandler implements
styles.push(...template.styles);
}

// Collect all explicitly deferred symbols from the `@Component.deferredImports` field
// if it exists. As a part of that process we also populate the `DeferredSymbolTracker` state.
// This operation is safe in local compilation mode, since it doesn't require
// accessing/resolving symbols outside of the current source file.
let explicitlyDeferredTypes: Map<string, string>|null = null;
if (metadata.isStandalone && rawDeferredImports !== null) {
explicitlyDeferredTypes = this.collectExplicitlyDeferredSymbols(rawDeferredImports);
}

const output: AnalysisOutput<ComponentAnalysisData> = {
analysis: {
baseClass: readBaseClass(node, this.reflector, this.evaluator),
Expand Down Expand Up @@ -524,6 +533,7 @@ export class ComponentDecoratorHandler implements
resolvedImports,
rawDeferredImports,
resolvedDeferredImports,
explicitlyDeferredTypes,
schemas,
decorator: decorator?.node as ts.Decorator | null ?? null,
},
Expand Down Expand Up @@ -652,6 +662,43 @@ export class ComponentDecoratorHandler implements
resolve(
node: ClassDeclaration, analysis: Readonly<ComponentAnalysisData>,
symbol: ComponentSymbol): ResolveResult<ComponentResolutionData> {
const metadata = analysis.meta as Readonly<R3ComponentMetadata<R3TemplateDependencyMetadata>>;
const diagnostics: ts.Diagnostic[] = [];
const context = getSourceFile(node);

// Check if there are some import declarations that contain symbols used within
// the `@Component.deferredImports` field, but those imports contain other symbols
// and thus the declaration can not be removed.
const nonRemovableImports = this.deferredSymbolTracker.getNonRemovableDeferredImports(context);
if (nonRemovableImports.length > 0) {
for (const importDecl of nonRemovableImports) {
const diagnostic = makeDiagnostic(
ErrorCode.DEFERRED_DEPENDENCY_IMPORTED_EAGERLY, importDecl,
`This import contains symbols used in the \`@Component.deferredImports\` array, ` +
`but also some other symbols, which prevents Angular compiler from ` +
`generating dynamic imports for deferred dependencies. ` +
`To fix this, make sure that this import contains *only* symbols ` +
`that are used within the \`@Component.deferredImports\` array.`);
diagnostics.push(diagnostic);
}
return {diagnostics};
}

if (this.compilationMode === CompilationMode.LOCAL) {
return {
data: {
declarationListEmitMode: (!analysis.meta.isStandalone || analysis.rawImports !== null) ?
DeclarationListEmitMode.RuntimeResolved :
DeclarationListEmitMode.Direct,
declarations: EMPTY_ARRAY,
deferBlocks: this.locateDeferBlocksWithoutScope(analysis.template),
deferBlockDepsEmitMode: DeferBlockDepsEmitMode.PerComponent,
deferrableDeclToImportDecl: new Map(),
deferrableTypes: new Map(),
},
};
}

if (this.semanticDepGraphUpdater !== null && analysis.baseClass instanceof Reference) {
symbol.baseClass = this.semanticDepGraphUpdater.getSymbol(analysis.baseClass.node);
}
Expand All @@ -660,18 +707,14 @@ export class ComponentDecoratorHandler implements
return {};
}

const context = getSourceFile(node);
const metadata = analysis.meta as Readonly<R3ComponentMetadata<R3TemplateDependencyMetadata>>;


const data: ComponentResolutionData = {
declarations: EMPTY_ARRAY,
declarationListEmitMode: DeclarationListEmitMode.Direct,
deferBlocks: new Map(),
deferBlockDepsEmitMode: DeferBlockDepsEmitMode.PerBlock,
deferrableDeclToImportDecl: new Map(),
deferrableTypes: new Map(),
};
const diagnostics: ts.Diagnostic[] = [];

const scope = this.scopeReader.getScopeForComponent(node);
if (scope !== null) {
Expand Down Expand Up @@ -1047,13 +1090,13 @@ export class ComponentDecoratorHandler implements
return [];
}

// Collect all explicitly deferred symbols from the `@Component.deferredImports` field
// if it exists. As a part of that process we also populate the `DeferredSymbolTracker` state,
// which is then used within the `collectDeferredSymbols` call.
this.collectExplicitlyDeferredSymbols(analysis);
const deferrableTypes = this.collectDeferredSymbols(resolution);

const meta: R3ComponentMetadata<R3TemplateDependency> = {...analysis.meta, ...resolution};
const meta: R3ComponentMetadata<R3TemplateDependency> = {
...analysis.meta,
...resolution,
deferrableTypes,
};
const fac = compileNgFactoryDefField(toFactoryMetadata(meta, FactoryTarget.Component));

removeDeferrableTypesFromComponentDecorator(analysis, deferrableTypes);
Expand Down Expand Up @@ -1100,24 +1143,25 @@ export class ComponentDecoratorHandler implements

compileLocal(
node: ClassDeclaration, analysis: Readonly<ComponentAnalysisData>,
pool: ConstantPool): CompileResult[] {
resolution: Readonly<ComponentResolutionData>, pool: ConstantPool): CompileResult[] {
if (analysis.template.errors !== null && analysis.template.errors.length > 0) {
return [];
}

const deferrableTypes = this.collectExplicitlyDeferredSymbols(analysis);
// In the local compilation mode we can only rely on the information available
// within the `@Component.deferredImports` array, because in this mode compiler
// doesn't have information on which dependencies belong to which defer blocks.
const deferrableTypes = analysis.explicitlyDeferredTypes;

const meta: R3ComponentMetadata<R3TemplateDependency> = {
...analysis.meta,
declarationListEmitMode: (!analysis.meta.isStandalone || analysis.rawImports !== null) ?
DeclarationListEmitMode.RuntimeResolved :
DeclarationListEmitMode.Direct,
declarations: EMPTY_ARRAY,
deferBlocks: this.locateDeferBlocksWithoutScope(analysis.template),
deferrableDeclToImportDecl: new Map(),
deferrableTypes,
...resolution,
deferrableTypes: deferrableTypes ?? new Map(),
};

removeDeferrableTypesFromComponentDecorator(analysis, deferrableTypes);
if (analysis.explicitlyDeferredTypes !== null) {
removeDeferrableTypesFromComponentDecorator(analysis, analysis.explicitlyDeferredTypes);
}

const fac = compileNgFactoryDefField(toFactoryMetadata(meta, FactoryTarget.Component));
const def = compileComponentFromMetadata(meta, pool, makeBindingParser());
Expand Down Expand Up @@ -1185,14 +1229,13 @@ export class ComponentDecoratorHandler implements
/**
* Collects a list of deferrable symbols based on the `@Component.deferredImports` field.
*/
private collectExplicitlyDeferredSymbols(analysis: Readonly<ComponentAnalysisData>):
Map<string, string> {
private collectExplicitlyDeferredSymbols(rawDeferredImports: ts.Expression): Map<string, string> {
const deferrableTypes = new Map<string, string>();
if (!analysis.meta.isStandalone || analysis.rawDeferredImports === null ||
!ts.isArrayLiteralExpression(analysis.rawDeferredImports))
if (!ts.isArrayLiteralExpression(rawDeferredImports)) {
return deferrableTypes;
}

for (const element of analysis.rawDeferredImports.elements) {
for (const element of rawDeferredImports.elements) {
const node = tryUnwrapForwardRef(element, this.reflector) || element;

if (!ts.isIdentifier(node)) {
Expand All @@ -1203,7 +1246,7 @@ export class ComponentDecoratorHandler implements
const imp = this.reflector.getImportOfIdentifier(node);
if (imp !== null) {
deferrableTypes.set(imp.name, imp.from);
this.deferredSymbolTracker.markAsExplicitlyDeferred(imp.node);
this.deferredSymbolTracker.markAsDeferrableCandidate(node, imp.node, true);
}
}
return deferrableTypes;
Expand Down Expand Up @@ -1380,11 +1423,7 @@ export class ComponentDecoratorHandler implements
resolutionData.deferrableDeclToImportDecl.set(
decl.node as unknown as Expression, imp.node as unknown as Expression);

if (isDeferredImport) {
this.deferredSymbolTracker.markAsExplicitlyDeferred(imp.node);
} else {
this.deferredSymbolTracker.markAsDeferrableCandidate(node, imp.node);
}
this.deferredSymbolTracker.markAsDeferrableCandidate(node, imp.node, isDeferredImport);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import {ParsedTemplateWithSource, StyleUrlMeta} from './resources';
export type ComponentMetadataResolvedFields = SubsetOfKeys<
R3ComponentMetadata<R3TemplateDependencyMetadata>,
'declarations'|'declarationListEmitMode'|'deferBlocks'|'deferrableDeclToImportDecl'|
'deferrableTypes'>;
'deferrableTypes'|'deferBlockDepsEmitMode'>;

export interface ComponentAnalysisData {
/**
Expand Down Expand Up @@ -74,6 +74,11 @@ export interface ComponentAnalysisData {
rawDeferredImports: ts.Expression|null;
resolvedDeferredImports: Reference<ClassDeclaration>[]|null;

/**
* Map of symbol name -> import path for types from `@Component.deferredImports` field.
*/
explicitlyDeferredTypes: Map<string, string>|null;

schemas: SchemaMetadata[]|null;

decorator: ts.Decorator|null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,10 @@ export class DirectiveDecoratorHandler implements

resolve(node: ClassDeclaration, analysis: DirectiveHandlerData, symbol: DirectiveSymbol):
ResolveResult<unknown> {
if (this.compilationMode === CompilationMode.LOCAL) {
return {};
}

if (this.semanticDepGraphUpdater !== null && analysis.baseClass instanceof Reference) {
symbol.baseClass = this.semanticDepGraphUpdater.getSymbol(analysis.baseClass.node);
}
Expand Down Expand Up @@ -246,7 +250,7 @@ export class DirectiveDecoratorHandler implements

compileLocal(
node: ClassDeclaration, analysis: Readonly<DirectiveHandlerData>,
pool: ConstantPool): CompileResult[] {
resolution: Readonly<unknown>, pool: ConstantPool): CompileResult[] {
const fac = compileNgFactoryDefField(toFactoryMetadata(analysis.meta, FactoryTarget.Directive));
const def = compileDirectiveFromMetadata(analysis.meta, pool, makeBindingParser());
const inputTransformFields = compileInputTransformFields(analysis.inputs);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -555,6 +555,10 @@ export class NgModuleDecoratorHandler implements

resolve(node: ClassDeclaration, analysis: Readonly<NgModuleAnalysis>):
ResolveResult<NgModuleResolution> {
if (this.compilationMode === CompilationMode.LOCAL) {
return {};
}

const scope = this.scopeRegistry.getScopeOfModule(node);
const diagnostics: ts.Diagnostic[] = [];

Expand Down
4 changes: 4 additions & 0 deletions packages/compiler-cli/src/ngtsc/annotations/src/injectable.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,10 @@ export class InjectableDecoratorHandler implements

resolve(node: ClassDeclaration, analysis: Readonly<InjectableHandlerData>, symbol: null):
ResolveResult<unknown> {
if (this.compilationMode === CompilationMode.LOCAL) {
return {};
}

if (requiresValidCtor(analysis.meta)) {
const diagnostic = checkInheritanceOfInjectable(
node, this.injectableRegistry, this.reflector, this.evaluator, this.strictCtorDeps,
Expand Down
4 changes: 4 additions & 0 deletions packages/compiler-cli/src/ngtsc/annotations/src/pipe.ts
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,10 @@ export class PipeDecoratorHandler implements
}

resolve(node: ClassDeclaration): ResolveResult<unknown> {
if (this.compilationMode === CompilationMode.LOCAL) {
return {};
}

const duplicateDeclData = this.scopeRegistry.getDuplicateDeclarations(node);
if (duplicateDeclData !== null) {
// This pipe was declared twice (or more).
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,6 @@ import {getContainingImportDeclaration} from '../../reflection/src/typescript';
const AssumeEager = 'AssumeEager';
type AssumeEager = typeof AssumeEager;

/**
* A marker indicating that a symbol from an import declaration
* was referenced in a `@Component.deferredImports` list.
*/
const ExplicitlyDeferred = 'ExplicitlyDeferred';
type ExplicitlyDeferred = typeof ExplicitlyDeferred;

/**
* Maps imported symbol name to a set of locations where the symbols is used
* in a source file.
Expand All @@ -34,7 +27,8 @@ type SymbolMap = Map<string, Set<ts.Identifier>|AssumeEager>;
* in favor of using a dynamic import for cases when defer blocks are used.
*/
export class DeferredSymbolTracker {
private readonly imports = new Map<ts.ImportDeclaration, ExplicitlyDeferred|SymbolMap>();
private readonly imports = new Map<ts.ImportDeclaration, SymbolMap>();
private readonly explicitlyDeferredImports = new Set<ts.ImportDeclaration>();

constructor(
private readonly typeChecker: ts.TypeChecker,
Expand Down Expand Up @@ -86,32 +80,40 @@ export class DeferredSymbolTracker {
}

/**
* Marks a given import declaration as explicitly deferred, since it's
* used in the `@Component.deferredImports` field.
* Retrieves a list of import declarations that contain symbols used within
* `@Component.deferredImports`, but those imports can not be removed, since
* there are other symbols imported alongside deferred components.
*/
markAsExplicitlyDeferred(importDecl: ts.ImportDeclaration): void {
this.imports.set(importDecl, ExplicitlyDeferred);
getNonRemovableDeferredImports(sourceFile: ts.SourceFile): ts.ImportDeclaration[] {
const affectedImports: ts.ImportDeclaration[] = [];
for (const importDecl of this.explicitlyDeferredImports) {
if (importDecl.getSourceFile() === sourceFile && !this.canDefer(importDecl)) {
affectedImports.push(importDecl);
}
}
return affectedImports;
}

/**
* Marks a given identifier and an associated import declaration as a candidate
* for defer loading.
*/
markAsDeferrableCandidate(identifier: ts.Identifier, importDecl: ts.ImportDeclaration): void {
if (this.onlyExplicitDeferDependencyImports) {
markAsDeferrableCandidate(
identifier: ts.Identifier, importDecl: ts.ImportDeclaration,
isExplicitlyDeferred: boolean): void {
if (this.onlyExplicitDeferDependencyImports && !isExplicitlyDeferred) {
// Ignore deferrable candidates when only explicit deferred imports mode is enabled.
// In that mode only dependencies from the `@Component.deferredImports` field are
// defer-loadable.
return;
}

let symbolMap = this.imports.get(importDecl);

// Do we come across this import as a part of `@Component.deferredImports` already?
if (symbolMap === ExplicitlyDeferred) {
return;
if (isExplicitlyDeferred) {
this.explicitlyDeferredImports.add(importDecl);
}

let symbolMap = this.imports.get(importDecl);

// Do we come across this import for the first time?
if (!symbolMap) {
symbolMap = this.extractImportedSymbols(importDecl);
Expand Down Expand Up @@ -147,10 +149,6 @@ export class DeferredSymbolTracker {
}

const symbolsMap = this.imports.get(importDecl)!;
if (symbolsMap === ExplicitlyDeferred) {
return true;
}

for (const [symbol, refs] of symbolsMap) {
if (refs === AssumeEager || refs.size > 0) {
// There may be still eager references to this symbol.
Expand Down
5 changes: 3 additions & 2 deletions packages/compiler-cli/src/ngtsc/transform/src/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -202,8 +202,9 @@ export interface DecoratorHandler<D, A, S extends SemanticSymbol|null, R> {
* Generates code based on each individual source file without using its
* dependencies (suitable for local dev edit/refresh workflow)
*/
compileLocal(node: ClassDeclaration, analysis: Readonly<A>, constantPool: ConstantPool):
CompileResult|CompileResult[];
compileLocal(
node: ClassDeclaration, analysis: Readonly<A>, resolution: Readonly<R>,
constantPool: ConstantPool): CompileResult|CompileResult[];
}

/**
Expand Down
Loading

0 comments on commit 50ac488

Please sign in to comment.