Skip to content

Commit 60b2ed4

Browse files
kamilmysliwieckamilmysliwiec
authored
Merge pull request #15504 from hajekjiri/bug/fix-undefined-injection
fix(core): fix race condition in class dependency resolution from imported modules
2 parents 059fe01 + ab93e4b commit 60b2ed4

File tree

5 files changed

+113
-146
lines changed

5 files changed

+113
-146
lines changed

‎integration/injector/e2e/many-global-modules.spec.ts‎

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,21 +49,42 @@ class TransientProvider {}
4949
@Injectable()
5050
class RequestProvider {}
5151

52+
@Injectable()
53+
class ForeignTransientProvider {}
54+
5255
@Injectable()
5356
export class Dependant {
5457
constructor(
5558
private readonly transientProvider: TransientProvider,
5659

60+
private readonly foreignTransientProvider: ForeignTransientProvider,
61+
5762
@Inject(RequestProvider)
5863
private readonly requestProvider: RequestProvider,
5964
) {}
6065

6166
public checkDependencies() {
6267
expect(this.transientProvider).to.be.instanceOf(TransientProvider);
68+
expect(this.foreignTransientProvider).to.be.instanceOf(
69+
ForeignTransientProvider,
70+
);
6371
expect(this.requestProvider).to.be.instanceOf(RequestProvider);
6472
}
6573
}
6674

