Skip to content

Commit 5cd26a9

Browse files
crisbetoatscott
crisbeto
authored and
atscott
committed
fix(compiler-cli): handle deferred blocks with shared dependencies correctly (#59926)
When the compiler analyzes the defer blocks in a component, it generates two sets of dependencies: ones specific for each block and others from all the deferred blocks within the component. The logic that combines all the defer block dependencies wasn't de-duplicating them which resulted in us producing `setClassMetadataAsync` calls where the callback can have multiple parameters with the same name. This was a problem both in full and partial compilation, but the latter was more visible, because Babel throws an error in such cases. These changes add some logic to de-duplicate the dependencies so that we produce valid code. Fixes #59922. PR Close #59926
1 parent b0266bd commit 5cd26a9

File tree

7 files changed

+200
-3
lines changed

7 files changed

+200
-3
lines changed

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

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1888,22 +1888,32 @@ export class ComponentDecoratorHandler
18881888
private resolveAllDeferredDependencies(
18891889
resolution: Readonly<ComponentResolutionData>,
18901890
): R3DeferPerComponentDependency[] {
1891+
const seenDeps = new Set<ClassDeclaration>();
18911892
const deferrableTypes: R3DeferPerComponentDependency[] = [];
18921893
// Go over all dependencies of all defer blocks and update the value of
18931894
// the `isDeferrable` flag and the `importPath` to reflect the current
18941895
// state after visiting all components during the `resolve` phase.
18951896
for (const [_, deps] of resolution.deferPerBlockDependencies) {
18961897
for (const deferBlockDep of deps) {
1897-
const importDecl =
1898-
resolution.deferrableDeclToImportDecl.get(deferBlockDep.declaration.node) ?? null;
1898+
const node = deferBlockDep.declaration.node;
1899+
const importDecl = resolution.deferrableDeclToImportDecl.get(node) ?? null;
18991900
if (importDecl !== null && this.deferredSymbolTracker.canDefer(importDecl)) {
19001901
deferBlockDep.isDeferrable = true;
19011902
deferBlockDep.importPath = (importDecl.moduleSpecifier as ts.StringLiteral).text;
19021903
deferBlockDep.isDefaultImport = isDefaultImport(importDecl);
1903-
deferrableTypes.push(deferBlockDep as R3DeferPerComponentDependency);
1904+
1905+
// The same dependency may be used across multiple deferred blocks. De-duplicate it
1906+
// because it can throw off other logic further down the compilation pipeline.
1907+
// Note that the logic above needs to run even if the dependency is seen before,
1908+
// because the object literals are different between each block.
1909+
if (!seenDeps.has(node)) {
1910+
seenDeps.add(node);
1911+
deferrableTypes.push(deferBlockDep as R3DeferPerComponentDependency);
1912+
}
19041913
}
19051914
}
19061915
}
1916+
19071917
return deferrableTypes;
19081918
}
19091919

‎packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_deferred/GOLDEN_PARTIAL.js‎

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1122,3 +1122,99 @@ i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDE
11221122
****************************************************************************************************/
11231123
export {};
11241124

1125+
/****************************************************************************************************
1126+
* PARTIAL FILE: deferred_with_duplicate_external_dep_lazy.js
1127+
****************************************************************************************************/
1128+
import { Directive } from '@angular/core';
1129+
import * as i0 from "@angular/core";
1130+
export class DuplicateLazyDep {
1131+
}
1132+
DuplicateLazyDep.ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: DuplicateLazyDep, deps: [], target: i0.ɵɵFactoryTarget.Directive });
1133+
DuplicateLazyDep.ɵdir = i0.ɵɵngDeclareDirective({ minVersion: "14.0.0", version: "0.0.0-PLACEHOLDER", type: DuplicateLazyDep, isStandalone: true, selector: "duplicate-lazy-dep", ngImport: i0 });
1134+
i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: DuplicateLazyDep, decorators: [{
1135+
type: Directive,
1136+
args: [{ selector: 'duplicate-lazy-dep' }]
1137+
}] });
1138+
1139+
/****************************************************************************************************
1140+
* PARTIAL FILE: deferred_with_duplicate_external_dep_lazy.d.ts
1141+
****************************************************************************************************/
1142+
import * as i0 from "@angular/core";
1143+
export declare class DuplicateLazyDep {
1144+
static ɵfac: i0.ɵɵFactoryDeclaration<DuplicateLazyDep, never>;
1145+
static ɵdir: i0.ɵɵDirectiveDeclaration<DuplicateLazyDep, "duplicate-lazy-dep", never, {}, {}, never, never, true, never>;
1146+
}
1147+
1148+
/****************************************************************************************************
1149+
* PARTIAL FILE: deferred_with_duplicate_external_dep_other.js
1150+
****************************************************************************************************/
1151+
import { Directive } from '@angular/core';
1152+
import * as i0 from "@angular/core";
1153+
export class OtherLazyDep {
1154+
}
1155+
OtherLazyDep.ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: OtherLazyDep, deps: [], target: i0.ɵɵFactoryTarget.Directive });
1156+
OtherLazyDep.ɵdir = i0.ɵɵngDeclareDirective({ minVersion: "14.0.0", version: "0.0.0-PLACEHOLDER", type: OtherLazyDep, isStandalone: true, selector: "other-lazy-dep", ngImport: i0 });
1157+
i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: OtherLazyDep, decorators: [{
1158+
type: Directive,
1159+
args: [{ selector: 'other-lazy-dep' }]
1160+
}] });
1161+
1162+
/****************************************************************************************************
1163+
* PARTIAL FILE: deferred_with_duplicate_external_dep_other.d.ts
1164+
****************************************************************************************************/
1165+
import * as i0 from "@angular/core";
1166+
export declare class OtherLazyDep {
1167+
static ɵfac: i0.ɵɵFactoryDeclaration<OtherLazyDep, never>;
1168+
static ɵdir: i0.ɵɵDirectiveDeclaration<OtherLazyDep, "other-lazy-dep", never, {}, {}, never, never, true, never>;
1169+
}
1170+
1171+
/****************************************************************************************************
1172+
* PARTIAL FILE: deferred_with_duplicate_external_dep.js
1173+
****************************************************************************************************/
1174+
import { Component } from '@angular/core';
1175+
import * as i0 from "@angular/core";
1176+
export class MyApp {
1177+
}
1178+
MyApp.ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyApp, deps: [], target: i0.ɵɵFactoryTarget.Component });
1179+
MyApp.ɵcmp = i0.ɵɵngDeclareComponent({ minVersion: "17.0.0", version: "0.0.0-PLACEHOLDER", type: MyApp, isStandalone: true, selector: "ng-component", ngImport: i0, template: `
1180+
@defer {
1181+
<duplicate-lazy-dep/>
1182+
}
1183+
1184+
@defer {
1185+
<duplicate-lazy-dep/>
1186+
}
1187+
1188+
@defer {
1189+
<other-lazy-dep/>
1190+
}
1191+
`, isInline: true, deferBlockDependencies: [() => [import("./deferred_with_duplicate_external_dep_lazy").then(m => m.DuplicateLazyDep)], () => [import("./deferred_with_duplicate_external_dep_lazy").then(m => m.DuplicateLazyDep)], () => [import("./deferred_with_duplicate_external_dep_other").then(m => m.OtherLazyDep)]] });
1192+
i0.ɵɵngDeclareClassMetadataAsync({ minVersion: "18.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyApp, resolveDeferredDeps: () => [import("./deferred_with_duplicate_external_dep_lazy").then(m => m.DuplicateLazyDep), import("./deferred_with_duplicate_external_dep_other").then(m => m.OtherLazyDep)], resolveMetadata: (DuplicateLazyDep, OtherLazyDep) => ({ decorators: [{
1193+
type: Component,
1194+
args: [{
1195+
template: `
1196+
@defer {
1197+
<duplicate-lazy-dep/>
1198+
}
1199+
1200+
@defer {
1201+
<duplicate-lazy-dep/>
1202+
}
1203+
1204+
@defer {
1205+
<other-lazy-dep/>
1206+
}
1207+
`,
1208+
imports: [DuplicateLazyDep, OtherLazyDep],
1209+
}]
1210+
}], ctorParameters: null, propDecorators: null }) });
1211+
1212+
/****************************************************************************************************
1213+
* PARTIAL FILE: deferred_with_duplicate_external_dep.d.ts
1214+
****************************************************************************************************/
1215+
import * as i0 from "@angular/core";
1216+
export declare class MyApp {
1217+
static ɵfac: i0.ɵɵFactoryDeclaration<MyApp, never>;
1218+
static ɵcmp: i0.ɵɵComponentDeclaration<MyApp, "ng-component", never, {}, {}, never, never, true, never>;
1219+
}
1220+

