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);
     });
   });