Merge pull request #12753 from h4ad-forks/perf/use-factory-instead-of…

…-use-value perf: prefer use factory instead of use value when possible
nestjs · Dec 18, 2023 · d165eed · d165eed
2 parents 0885d4f + ae03ccc
commit d165eed
Show file tree

Hide file tree

Showing 10 changed files with 45 additions and 39 deletions.
diff --git a/integration/inspector/e2e/fixtures/post-init-graph.json b/integration/inspector/e2e/fixtures/post-init-graph.json
@@ -60,10 +60,10 @@
  "sourceModuleName": "InternalCoreModule",
  "durable": false,
  "static": true,
- "scope": 0,
  "transient": false,
  "exported": true,
- "token": "ExternalContextCreator"
+ "token": "ExternalContextCreator",
+ "initTime": 0
  }
  },
  "208171089": {
@@ -800,10 +800,10 @@
  "sourceModuleName": "InternalCoreModule",
  "durable": false,
  "static": true,
- "scope": 0,
  "transient": false,
  "exported": true,
- "token": "ModulesContainer"
+ "token": "ModulesContainer",
+ "initTime": 0
  }
  },
  "-326832201": {
@@ -816,10 +816,10 @@
  "sourceModuleName": "InternalCoreModule",
  "durable": false,
  "static": true,
- "scope": 0,
  "transient": false,
  "exported": true,
- "token": "HttpAdapterHost"
+ "token": "HttpAdapterHost",
+ "initTime": 0
  }
  },
  "-702581189": {
@@ -848,10 +848,10 @@
  "sourceModuleName": "InternalCoreModule",
  "durable": false,
  "static": true,
- "scope": 0,
  "transient": false,
  "exported": true,
- "token": "SerializedGraph"
+ "token": "SerializedGraph",
+ "initTime": 0
  }
  },
  "-1251270035": {

diff --git a/integration/inspector/e2e/fixtures/pre-init-graph.json b/integration/inspector/e2e/fixtures/pre-init-graph.json
@@ -60,10 +60,10 @@
  "sourceModuleName": "InternalCoreModule",
  "durable": false,
  "static": true,
- "scope": 0,
  "transient": false,
  "exported": true,
- "token": "ExternalContextCreator"
+ "token": "ExternalContextCreator",
+ "initTime": 0
  }
  },
  "208171089": {
@@ -784,10 +784,10 @@
  "sourceModuleName": "InternalCoreModule",
  "durable": false,
  "static": true,
- "scope": 0,
  "transient": false,
  "exported": true,
- "token": "ModulesContainer"
+ "token": "ModulesContainer",
+ "initTime": 0
  }
  },
  "-326832201": {
@@ -800,10 +800,10 @@
  "sourceModuleName": "InternalCoreModule",
  "durable": false,
  "static": true,
- "scope": 0,
  "transient": false,
  "exported": true,
- "token": "HttpAdapterHost"
+ "token": "HttpAdapterHost",
+ "initTime": 0
  }
  },
  "-702581189": {
@@ -832,10 +832,10 @@
  "sourceModuleName": "InternalCoreModule",
  "durable": false,
  "static": true,
- "scope": 0,
  "transient": false,
  "exported": true,
- "token": "SerializedGraph"
+ "token": "SerializedGraph",
+ "initTime": 0
  }
  },
  "-1251270035": {

diff --git a/package.json b/package.json
@@ -32,7 +32,7 @@
  "test:dev": "mocha -w --watch-files \"packages\" packages/**/*.spec.ts",
  "pretest:cov": "npm run clean",
  "test:cov": "nyc mocha packages/**/*.spec.ts --reporter spec",
- "test:integration": "mocha \"integration/*/{,!(node_modules)/**/}/*.spec.ts\"",
+ "test:integration": "mocha --reporter-option maxDiffSize=0 \"integration/*/{,!(node_modules)/**/}/*.spec.ts\"",
  "test:docker:up": "docker-compose -f integration/docker-compose.yml up -d",
  "test:docker:down": "docker-compose -f integration/docker-compose.yml down",
  "lint": "concurrently 'npm run lint:packages' 'npm run lint:integration' 'npm run lint:spec'",

diff --git a/packages/common/module-utils/configurable-module.builder.ts b/packages/common/module-utils/configurable-module.builder.ts
@@ -209,7 +209,7 @@ export class ConfigurableModuleBuilder<
  const providers: Array<Provider> = [
  {
  provide: self.options.optionsInjectionToken,
- useValue: this.omitExtras(options, self.extras),
+ useFactory: () => this.omitExtras(options, self.extras),
  },
  ];
  if (self.options.alwaysTransient) {

diff --git a/packages/core/injector/internal-core-module/internal-core-module-factory.ts b/packages/core/injector/internal-core-module/internal-core-module-factory.ts
@@ -45,23 +45,23 @@ export class InternalCoreModuleFactory {
  return InternalCoreModule.register([
  {
  provide: ExternalContextCreator,
- useValue: ExternalContextCreator.fromContainer(container),
+ useFactory: () => ExternalContextCreator.fromContainer(container),
  },
  {
  provide: ModulesContainer,
- useValue: container.getModules(),
+ useFactory: () => container.getModules(),
  },
  {
  provide: HttpAdapterHost,
- useValue: httpAdapterHost,
+ useFactory: () => httpAdapterHost,
  },
  {
  provide: LazyModuleLoader,
  useFactory: lazyModuleLoaderFactory,
  },
  {
  provide: SerializedGraph,
- useValue: container.serializedGraph,
+ useFactory: () => container.serializedGraph,
  },
  ]);
  }

diff --git a/packages/core/test/router/router-module.spec.ts b/packages/core/test/router/router-module.spec.ts
@@ -7,6 +7,7 @@ import {
  ROUTES,
  targetModulesByContainer,
 } from '../../router/router-module';
+import { FactoryProvider } from '@nestjs/common';
 
 class TestModuleClass {}
 

diff --git a/packages/microservices/module/clients.module.ts b/packages/microservices/module/clients.module.ts
@@ -17,10 +17,13 @@ import {
 export class ClientsModule {
  static register(options: ClientsModuleOptions): DynamicModule {
  const clientsOptions = !Array.isArray(options) ? options.clients : options;
- const clients = (clientsOptions || []).map(item => ({
- provide: item.name,
- useValue: this.assignOnAppShutdownHook(ClientProxyFactory.create(item)),
- }));
+ const clients = (clientsOptions || []).map(item => {
+ return {
+ provide: item.name,
+ useFactory: () =>
+ this.assignOnAppShutdownHook(ClientProxyFactory.create(item)),
+ };
+ });
  return {
  module: ClientsModule,
  global: !Array.isArray(options) && options.isGlobal,

diff --git a/packages/microservices/test/module/clients.module.spec.ts b/packages/microservices/test/module/clients.module.spec.ts
@@ -21,14 +21,13 @@ describe('ClientsModule', () => {
  expect(dynamicModule.module).to.be.eql(ClientsModule);
  });
  it('should return an expected providers array', () => {
- expect(dynamicModule.providers).to.be.deep.eq([
- {
- provide: 'test',
- useValue: ClientsModule['assignOnAppShutdownHook'](
- ClientProxyFactory.create({}),
- ),
- },
- ]);
+ const provider = dynamicModule.providers.find(
+ p => 'useFactory' in p && p.provide === 'test',
+ ) as FactoryProvider;
+ expect(provider).to.not.be.undefined;
+ expect(provider.useFactory()).to.be.deep.eq(
+ ClientsModule['assignOnAppShutdownHook'](ClientProxyFactory.create({})),
+ );
  });
  });
  describe('registerAsync', () => {

diff --git a/packages/platform-express/multer/multer.module.ts b/packages/platform-express/multer/multer.module.ts
@@ -17,7 +17,7 @@ export class MulterModule {
  return {
  module: MulterModule,
  providers: [
- { provide: MULTER_MODULE_OPTIONS, useValue: options },
+ { provide: MULTER_MODULE_OPTIONS, useFactory: () => options },
  {
  provide: MULTER_MODULE_ID,
  useValue: randomStringGenerator(),

diff --git a/packages/platform-express/test/multer/multer/multer.module.spec.ts b/packages/platform-express/test/multer/multer/multer.module.spec.ts
@@ -2,6 +2,7 @@ import { expect } from 'chai';
 import * as sinon from 'sinon';
 import { MULTER_MODULE_OPTIONS } from '../../../multer/files.constants';
 import { MulterModule } from '../../../multer/multer.module';
+import { FactoryProvider } from '@nestjs/common';
 
 describe('MulterModule', () => {
  describe('register', () => {
@@ -14,10 +15,12 @@ describe('MulterModule', () => {
  expect(dynamicModule.providers).to.have.length(2);
  expect(dynamicModule.imports).to.be.undefined;
  expect(dynamicModule.exports).to.include(MULTER_MODULE_OPTIONS);
- expect(dynamicModule.providers).to.deep.include({
- provide: MULTER_MODULE_OPTIONS,
- useValue: options,
- });
+
+ const moduleOptionsProvider = dynamicModule.providers.find(
+ p => 'useFactory' in p && p.provide === MULTER_MODULE_OPTIONS,
+ ) as FactoryProvider;
+ expect(moduleOptionsProvider).to.not.be.undefined;
+ expect(moduleOptionsProvider.useFactory()).to.be.eq(options);
  });
  });