Skip to content

Commit

Permalink
Merge pull request #12035 from nestjs/fix/globals-in-lazy-modules
Browse files Browse the repository at this point in the history
fix(core): allow importing providers from global modules in lazy modules
  • Loading branch information
kamilmysliwiec authored Jul 17, 2023
2 parents 0c1fc1b + 2545a4c commit 33b0aaa
Show file tree
Hide file tree
Showing 14 changed files with 195 additions and 46 deletions.
29 changes: 29 additions & 0 deletions integration/lazy-modules/e2e/lazy-import-global-modules.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
import { INestApplication } from '@nestjs/common';
import { Test } from '@nestjs/testing';
import * as chai from 'chai';
import { expect } from 'chai';
import chaiAsPromised = require('chai-as-promised');
import { AppModule } from '../src/app.module';
chai.use(chaiAsPromised);

describe('Lazy imports', () => {
let server;
let app: INestApplication;

beforeEach(async () => {
const module = await Test.createTestingModule({
imports: [AppModule],
}).compile();

app = module.createNestApplication();
server = app.getHttpAdapter().getInstance();
});

it(`should allow imports of global modules`, async () => {
await expect(app.init()).to.eventually.be.fulfilled;
});

afterEach(async () => {
await app.close();
});
});
16 changes: 16 additions & 0 deletions integration/lazy-modules/src/app.module.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
import { Module } from '@nestjs/common';
import { LazyModuleLoader } from '@nestjs/core';
import { EagerModule } from './eager.module';
import { GlobalModule } from './global.module';
import { LazyModule } from './lazy.module';

@Module({
imports: [GlobalModule, EagerModule],
})
export class AppModule {
constructor(public loader: LazyModuleLoader) {}

async onApplicationBootstrap() {
await this.loader.load(() => LazyModule);
}
}
12 changes: 12 additions & 0 deletions integration/lazy-modules/src/eager.module.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import { Module, Injectable } from '@nestjs/common';
import { GlobalService } from './global.module';

@Injectable()
export class EagerService {
constructor(public globalService: GlobalService) {}
}

@Module({
providers: [EagerService],
})
export class EagerModule {}
13 changes: 13 additions & 0 deletions integration/lazy-modules/src/global.module.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import { Module, Injectable, Global } from '@nestjs/common';

@Injectable()
export class GlobalService {
constructor() {}
}

@Global()
@Module({
providers: [GlobalService],
exports: [GlobalService],
})
export class GlobalModule {}
12 changes: 12 additions & 0 deletions integration/lazy-modules/src/lazy.module.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import { Module, Injectable } from '@nestjs/common';
import { GlobalService } from './global.module';

@Injectable()
export class LazyService {
constructor(public globalService: GlobalService) {}
}

@Module({
providers: [LazyService],
})
export class LazyModule {}
8 changes: 8 additions & 0 deletions integration/lazy-modules/src/main.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import { NestFactory } from '@nestjs/core';
import { AppModule } from './app.module';