‎packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_deferred/TEST_CASES.json‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -290,6 +290,25 @@
290290
"failureMessage": "Incorrect template"
291291
}
292292
]
293+
},
294+
{
295+
"description": "should handle a component with deferred blocks that share the same dependency",
296+
"inputFiles": [
297+
"deferred_with_duplicate_external_dep.ts",
298+
"deferred_with_duplicate_external_dep_lazy.ts",
299+
"deferred_with_duplicate_external_dep_other.ts"
300+
],
301+
"expectations": [
302+
{
303+
"files": [
304+
{
305+
"expected": "deferred_with_duplicate_external_dep_template.js",
306+
"generated": "deferred_with_duplicate_external_dep.js"
307+
}
308+
],
309+
"failureMessage": "Incorrect template"
310+
}
311+
]
293312
}
294313
]
295314
}

‎packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_deferred/deferred_with_duplicate_external_dep.ts‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
import {Component} from '@angular/core';
2+
import {DuplicateLazyDep} from './deferred_with_duplicate_external_dep_lazy';
3+
import {OtherLazyDep} from './deferred_with_duplicate_external_dep_other';
4+
5+
@Component({
6+
template: `
7+
@defer {
8+
<duplicate-lazy-dep/>
9+
}
10+
11+
@defer {
12+
<duplicate-lazy-dep/>
13+
}
14+
15+
@defer {
16+
<other-lazy-dep/>
17+
}
18+
`,
19+
imports: [DuplicateLazyDep, OtherLazyDep],
20+
})
21+
export class MyApp {}