75+
@Global()
76+
@Module({
77+
providers: [
78+
{
79+
provide: ForeignTransientProvider,
80+
scope: Scope.TRANSIENT,
81+
useClass: ForeignTransientProvider,
82+
},
83+
],
84+
exports: [ForeignTransientProvider],
85+
})
86+
export class ModuleWithForeignTransientProvider {}
87+
6788
@Global()
6889
@Module({
6990
providers: [
@@ -98,6 +119,12 @@ export class GlobalModuleWithRequestProvider {}
98119

99120
@Module({
100121
imports: [
122+
/*
123+
* ForeginTransientProvider will be resolved quickly because its host module is imported first.
124+
* IMPORTANT: Do not move this module, otherwise we may not catch future regressions.
125+
*/
126+
ModuleWithForeignTransientProvider,
127+
101128
GlobalModule1,
102129
GlobalModule2,
103130
GlobalModule3,
@@ -115,7 +142,7 @@ export class GlobalModuleWithRequestProvider {}
115142
export class AppModule {}
116143

117144
describe('Many global modules', () => {
118-
it('should inject request-scoped useFactory provider and transient-scoped useClass provider from different modules', async () => {
145+
it('should inject request-scoped and transient-scoped providers from different modules', async () => {
119146
const moduleBuilder = Test.createTestingModule({
120147
imports: [AppModule],
121148
});

‎integration/inspector/e2e/fixtures/post-init-graph.json‎

Lines changed: 32 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -2263,6 +2263,22 @@
22632263
},
22642264
"id": "-1303681274"
22652265
},
2266+
"-831049991": {
2267+
"source": "594986539",
2268+
"target": "-1721730431",
2269+
"metadata": {
2270+
"type": "class-to-class",
2271+
"sourceModuleName": "InputModule",
2272+
"sourceClassName": "InputService",
2273+
"targetClassName": "CircularService",
2274+
"sourceClassToken": "InputService",
2275+
"targetClassToken": "CircularService",
2276+
"targetModuleName": "CircularModule",
2277+
"keyOrIndex": 0,
2278+
"injectionType": "constructor"
2279+
},
2280+
"id": "-831049991"
2281+
},
22662282
"-886102564": {
22672283
"source": "208171089",
22682284
"target": "671882984",
@@ -2280,6 +2296,22 @@
22802296
},
22812297
"id": "-886102564"
22822298
},
2299+
"-2146943494": {
2300+
"source": "-234035039",
2301+
"target": "928565345",
2302+
"metadata": {
2303+
"type": "class-to-class",
2304+
"sourceModuleName": "RequestChainModule",
2305+
"sourceClassName": "RequestChainService",
2306+
"targetClassName": "HelperService",
2307+
"sourceClassToken": "RequestChainService",
2308+
"targetClassToken": "HelperService",
2309+
"targetModuleName": "HelperModule",
2310+
"keyOrIndex": 0,
2311+
"injectionType": "constructor"
2312+
},
2313+
"id": "-2146943494"
2314+
},
22832315
"-2003045613": {
22842316
"source": "-377928898",
22852317
"target": "-616397055",
@@ -2312,38 +2344,6 @@
23122344
},
23132345
"id": "-881420795"
23142346
},
2315-
"-831049991": {
2316-
"source": "594986539",
2317-
"target": "-1721730431",
2318-
"metadata": {
2319-
"type": "class-to-class",
2320-
"sourceModuleName": "InputModule",
2321-
"sourceClassName": "InputService",
2322-
"targetClassName": "CircularService",
2323-
"sourceClassToken": "InputService",
2324-
"targetClassToken": "CircularService",
2325-
"targetModuleName": "CircularModule",
2326-
"keyOrIndex": 0,
2327-
"injectionType": "constructor"
2328-
},
2329-
"id": "-831049991"
2330-
},
2331-
"-2146943494": {
2332-
"source": "-234035039",
2333-
"target": "928565345",
2334-
"metadata": {
2335-
"type": "class-to-class",
2336-
"sourceModuleName": "RequestChainModule",
2337-
"sourceClassName": "RequestChainService",
2338-
"targetClassName": "HelperService",
2339-
"sourceClassToken": "RequestChainService",
2340-
"targetClassToken": "HelperService",
2341-
"targetModuleName": "HelperModule",
2342-
"keyOrIndex": 0,
2343-
"injectionType": "constructor"
2344-
},
2345-
"id": "-2146943494"
2346-
},
23472347
"-1816180282": {
23482348
"source": "-848516688",
23492349
"target": "-1673986099",

‎integration/inspector/e2e/fixtures/pre-init-graph.json‎

Lines changed: 48 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -2181,22 +2181,6 @@
21812181
},
21822182
"id": "1976848738"
21832183
},
2184-
"-2105726668": {
2185-
"source": "-1803759743",
2186-
"target": "1010833816",
2187-
"metadata": {
2188-
"type": "class-to-class",
2189-
"sourceModuleName": "PropertiesModule",
2190-
"sourceClassName": "PropertiesService",
2191-
"targetClassName": "token",
2192-
"sourceClassToken": "PropertiesService",
2193-
"targetClassToken": "token",
2194-
"targetModuleName": "PropertiesModule",
2195-
"keyOrIndex": "token",
2196-
"injectionType": "property"
2197-
},
2198-
"id": "-2105726668"
2199-
},
22002184
"-21463590": {
22012185
"source": "-1378706112",
22022186
"target": "1004276345",
@@ -2213,6 +2197,22 @@
22132197
},
22142198
"id": "-21463590"
22152199
},
2200+
"-2105726668": {
2201+
"source": "-1803759743",
2202+
"target": "1010833816",
2203+
"metadata": {
2204+
"type": "class-to-class",
2205+
"sourceModuleName": "PropertiesModule",
2206+
"sourceClassName": "PropertiesService",
2207+
"targetClassName": "token",
2208+
"sourceClassToken": "PropertiesService",
2209+
"targetClassToken": "token",
2210+
"targetModuleName": "PropertiesModule",
2211+
"keyOrIndex": "token",
2212+
"injectionType": "property"
2213+
},
2214+
"id": "-2105726668"
2215+
},
22162216
"-1657371464": {
22172217
"source": "-1673986099",
22182218
"target": "1919157847",
@@ -2247,6 +2247,22 @@
22472247
},
22482248
"id": "-1303681274"
22492249
},
2250+
"-831049991": {
2251+
"source": "594986539",
2252+
"target": "-1721730431",
2253+
"metadata": {
2254+
"type": "class-to-class",
2255+
"sourceModuleName": "InputModule",
2256+
"sourceClassName": "InputService",
2257+
"targetClassName": "CircularService",
2258+
"sourceClassToken": "InputService",
2259+
"targetClassToken": "CircularService",
2260+
"targetModuleName": "CircularModule",
2261+
"keyOrIndex": 0,
2262+
"injectionType": "constructor"
2263+
},
2264+
"id": "-831049991"
2265+
},
22502266
"-886102564": {
22512267
"source": "208171089",
22522268
"target": "671882984",
@@ -2264,6 +2280,22 @@
22642280
},
22652281
"id": "-886102564"
22662282
},
2283+
"-2146943494": {
2284+
"source": "-234035039",
2285+
"target": "928565345",
2286+
"metadata": {
2287+
"type": "class-to-class",
2288+
"sourceModuleName": "RequestChainModule",
2289+
"sourceClassName": "RequestChainService",
2290+
"targetClassName": "HelperService",
2291+
"sourceClassToken": "RequestChainService",
2292+
"targetClassToken": "HelperService",
2293+
"targetModuleName": "HelperModule",
2294+
"keyOrIndex": 0,
2295+
"injectionType": "constructor"
2296+
},
2297+
"id": "-2146943494"
2298+
},
22672299
"-2003045613": {
22682300
"source": "-377928898",
22692301
"target": "-616397055",
@@ -2296,38 +2328,6 @@
22962328
},
22972329
"id": "-881420795"
22982330
},
2299-
"-831049991": {
2300-
"source": "594986539",
2301-
"target": "-1721730431",
2302-
"metadata": {
2303-
"type": "class-to-class",
2304-
"sourceModuleName": "InputModule",
2305-
"sourceClassName": "InputService",
2306-
"targetClassName": "CircularService",
2307-
"sourceClassToken": "InputService",
2308-
"targetClassToken": "CircularService",
2309-
"targetModuleName": "CircularModule",
2310-
"keyOrIndex": 0,
2311-
"injectionType": "constructor"
2312-
},
2313-
"id": "-831049991"
2314-
},
2315-
"-2146943494": {
2316-
"source": "-234035039",
2317-
"target": "928565345",
2318-
"metadata": {
2319-
"type": "class-to-class",
2320-
"sourceModuleName": "RequestChainModule",
2321-
"sourceClassName": "RequestChainService",
2322-
"targetClassName": "HelperService",
2323-
"sourceClassToken": "RequestChainService",
2324-
"targetClassToken": "HelperService",
2325-
"targetModuleName": "HelperModule",
2326-
"keyOrIndex": 0,
2327-
"injectionType": "constructor"
2328-
},
2329-
"id": "-2146943494"
2330-
},
23312331
"-1816180282": {
23322332
"source": "-848516688",
23332333
"target": "-1673986099",

‎packages/core/injector/injector.ts‎

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -682,14 +682,11 @@ export class Injector {
682682
inquirerId,
683683
);
684684
if (!instanceHost.isResolved && !instanceWrapperRef.forwardRef) {
685-
wrapper.settlementSignal?.insertRef(instanceWrapperRef.id);
686-
687-
await this.loadProvider(
688-
instanceWrapperRef,
689-
relatedModule,
690-
contextId,
691-
wrapper,
692-
);
685+
/*
686+
* Provider will be loaded shortly in resolveComponentHost() once we pass the current
687+
* Barrier. We cannot load it here because doing so could incorrectly evaluate the
688+
* staticity of the dependency tree and lead to undefined / null injection.
689+
*/
693690
break;
694691
}
695692
}

‎packages/core/test/injector/injector.spec.ts‎

Lines changed: 0 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -447,63 +447,6 @@ describe('Injector', () => {
447447
),
448448
).to.eventually.be.eq(null);
449449
});
450-
451-
it('should call "loadProvider" when component is not resolved', async () => {
452-
const moduleFixture = {
453-
imports: new Map([
454-
[
455-
'key',
456-
{
457-
providers: {
458-
has: () => true,
459-
get: () =>
460-
new InstanceWrapper({
461-
isResolved: false,
462-
}),
463-
},
464-
exports: {
465-
has: () => true,
466-
},
467-
imports: new Map(),
468-
},
469-
],
470-
] as any),
471-
};
472-
await injector.lookupComponentInImports(
473-
moduleFixture as any,
474-
metatype as any,
475-
new InstanceWrapper(),
476-
);
477-
expect(loadProvider.called).to.be.true;
478-
});
479-
480-
it('should not call "loadProvider" when component is resolved', async () => {
481-
const moduleFixture = {
482-
relatedModules: new Map([
483-
[
484-
'key',
485-
{
486-
providers: {
487-
has: () => true,
488-
get: () => ({
489-
isResolved: true,
490-
}),
491-
},
492-
exports: {
493-
has: () => true,
494-
},
495-
relatedModules: new Map(),
496-
},
497-
],
498-
] as any),
499-
};
500-
await injector.lookupComponentInImports(
501-
moduleFixture as any,
502-
metatype as any,
503-
null!,
504-
);
505-
expect(loadProvider.called).to.be.false;
506-
});
507450
});
508451

509452
describe('resolveParamToken', () => {

0 commit comments

Comments
 (0)