Skip to content

Commit 95dcf5f

Browse files
crisbetothePunderWoman
authored andcommitted
fix(compiler-cli): handle default imports in defer blocks (angular#53695)
Fixes that `@defer` blocks weren't recognizing default imports and generating the proper code for them. Default symbols need to be accessed through the `default` property in the `import` statement, rather than by their name. PR Close angular#53695
1 parent ebbacb9 commit 95dcf5f

File tree

9 files changed

+162
-24
lines changed

9 files changed

+162
-24
lines changed

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

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -483,12 +483,16 @@ export class ComponentDecoratorHandler implements
483483
// (if it exists) and populate the `DeferredSymbolTracker` state. These operations are safe
484484
// for the local compilation mode, since they don't require accessing/resolving symbols
485485
// outside of the current source file.
486-
let explicitlyDeferredTypes: Map<string, string>|null = null;
486+
let explicitlyDeferredTypes: Map<string, {importPath: string, isDefaultImport: boolean}>|null =
487+
null;
487488
if (metadata.isStandalone && rawDeferredImports !== null) {
488489
const deferredTypes = this.collectExplicitlyDeferredSymbols(rawDeferredImports);
489490
for (const [deferredType, importDetails] of deferredTypes) {
490491
explicitlyDeferredTypes ??= new Map();
491-
explicitlyDeferredTypes.set(importDetails.name, importDetails.from);
492+
explicitlyDeferredTypes.set(importDetails.name, {
493+
importPath: importDetails.from,
494+
isDefaultImport: isDefaultImport(importDetails.node),
495+
});
492496
this.deferredSymbolTracker.markAsDeferrableCandidate(
493497
deferredType, importDetails.node, node, true /* isExplicitlyDeferred */);
494498
}
@@ -1255,9 +1259,8 @@ export class ComponentDecoratorHandler implements
12551259
* Computes a list of deferrable symbols based on dependencies from
12561260
* the `@Component.imports` field and their usage in `@defer` blocks.
12571261
*/
1258-
private collectDeferredSymbols(resolution: Readonly<ComponentResolutionData>):
1259-
Map<string, string> {
1260-
const deferrableTypes = new Map<string, string>();
1262+
private collectDeferredSymbols(resolution: Readonly<ComponentResolutionData>) {
1263+
const deferrableTypes = new Map<string, {importPath: string, isDefaultImport: boolean}>();
12611264
// Go over all dependencies of all defer blocks and update the value of
12621265
// the `isDeferrable` flag and the `importPath` to reflect the current
12631266
// state after visiting all components during the `resolve` phase.
@@ -1272,7 +1275,10 @@ export class ComponentDecoratorHandler implements
12721275
if (importDecl !== null && this.deferredSymbolTracker.canDefer(importDecl)) {
12731276
deferBlockDep.isDeferrable = true;
12741277
deferBlockDep.importPath = (importDecl.moduleSpecifier as ts.StringLiteral).text;
1275-
deferrableTypes.set(deferBlockDep.symbolName, deferBlockDep.importPath);
1278+
deferrableTypes.set(deferBlockDep.symbolName, {
1279+
importPath: deferBlockDep.importPath,
1280+
isDefaultImport: isDefaultImport(importDecl),
1281+
});
12761282
}
12771283
}
12781284
}
@@ -1368,13 +1374,14 @@ export class ComponentDecoratorHandler implements
13681374
continue;
13691375
}
13701376
// Collect initial information about this dependency.
1371-
// The `isDeferrable` flag and the `importPath` info would be
1377+
// `isDeferrable`, `importPath` and `isDefaultImport` will be
13721378
// added later during the `compile` step.
13731379
deps.push({
13741380
type: decl.type as WrappedNodeExpr<ts.Identifier>,
13751381
symbolName: decl.ref.node.name.escapedText as string,
13761382
isDeferrable: false,
13771383
importPath: null,
1384+
isDefaultImport: false,
13781385
// Extra info to match corresponding import during the `compile` phase.
13791386
classDeclaration: decl.ref.node as ts.ClassDeclaration,
13801387
});
@@ -1532,7 +1539,8 @@ function extractPipes(dependencies: Array<PipeMeta|DirectiveMeta|NgModuleMeta>):
15321539
* in the `setClassMetadataAsync` call. Otherwise, an import declaration gets retained.
15331540
*/
15341541
function removeDeferrableTypesFromComponentDecorator(
1535-
analysis: Readonly<ComponentAnalysisData>, deferrableTypes: Map<string, string>) {
1542+
analysis: Readonly<ComponentAnalysisData>,
1543+
deferrableTypes: Map<string, {importPath: string, isDefaultImport: boolean}>) {
15361544
if (analysis.classMetadata) {
15371545
const deferrableSymbols = new Set(deferrableTypes.keys());
15381546
const rewrittenDecoratorsNode = removeIdentifierReferences(
@@ -1608,3 +1616,8 @@ function validateStandaloneImports(
16081616

16091617
return diagnostics;
16101618
}
1619+
1620+
/** Returns whether an ImportDeclaration is a default import. */
1621+
function isDefaultImport(node: ts.ImportDeclaration): boolean {
1622+
return node.importClause !== undefined && node.importClause.namedBindings === undefined;
1623+
}

packages/compiler-cli/src/ngtsc/annotations/component/src/metadata.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ export interface ComponentAnalysisData {
7777
/**
7878
* Map of symbol name -> import path for types from `@Component.deferredImports` field.
7979
*/
80-
explicitlyDeferredTypes: Map<string, string>|null;
80+
explicitlyDeferredTypes: Map<string, {importPath: string, isDefaultImport: boolean}>|null;
8181

8282
schemas: SchemaMetadata[]|null;
8383

packages/compiler-cli/src/ngtsc/reflection/src/typescript.ts

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -670,13 +670,19 @@ function getFarLeftIdentifier(propertyAccess: ts.PropertyAccessExpression): ts.I
670670
}
671671

672672
/**
673-
* Return the ImportDeclaration for the given `node` if it is either an `ImportSpecifier` or a
674-
* `NamespaceImport`. If not return `null`.
673+
* Gets the closest ancestor `ImportDeclaration` to a node.
675674
*/
676675
export function getContainingImportDeclaration(node: ts.Node): ts.ImportDeclaration|null {
677-
return ts.isImportSpecifier(node) ? node.parent!.parent!.parent! :
678-
ts.isNamespaceImport(node) ? node.parent.parent :
679-
null;
676+
let parent = node.parent;
677+
678+
while (parent && !ts.isSourceFile(parent)) {
679+
if (ts.isImportDeclaration(parent)) {
680+
return parent;
681+
}
682+
parent = parent.parent;
683+
}
684+
685+
return null;
680686
}
681687

682688
/**

packages/compiler-cli/test/ngtsc/ngtsc_spec.ts

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9559,6 +9559,55 @@ function allTests(os: string) {
95599559
expect(jsContents).not.toContain('import { CmpA }');
95609560
expect(jsContents).not.toContain('import { CmpB }');
95619561
});
9562+
9563+
it('should handle deferred dependencies imported through a default import', () => {
9564+
env.write('cmp-a.ts', `
9565+
import { Component } from '@angular/core';
9566+
9567+
@Component({
9568+
standalone: true,
9569+
selector: 'cmp-a',
9570+
template: 'CmpA!'
9571+
})
9572+
export default class CmpA {}
9573+
`);
9574+
9575+
env.write('/test.ts', `
9576+
import { Component } from '@angular/core';
9577+
import CmpA from './cmp-a';
9578+
9579+
@Component({
9580+
selector: 'local-dep',
9581+
standalone: true,
9582+
template: 'Local dependency',
9583+
})
9584+
export class LocalDep {}
9585+
9586+
@Component({
9587+
selector: 'test-cmp',
9588+
standalone: true,
9589+
imports: [CmpA, LocalDep],
9590+
template: \`
9591+
@defer {
9592+
<cmp-a />
9593+
<local-dep />
9594+
}
9595+
\`,
9596+
})
9597+
export class TestCmp {}
9598+
`);
9599+
9600+
env.driveMain();
9601+
9602+
const jsContents = env.getContents('test.js');
9603+
9604+
expect(jsContents).toContain('ɵɵdefer(1, 0, TestCmp_Defer_1_DepsFn)');
9605+
expect(jsContents).toContain('() => [import("./cmp-a").then(m => m.default)]');
9606+
9607+
// The `CmpA` symbol wasn't referenced elsewhere, so it can be defer-loaded
9608+
// via dynamic imports and an original import can be removed.
9609+
expect(jsContents).not.toContain('import CmpA');
9610+
});
95629611
});
95639612

95649613
it('should detect pipe used in the `when` trigger as an eager dependency', () => {
@@ -10118,6 +10167,60 @@ function allTests(os: string) {
1011810167
expect(jsContents).toContain('setClassMetadata');
1011910168
});
1012010169
});
10170+
10171+
it('should generate setClassMetadataAsync for default imports', () => {
10172+
env.write('cmp-a.ts', `
10173+
import {Component} from '@angular/core';
10174+
10175+
@Component({
10176+
standalone: true,
10177+
selector: 'cmp-a',
10178+
template: 'CmpA!'
10179+
})
10180+
export default class CmpA {}
10181+
`);
10182+
10183+
env.write('/test.ts', `
10184+
import {Component} from '@angular/core';
10185+
import CmpA from './cmp-a';
10186+
10187+
@Component({
10188+
selector: 'local-dep',
10189+
standalone: true,
10190+
template: 'Local dependency',
10191+
})
10192+
export class LocalDep {}
10193+
10194+
@Component({
10195+
selector: 'test-cmp',
10196+
standalone: true,
10197+
imports: [CmpA, LocalDep],
10198+
template: \`
10199+
@defer {
10200+
<cmp-a />
10201+
<local-dep />
10202+
}
10203+
\`,
10204+
})
10205+
export class TestCmp {}
10206+
`);
10207+
10208+
env.driveMain();
10209+
10210+
const jsContents = env.getContents('test.js');
10211+
10212+
expect(jsContents).toContain('ɵɵdefer(1, 0, TestCmp_Defer_1_DepsFn)');
10213+
expect(jsContents)
10214+
.toContain(
10215+
// ngDevMode check is present
10216+
'(() => { (typeof ngDevMode === "undefined" || ngDevMode) && ' +
10217+
// Main `setClassMetadataAsync` call
10218+
'i0.ɵsetClassMetadataAsync(TestCmp, ' +
10219+
// Dependency loading function (note: no local `LocalDep` here)
10220+
'() => [import("./cmp-a").then(m => m.default)], ' +
10221+
// Callback that invokes `setClassMetadata` at the end
10222+
'CmpA => { i0.ɵsetClassMetadata(TestCmp');
10223+
});
1012110224
});
1012210225

1012310226
describe('debug info', () => {

packages/compiler/src/render3/r3_class_metadata_compiler.ts

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -72,18 +72,23 @@ export function compileClassMetadata(metadata: R3ClassMetadata): o.Expression {
7272
* check to tree-shake away this code in production mode.
7373
*/
7474
export function compileComponentClassMetadata(
75-
metadata: R3ClassMetadata, deferrableTypes: Map<string, string>|null): o.Expression {
75+
metadata: R3ClassMetadata,
76+
deferrableTypes: Map<string, {importPath: string, isDefaultImport: boolean}>|
77+
null): o.Expression {
7678
if (deferrableTypes === null || deferrableTypes.size === 0) {
7779
// If there are no deferrable symbols - just generate a regular `setClassMetadata` call.
7880
return compileClassMetadata(metadata);
7981
}
8082

8183
const dynamicImports: o.Expression[] = [];
8284
const importedSymbols: o.FnParam[] = [];
83-
for (const [symbolName, importPath] of deferrableTypes) {
85+
for (const [symbolName, {importPath, isDefaultImport}] of deferrableTypes) {
8486
// e.g. `(m) => m.CmpA`
8587
const innerFn =
86-
o.arrowFn([new o.FnParam('m', o.DYNAMIC_TYPE)], o.variable('m').prop(symbolName));
88+
// Default imports are always accessed through the `default` property.
89+
o.arrowFn(
90+
[new o.FnParam('m', o.DYNAMIC_TYPE)],
91+
o.variable('m').prop(isDefaultImport ? 'default' : symbolName));
8792

8893
// e.g. `import('./cmp-a').then(...)`
8994
const importExpr = (new o.DynamicImportExpr(importPath)).prop('then').callFn([innerFn]);

packages/compiler/src/render3/view/api.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -219,6 +219,11 @@ export interface R3DeferBlockTemplateDependency {
219219
* Import path where this dependency is located.
220220
*/
221221
importPath: string|null;
222+
223+
/**
224+
* Whether the symbol is the default export.
225+
*/
226+
isDefaultImport: boolean;
222227
}
223228

224229
/**
@@ -282,7 +287,7 @@ export interface R3ComponentMetadata<DeclarationT extends R3TemplateDependency>
282287
/**
283288
* Map of deferrable symbol names -> corresponding import paths.
284289
*/
285-
deferrableTypes: Map<string, string>;
290+
deferrableTypes: Map<string, {importPath: string, isDefaultImport: boolean}>;
286291

287292
/**
288293
* Specifies how the 'directives' and/or `pipes` array, if generated, need to be emitted.

packages/compiler/src/render3/view/compiler.ts

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -164,14 +164,16 @@ export function compileDirectiveFromMetadata(
164164
* deferrable dependencies.
165165
*/
166166
function createDeferredDepsFunction(
167-
constantPool: ConstantPool, name: string, deps: Map<string, string>) {
167+
constantPool: ConstantPool, name: string,
168+
deps: Map<string, {importPath: string, isDefaultImport: boolean}>) {
168169
// This defer block has deps for which we need to generate dynamic imports.
169170
const dependencyExp: o.Expression[] = [];
170171

171-
for (const [symbolName, importPath] of deps) {
172+
for (const [symbolName, {importPath, isDefaultImport}] of deps) {
172173
// Callback function, e.g. `m () => m.MyCmp;`.
173-
const innerFn =
174-
o.arrowFn([new o.FnParam('m', o.DYNAMIC_TYPE)], o.variable('m').prop(symbolName));
174+
const innerFn = o.arrowFn(
175+
[new o.FnParam('m', o.DYNAMIC_TYPE)],
176+
o.variable('m').prop(isDefaultImport ? 'default' : symbolName));
175177

176178
// Dynamic import, e.g. `import('./a').then(...)`.
177179
const importExpr = (new o.DynamicImportExpr(importPath)).prop('then').callFn([innerFn]);

packages/compiler/src/render3/view/template.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1400,7 +1400,9 @@ export class TemplateDefinitionBuilder implements t.Visitor<void>, LocalResolver
14001400
if (deferredDep.isDeferrable) {
14011401
// Callback function, e.g. `m () => m.MyCmp;`.
14021402
const innerFn = o.arrowFn(
1403-
[new o.FnParam('m', o.DYNAMIC_TYPE)], o.variable('m').prop(deferredDep.symbolName));
1403+
[new o.FnParam('m', o.DYNAMIC_TYPE)],
1404+
// Default imports are always accessed through the `default` property.
1405+
o.variable('m').prop(deferredDep.isDefaultImport ? 'default' : deferredDep.symbolName));
14041406

14051407
// Dynamic import, e.g. `import('./a').then(...)`.
14061408
const importExpr =

packages/compiler/src/template/pipeline/src/phases/create_defer_deps_fns.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,9 @@ export function createDeferDepsFns(job: ComponentCompilationJob): void {
2828
if (dep.isDeferrable) {
2929
// Callback function, e.g. `m () => m.MyCmp;`.
3030
const innerFn = o.arrowFn(
31-
[new o.FnParam('m', o.DYNAMIC_TYPE)], o.variable('m').prop(dep.symbolName));
31+
// Default imports are always accessed through the `default` property.
32+
[new o.FnParam('m', o.DYNAMIC_TYPE)],
33+
o.variable('m').prop(dep.isDefaultImport ? 'default' : dep.symbolName));
3234

3335
// Dynamic import, e.g. `import('./a').then(...)`.
3436
const importExpr =

0 commit comments

Comments
 (0)