fixup! perf: prefer use factory instead of use value when possible

nestjs · Nov 16, 2023 · 8a2d19d · 8a2d19d
1 parent 53d2094
commit 8a2d19d
Show file tree

Hide file tree

Showing 8 changed files with 31 additions and 21 deletions.
diff --git a/packages/common/module-utils/configurable-module.builder.ts b/packages/common/module-utils/configurable-module.builder.ts
@@ -209,6 +209,8 @@ export class ConfigurableModuleBuilder<
         const providers: Array<Provider> = [
           {
             provide: self.options.optionsInjectionToken,
+            // useFactory is for performance reasons
+            // see more: https://github.com/nestjs/nest/issues/12738#issuecomment-1810987001
             useFactory: () => this.omitExtras(options, self.extras),
           },
         ];

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
@@ -42,6 +42,8 @@ export class InternalCoreModuleFactory {
       );
     };
 
+    // useFactory is for performance reasons
+    // see more: https://github.com/nestjs/nest/issues/12738#issuecomment-1810987001
     return InternalCoreModule.register([
       {
         provide: ExternalContextCreator,

diff --git a/packages/core/router/router-module.ts b/packages/core/router/router-module.ts
@@ -32,6 +32,8 @@ export class RouterModule {
       providers: [
         {
           provide: ROUTES,
+          // useFactory is for performance reasons
+          // see more: https://github.com/nestjs/nest/issues/12738#issuecomment-1810987001
           useFactory: () => routes,
         },
       ],

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 {}
 
@@ -15,15 +16,12 @@ describe('RouterModule', () => {
 
   describe('register', () => {
     it('should return a dynamic module with routes registered as a provider', () => {
-      expect(RouterModule.register(routes)).to.deep.equal({
-        module: RouterModule,
-        providers: [
-          {
-            provide: ROUTES,
-            useValue: routes,
-          },
-        ],
-      });
+      const moduleRegistered = RouterModule.register(routes);
+      const provider = moduleRegistered.providers.find(
+        p => 'useFactory' in p && p.provide === ROUTES,
+      ) as FactoryProvider;
+      expect(provider).to.not.be.undefined;
+      expect(provider.useFactory()).to.be.eq(routes);
     });
   });
   describe('when instantiated', () => {

diff --git a/packages/microservices/module/clients.module.ts b/packages/microservices/module/clients.module.ts
@@ -19,6 +19,8 @@ export class ClientsModule {
     const clientsOptions = !Array.isArray(options) ? options.clients : options;
     const clients = (clientsOptions || []).map(item => ({
       provide: item.name,
+      // useFactory is for performance reasons
+      // see more: https://github.com/nestjs/nest/issues/12738#issuecomment-1810987001
       useFactory: () =>
         this.assignOnAppShutdownHook(ClientProxyFactory.create(item)),
     }));

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,6 +17,8 @@ export class MulterModule {
     return {
       module: MulterModule,
       providers: [
+        // useFactory is for performance reasons
+        // see more: https://github.com/nestjs/nest/issues/12738#issuecomment-1810987001
         { provide: MULTER_MODULE_OPTIONS, useFactory: () => options },
         {
           provide: MULTER_MODULE_ID,

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