async function bootstrap() {
const app = await NestFactory.create(AppModule);
await app.listen(3000);
}
bootstrap();
59 changes: 40 additions & 19 deletions packages/core/injector/container.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,13 @@ export class NestContainer {
public async addModule(
metatype: ModuleMetatype,
scope: ModuleScope,
): Promise<Module | undefined> {
): Promise<
| {
moduleRef: Module;
inserted: boolean;
}
| undefined
> {
// In DependenciesScanner#scanForModules we already check for undefined or invalid modules
// We still need to catch the edge-case of `forwardRef(() => undefined)`
if (!metatype) {
Expand All @@ -80,24 +86,36 @@ export class NestContainer {
metatype,
);
if (this.modules.has(token)) {
return this.modules.get(token);
return {
moduleRef: this.modules.get(token),
inserted: true,
};
}

return this.setModule(
{
token,
type,
dynamicMetadata,
},
scope,
);
return {
moduleRef: await this.setModule(
{
token,
type,
dynamicMetadata,
},
scope,
),
inserted: true,
};
}

public async replaceModule(
metatypeToReplace: ModuleMetatype,
newMetatype: ModuleMetatype,
scope: ModuleScope,
): Promise<Module | undefined> {
): Promise<
| {
moduleRef: Module;
inserted: boolean;
}
| undefined
> {
// In DependenciesScanner#scanForModules we already check for undefined or invalid modules
// We still need to catch the edge-case of `forwardRef(() => undefined)`
if (!metatypeToReplace || !newMetatype) {
Expand All @@ -109,14 +127,17 @@ export class NestContainer {
newMetatype,
);

return this.setModule(
{
token,
type,
dynamicMetadata,
},
scope,
);
return {
moduleRef: await this.setModule(
{
token,
type,
dynamicMetadata,
},
scope,
),
inserted: false,
};
}

private async setModule(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ export class LazyModuleLoader {
const moduleInstances = await this.dependenciesScanner.scanForModules({
moduleDefinition: moduleClassOrDynamicDefinition,
overrides: this.moduleOverrides,
lazy: true,
});
if (moduleInstances.length === 0) {
// The module has been loaded already. In this case, we must
Expand Down
43 changes: 34 additions & 9 deletions packages/core/scanner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@ import { DynamicModule, ForwardReference, Provider } from '@nestjs/common';
import {
CATCH_WATERMARK,
CONTROLLER_WATERMARK,
EnhancerSubtype,
ENHANCER_KEY_TO_SUBTYPE_MAP,
EXCEPTION_FILTERS_METADATA,
EnhancerSubtype,
GUARDS_METADATA,
INJECTABLE_WATERMARK,
INTERCEPTORS_METADATA,
Expand Down Expand Up @@ -68,6 +68,7 @@ interface ModulesScanParameters {
scope?: Type<unknown>[];
ctxRegistry?: (ForwardReference | DynamicModule | Type<unknown>)[];
overrides?: ModuleOverride[];
lazy?: boolean;
}

export class DependenciesScanner {
Expand Down Expand Up @@ -99,23 +100,24 @@ export class DependenciesScanner {

public async scanForModules({
moduleDefinition,
lazy,
scope = [],
ctxRegistry = [],
overrides = [],
}: ModulesScanParameters): Promise<Module[]> {
const moduleInstance = await this.insertOrOverrideModule(
moduleDefinition,
overrides,
scope,
);
const { moduleRef: moduleInstance, inserted: moduleInserted } =
(await this.insertOrOverrideModule(moduleDefinition, overrides, scope)) ??
{};

moduleDefinition =
this.getOverrideModuleByModule(moduleDefinition, overrides)?.newModule ??
moduleDefinition;

moduleDefinition =
moduleDefinition instanceof Promise
? await moduleDefinition
: moduleDefinition;

ctxRegistry.push(moduleDefinition);

if (this.isForwardReference(moduleDefinition)) {
Expand Down Expand Up @@ -153,19 +155,30 @@ export class DependenciesScanner {
scope: [].concat(scope, moduleDefinition),
ctxRegistry,
overrides,
lazy,
});
registeredModuleRefs = registeredModuleRefs.concat(moduleRefs);
}
if (!moduleInstance) {
return registeredModuleRefs;
}

if (lazy && moduleInserted) {
this.container.bindGlobalsToImports(moduleInstance);
}
return [moduleInstance].concat(registeredModuleRefs);
}

public async insertModule(
moduleDefinition: any,
scope: Type<unknown>[],
): Promise<Module | undefined> {
): Promise<
| {
moduleRef: Module;
inserted: boolean;
}
| undefined
> {
const moduleToAdd = this.isForwardReference(moduleDefinition)
? moduleDefinition.forwardRef()
: moduleDefinition;
Expand Down Expand Up @@ -523,7 +536,13 @@ export class DependenciesScanner {
moduleDefinition: ModuleDefinition,
overrides: ModuleOverride[],
scope: Type<unknown>[],
): Promise<Module | undefined> {
): Promise<
| {
moduleRef: Module;
inserted: boolean;
}
| undefined
> {
const overrideModule = this.getOverrideModuleByModule(
moduleDefinition,
overrides,
Expand Down Expand Up @@ -563,7 +582,13 @@ export class DependenciesScanner {
moduleToOverride: ModuleDefinition,
newModule: ModuleDefinition,
scope: Type<unknown>[],
): Promise<Module | undefined> {
): Promise<
| {
moduleRef: Module;
inserted: boolean;
}
| undefined
> {
return this.container.replaceModule(
this.isForwardReference(moduleToOverride)
? moduleToOverride.forwardRef()
Expand Down
10 changes: 5 additions & 5 deletions packages/core/test/injector/injector.spec.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { Optional } from '@nestjs/common';
import { PARAMTYPES_METADATA } from '@nestjs/common/constants';
import * as chai from 'chai';
import { expect } from 'chai';
import * as chaiAsPromised from 'chai-as-promised';
Expand All @@ -10,7 +11,6 @@ import { NestContainer } from '../../injector/container';
import { Injector, PropertyDependency } from '../../injector/injector';
import { InstanceWrapper } from '../../injector/instance-wrapper';
import { Module } from '../../injector/module';
import { PARAMTYPES_METADATA } from '@nestjs/common/constants';

chai.use(chaiAsPromised);

Expand Down Expand Up @@ -710,17 +710,17 @@ describe('Injector', () => {
const container = new NestContainer();
const moduleCtor = class TestModule {};
const ctx = STATIC_CONTEXT;
const module = await container.addModule(moduleCtor, []);
const { moduleRef } = await container.addModule(moduleCtor, []);

module.addProvider({
moduleRef.addProvider({
provide: TestClass,
useClass: TestClass,
});

const instance = await injector.loadPerContext(
new TestClass(),
module,
module.providers,
moduleRef,
moduleRef.providers,
ctx,
);
expect(instance).to.be.instanceOf(TestClass);
Expand Down
2 changes: 1 addition & 1 deletion packages/core/test/inspector/graph-inspector.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ describe('GraphInspector', () => {
class RandomPipe {}

it('should inspect all modules', async () => {
const moduleRef = await container.addModule(TestModule, []);
const { moduleRef } = await container.addModule(TestModule, []);
moduleRef.addController(AController);

const subtype = 'interceptor';
Expand Down
8 changes: 4 additions & 4 deletions packages/core/test/nest-application-context.spec.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
import { InjectionToken, Scope } from '@nestjs/common';
import { expect } from 'chai';
import * as sinon from 'sinon';
import { ContextIdFactory } from '../helpers/context-id-factory';
import { NestContainer } from '../injector/container';
import { Injector } from '../injector/injector';
import { InstanceLoader } from '../injector/instance-loader';
import { GraphInspector } from '../inspector/graph-inspector';
import { NestApplicationContext } from '../nest-application-context';
import * as sinon from 'sinon';

describe('NestApplicationContext', () => {
class A {}
Expand All @@ -22,15 +22,15 @@ describe('NestApplicationContext', () => {
injector,
new GraphInspector(nestContainer),
);
const module = await nestContainer.addModule(class T {}, []);
const { moduleRef } = await nestContainer.addModule(class T {}, []);

nestContainer.addProvider(
{
provide: injectionKey,
useClass: A,
scope,
},
module.token,
moduleRef.token,
);

nestContainer.addInjectable(
Expand All @@ -39,7 +39,7 @@ describe('NestApplicationContext', () => {
useClass: A,
scope,
},
module.token,
moduleRef.token,
'interceptor',
);

Expand Down
Loading

0 comments on commit 33b0aaa

Please sign in to comment.