‎packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_deferred/deferred_with_duplicate_external_dep_lazy.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
import {Directive} from '@angular/core';
2+
3+
@Directive({selector: 'duplicate-lazy-dep'})
4+
export class DuplicateLazyDep {}

‎packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_deferred/deferred_with_duplicate_external_dep_other.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
import {Directive} from '@angular/core';
2+
3+
@Directive({selector: 'other-lazy-dep'})
4+
export class OtherLazyDep {}

‎packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_deferred/deferred_with_duplicate_external_dep_template.js‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
const MyApp_Defer_1_DepsFn = () => [import("./deferred_with_duplicate_external_dep_lazy").then(m => m.DuplicateLazyDep)];
2+
// NOTE: in linked tests there is one more loader here, because linked compilation doesn't have the ability to de-dupe identical functions.
3+
…
4+
const MyApp_Defer_7_DepsFn = () => [import("./deferred_with_duplicate_external_dep_other").then(m => m.OtherLazyDep)];
5+
6+
…
7+
8+
$r3$.ɵɵdefineComponent({
9+
…
10+
template: function MyApp_Template(rf, ctx) {
11+
if (rf & 1) {
12+
$r3$.ɵɵtemplate(0, MyApp_Defer_0_Template, 1, 0);
13+
$r3$.ɵɵdefer(1, 0, MyApp_Defer_1_DepsFn);
14+
$r3$.ɵɵdeferOnIdle();
15+
$r3$.ɵɵtemplate(3, MyApp_Defer_3_Template, 1, 0);
16+
// NOTE: does not check the function name, because linked compilation doesn't have the ability to de-dupe identical functions.
17+
$r3$.ɵɵdefer(4, 3, …);
18+
$r3$.ɵɵdeferOnIdle();
19+
$r3$.ɵɵtemplate(6, MyApp_Defer_6_Template, 1, 0);
20+
$r3$.ɵɵdefer(7, 6, MyApp_Defer_7_DepsFn);
21+
$r3$.ɵɵdeferOnIdle();
22+
}
23+
},
24+
encapsulation: 2
25+
});
26+
27+
…
28+
29+
(() => {
30+
(typeof ngDevMode === "undefined" || ngDevMode) && $r3$.ɵsetClassMetadataAsync(MyApp, () => [
31+
import("./deferred_with_duplicate_external_dep_lazy").then(m => m.DuplicateLazyDep),
32+
import("./deferred_with_duplicate_external_dep_other").then(m => m.OtherLazyDep)
33+
], (DuplicateLazyDep, OtherLazyDep) => {
34+
$r3$.ɵsetClassMetadata(MyApp, [{
35+
type: Component,
36+
args: [{
37+
template: …,
38+
// NOTE: there's a ... after the `imports`, because linked compilation produces a trailing comma while full compilation doesn't.
39+
imports: [DuplicateLazyDep, OtherLazyDep]…
40+
}]
41+
}], null, null);
42+
});
43+
})();

0 commit comments

Comments
 (0)