Skip to content

Commit 873e80b

Browse files
pmvaldpkozlowski-opensource
pmvald
authored and
pkozlowski-opensource
committed
refactor(core): fix NgModule compilation scope in deps tracker (#51791)
Currently deps tracker includes the exported scope of the exported NgModule only in the exported scope of that NgModule. This is in agreement with what AoT does today. But JIT diverges from this behavior by including these exported scopes into the compilation scope as well. Since deps tracker is going to be used for both AoT (local compilation mode) and JIT, the question might be which behavior the deps tracker should follow? Today it follows the AoT one, but it breaks some tests in Google which seem to depend on this behavior of JIT. So it is better to migrate deps tracker to what JIT does. This leads to a wider compilation scope in local compilation compared to full compilations, but it won't break any existing thing. PR Close #51791
1 parent 3d06a81 commit 873e80b

File tree

2 files changed

+47
-40
lines changed

2 files changed

+47
-40
lines changed

‎packages/core/src/render3/deps_tracker/deps_tracker.ts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -209,6 +209,12 @@ class DepsTracker implements DepsTrackerApi {
209209
addSet(exportedScope.exported.directives, scope.exported.directives);
210210
addSet(exportedScope.exported.pipes, scope.exported.pipes);
211211

212+
// Some test toolings which run in JIT mode depend on this behavior that the exported scope
213+
// should also be present in the compilation scope, even though AoT does not support this
214+
// and it is also in odds with NgModule metadata definitions. Without this some tests in
215+
// Google will fail.
216+
addSet(exportedScope.exported.directives, scope.compilation.directives);
217+
addSet(exportedScope.exported.pipes, scope.compilation.pipes);
212218
} else if (isPipe(exported)) {
213219
scope.exported.pipes.add(exported);
214220
} else {

‎packages/core/test/render3/deps_tracker_spec.ts‎

Lines changed: 41 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -76,45 +76,46 @@ describe('runtime dependency tracker', () => {
7676
});
7777
});
7878

79-
it('should include the exported scope of an exported module in the exported scope', () => {
80-
@Directive({})
81-
class Directive1 {
82-
}
79+
it('should include the exported scope of an exported module in the exported scope and compilation scope',
80+
() => {
81+
@Directive({})
82+
class Directive1 {
83+
}
8384

84-
@Pipe({name: 'pipe1'})
85-
class Pipe1 {
86-
}
85+
@Pipe({name: 'pipe1'})
86+
class Pipe1 {
87+
}
8788

88-
@Component({})
89-
class Component1 {
90-
}
89+
@Component({})
90+
class Component1 {
91+
}
9192

92-
@NgModule({
93-
exports: [Directive1, Pipe1, Component1],
94-
})
95-
class SubModule {
96-
}
93+
@NgModule({
94+
exports: [Directive1, Pipe1, Component1],
95+
})
96+
class SubModule {
97+
}
9798

98-
@NgModule({
99-
exports: [SubModule],
100-
})
101-
class MainModule {
102-
}
99+
@NgModule({
100+
exports: [SubModule],
101+
})
102+
class MainModule {
103+
}
103104

104-
const ans = depsTracker.getNgModuleScope(MainModule as NgModuleType);
105+
const ans = depsTracker.getNgModuleScope(MainModule as NgModuleType);
105106

106-
expect(ans.exported).toEqual({
107-
pipes: new Set([Pipe1]),
108-
directives: new Set([Directive1, Component1]),
109-
});
107+
expect(ans.exported).toEqual({
108+
pipes: new Set([Pipe1]),
109+
directives: new Set([Directive1, Component1]),
110+
});
110111

111-
expect(ans.compilation).toEqual({
112-
pipes: new Set(),
113-
directives: new Set(),
114-
});
115-
});
112+
expect(ans.compilation).toEqual({
113+
pipes: new Set([Pipe1]),
114+
directives: new Set([Directive1, Component1]),
115+
});
116+
});
116117

117-
it('should combine the directly exported elements with the exported scope of exported module',
118+
it('should combine the directly exported elements with the exported scope of exported module in both exported and compilation scopes',
118119
() => {
119120
@Directive({})
120121
class Directive1 {
@@ -139,7 +140,7 @@ describe('runtime dependency tracker', () => {
139140
}
140141

141142
@NgModule({
142-
exports: [SubModule, MainComponent, Directive1, Pipe1, Component1],
143+
exports: [SubModule, MainComponent],
143144
})
144145
class MainModule {
145146
}
@@ -152,8 +153,8 @@ describe('runtime dependency tracker', () => {
152153
});
153154

154155
expect(ans.compilation).toEqual({
155-
pipes: new Set(),
156-
directives: new Set(),
156+
pipes: new Set([Pipe1]),
157+
directives: new Set([Directive1, Component1]),
157158
});
158159
});
159160
});
@@ -620,7 +621,7 @@ describe('runtime dependency tracker', () => {
620621
});
621622
});
622623

623-
it('should include the exported scope of an exported forward ref module in the exported scope when compiling in JIT mode',
624+
it('should include the exported scope of an exported forward ref module in the exported and compilation scope when compiling in JIT mode',
624625
() => {
625626
@NgModule({exports: [forwardRef(() => SubModule)]})
626627
class MainModule {
@@ -645,16 +646,16 @@ describe('runtime dependency tracker', () => {
645646
const ans = depsTracker.getNgModuleScope(MainModule as NgModuleType);
646647

647648
expect(ans.compilation).toEqual({
648-
pipes: new Set(),
649-
directives: new Set(),
649+
pipes: new Set([Pipe1]),
650+
directives: new Set([Component1, Directive1]),
650651
});
651652
expect(ans.exported).toEqual({
652653
pipes: new Set([Pipe1]),
653654
directives: new Set([Component1, Directive1]),
654655
});
655656
});
656657

657-
it('should include the exported scope of an exported forward ref module in the exported scope when compiling in AOT mode',
658+
it('should include the exported scope of an exported forward ref module in the exported and compilation scopes when compiling in AOT mode',
658659
() => {
659660
class MainModule {}
660661
(MainModule as NgModuleType).ɵmod = createNgModuleDef({exports: () => ([SubModule])});
@@ -678,8 +679,8 @@ describe('runtime dependency tracker', () => {
678679
const ans = depsTracker.getNgModuleScope(MainModule as NgModuleType);
679680

680681
expect(ans.compilation).toEqual({
681-
pipes: new Set(),
682-
directives: new Set(),
682+
pipes: new Set([Pipe1]),
683+
directives: new Set([Component1, Directive1]),
683684
});
684685
expect(ans.exported).toEqual({
685686
pipes: new Set([Pipe1]),

0 commit comments

Comments
 (0)