fix(reactant-module): fix DI performance (#137)

unadlib · Sep 1, 2024 · b1def5c · b1def5c
1 parent e1b7278
commit b1def5c
Show file tree

Hide file tree

Showing 9 changed files with 99 additions and 27 deletions.
diff --git a/packages/reactant-module/src/core/createStore.ts b/packages/reactant-module/src/core/createStore.ts
@@ -116,6 +116,32 @@ export function createStore<T = any>({
     }
   }
   const multipleInjectMap = getMetadata(METADATA_KEY.multiple);
+  // it's just a workaround for the issue of `ServiceIdentifiers` order.
+  // InversifyJS issue: https://github.com/inversify/InversifyJS/issues/1578
+  const allServiceIdentifiers = Array.from(ServiceIdentifiers);
+  allServiceIdentifiers.sort(([a], [b]) => {
+    let aDeps = [];
+    try {
+      aDeps = Reflect.getMetadata(METADATA_KEY.paramtypes, a) ?? [];
+    } catch (e) {
+      //
+    }
+    let bDeps = [];
+    try {
+      bDeps = Reflect.getMetadata(METADATA_KEY.paramtypes, b) ?? [];
+    } catch (e) {
+      //
+    }
+    return aDeps.length - bDeps.length;
+  });
+  ServiceIdentifiers.clear();
+  // ServiceIdentifiers is sorted by the number of dependencies.
+  // The purpose is to ensure that the module is loaded in the correct order.
+  // The module with fewer dependencies is loaded first.
+  // ServiceIdentifiers is a mutable object, so it needs to be redefined.
+  allServiceIdentifiers.forEach(([ServiceIdentifier, value]) => {
+    ServiceIdentifiers.set(ServiceIdentifier, value);
+  });
   for (const [ServiceIdentifier] of ServiceIdentifiers) {
     // `Service` should be bound before `createStore`.
     const isMultipleInjection = multipleInjectMap.has(ServiceIdentifier);

diff --git a/packages/reactant-module/test/core/getRef.test.ts b/packages/reactant-module/test/core/getRef.test.ts
@@ -117,16 +117,16 @@ test('getRef for base module', () => {
   expect(counter.count).toBe(0);
   expect(counter.string).toBe('test');
   expect(Object.values(store.getState())).toEqual([
-    { count: 0, others: { list: [] } },
     { list: [] },
+    { count: 0, others: { list: [] } },
   ]);
   counter.increase();
   expect(counter.count).toBe(1);
   counter.add();
   expect(counter.others.list).toEqual([0]);
   expect(Object.values(store.getState())).toEqual([
-    { count: 1, others: { list: [0] } },
     { list: [] },
+    { count: 1, others: { list: [0] } },
   ]);
   expect((counter as any)[initStateKey].count).toBe(0);
   expect(getRef(counter).initState!.count).toBe(0);
@@ -140,7 +140,6 @@ test('getRef for base module', () => {
       (name) => !/^@@reactant/.test(name)
     )
   ).toEqual([
-    'counter',
     'string',
     'number',
     'symbol',
@@ -150,5 +149,6 @@ test('getRef for base module', () => {
     'array',
     'set',
     'map',
+    'counter',
   ]);
 });
diff --git a/packages/reactant-module/test/decorators/computed.test.ts b/packages/reactant-module/test/decorators/computed.test.ts
@@ -74,7 +74,7 @@ describe('@computed', () => {
     });
     expect(computedFn.mock.calls.length).toBe(0);
     counter.increase();
-    expect(Object.values(store.getState())[0]).toEqual({ count: 1 });
+    expect(Object.values(store.getState())[1]).toEqual({ count: 1 });
     expect(counter.num).toBe(2);
     expect(computedFn.mock.calls.length).toBe(1);
     expect(counter.num).toBe(2);
@@ -977,7 +977,7 @@ describe('@computed with automatic dependencies collection', () => {
     expect(computedFn.mock.calls.length).toBe(0);
     expect(computedFn0.mock.calls.length).toBe(1);
     counter.increase();
-    expect(Object.values(store.getState())[0]).toEqual({ count: 1 });
+    expect(Object.values(store.getState())[1]).toEqual({ count: 1 });
     expect(counter.num).toBe(3);
     expect(computedFn.mock.calls.length).toBe(1);
     expect(computedFn0.mock.calls.length).toBe(2);
@@ -1093,7 +1093,7 @@ describe('@computed with automatic dependencies collection', () => {
     expect(computedFn0.mock.calls.length).toBe(1);
     expect(computedFn1.mock.calls.length).toBe(1);
     counter.increase();
-    expect(Object.values(store.getState())[0]).toEqual({ count: 1 });
+    expect(Object.values(store.getState())[1]).toEqual({ count: 1 });
     expect(counter.num).toBe(3);
     expect(computedFn.mock.calls.length).toBe(1);
     expect(computedFn0.mock.calls.length).toBe(2);
@@ -1229,7 +1229,7 @@ describe('@computed with automatic dependencies collection', () => {
     });
     expect(computedFn.mock.calls.length).toBe(0);
     counter.increase();
-    expect(Object.values(store.getState())[0]).toEqual({ count: 1 });
+    expect(Object.values(store.getState())[1]).toEqual({ count: 1 });
     expect(counter.num).toBe(3);
     expect(computedFn.mock.calls.length).toBe(1);
     expect(counter.num).toBe(3);
@@ -1309,7 +1309,7 @@ describe('@computed with automatic dependencies collection', () => {
     });
     expect(computedFn.mock.calls.length).toBe(0);
     counter.increase();
-    expect(Object.values(store.getState())[0]).toEqual({ count: 1 });
+    expect(Object.values(store.getState())[1]).toEqual({ count: 1 });
     expect(counter.num).toBe(2);
     expect(computedFn.mock.calls.length).toBe(1);
     expect(counter.num).toBe(2);
@@ -1384,7 +1384,7 @@ describe('@computed with automatic dependencies collection', () => {
     });
     expect(computedFn.mock.calls.length).toBe(0);
     counter.increase();
-    expect(Object.values(store.getState())[0]).toEqual({ count: 1 });
+    expect(Object.values(store.getState())[1]).toEqual({ count: 1 });
     expect(counter.num).toBe(2);
     expect(computedFn.mock.calls.length).toBe(1);
     expect(counter.num).toBe(2);
@@ -1887,7 +1887,7 @@ describe('@computed with automatic dependencies collection', () => {
     expect(computedFn0.mock.calls.length).toBe(0);
     expect(counter.num).toBe(1);
     counter.increase();
-    expect(Object.values(store.getState())[0]).toEqual({ count: 1 });
+    expect(Object.values(store.getState())[1]).toEqual({ count: 1 });
     expect(counter.num).toBe(2);
     expect(counter.counter0.num).toBe(1);
     expect(computedFn0.mock.calls.length).toBe(1);

diff --git a/packages/reactant-module/test/index.test.ts b/packages/reactant-module/test/index.test.ts
@@ -89,16 +89,16 @@ test('base module with @state and @action', () => {
   expect(counter.count).toBe(0);
   expect(counter.string).toBe('test');
   expect(Object.values(store.getState())).toEqual([
-    { count: 0, others: { list: [] } },
     { list: [] },
+    { count: 0, others: { list: [] } },
   ]);
   counter.increase();
   expect(counter.count).toBe(1);
   counter.add();
   expect(counter.others.list).toEqual([0]);
   expect(Object.values(store.getState())).toEqual([
-    { count: 1, others: { list: [0] } },
     { list: [] },
+    { count: 1, others: { list: [0] } },
   ]);
   expect((counter as any)[initStateKey].count).toBe(0);
 });

diff --git a/packages/reactant-module/test/utils/reduxDevToolsCompose.test.ts b/packages/reactant-module/test/utils/reduxDevToolsCompose.test.ts
@@ -86,16 +86,16 @@ test('base module for reduxDevToolsCompose', () => {
   expect(counter.count).toBe(0);
   expect(counter.string).toBe('test');
   expect(Object.values(store.getState())).toEqual([
-    { count: 0, others: { list: [] } },
     { list: [] },
+    { count: 0, others: { list: [] } },
   ]);
   counter.increase();
   expect(counter.count).toBe(1);
   counter.add();
   expect(counter.others.list).toEqual([0]);
   expect(Object.values(store.getState())).toEqual([
-    { count: 1, others: { list: [0] } },
     { list: [] },
+    { count: 1, others: { list: [0] } },
   ]);
   expect(fn.mock.calls.length).toBe(1);
   expect(JSON.parse(JSON.stringify(fn.mock.calls, null, 2))).toEqual([
@@ -212,15 +212,15 @@ test('base module with error for reduxDevToolsCompose', () => {
   expect(counter.count).toBe(0);
   expect(counter.string).toBe('test');
   expect(Object.values(store.getState())).toEqual([
-    { count: 0, others: { list: [] } },
     { list: [] },
+    { count: 0, others: { list: [] } },
   ]);
   counter.increase();
   expect(counter.count).toBe(1);
   counter.add();
   expect(counter.others.list).toEqual([0]);
   expect(Object.values(store.getState())).toEqual([
-    { count: 1, others: { list: [0] } },
     { list: [] },
+    { count: 1, others: { list: [0] } },
   ]);
 });
diff --git a/packages/reactant-router/src/router.tsx b/packages/reactant-router/src/router.tsx
@@ -1,7 +1,7 @@
 /* eslint-disable consistent-return */
 import React, { PropsWithChildren, FunctionComponent } from 'react';
 import { PluginModule, injectable, inject, storeKey } from 'reactant-module';
-import type { ReducersMapObject, Store } from 'redux';
+import type { ReducersMapObject } from 'redux';
 import {
   connectRouter,
   ConnectedRouter,
@@ -19,6 +19,7 @@ export {
 } from 'history';
 
 export { LOCATION_CHANGE } from 'connected-react-router';
+export type { LocationChangeAction } from 'connected-react-router';
 
 const RouterOptions = Symbol('RouterOptions');
 

diff --git a/packages/reactant-share/src/modules/router.ts b/packages/reactant-share/src/modules/router.ts
@@ -1,3 +1,4 @@
+/* eslint-disable prefer-destructuring */
 /* eslint-disable no-shadow */
 /* eslint-disable @typescript-eslint/no-explicit-any */
 /* eslint-disable consistent-return */
@@ -10,9 +11,14 @@ import {
   stateKey,
   modulesKey,
 } from 'reactant';
-import { Router as BaseReactantRouter, RouterOptions } from 'reactant-router';
+import {
+  Router as BaseReactantRouter,
+  LOCATION_CHANGE,
+  RouterOptions,
+} from 'reactant-router';
 import type {
   IRouterOptions as IBaseRouterOptions,
+  LocationChangeAction,
   RouterState,
 } from 'reactant-router';
 import type { LocationState } from 'history';
@@ -220,12 +226,21 @@ class ReactantRouter extends BaseReactantRouter {
   }
 
   watchRehydratedRouting() {
-    const stopWatching = watch(
+    // The first rendering and the hydration of the persistent router may emit actions in a different order due to the module order.
+    let firstTrigger = false;
+    const stopWatchingRehydrated = watch(
       this,
       () => (this as any)[stateKey]._persist?.rehydrated,
       (rehydrated) => {
+        if (!this.enableCacheRouting) {
+          stopWatchingRehydrated();
+        }
         if (rehydrated) {
-          stopWatching();
+          stopWatchingRehydrated();
+          if (!firstTrigger) {
+            firstTrigger = true;
+            return;
+          }
           const router = this._routers[this.portDetector.name];
           this._changeRoutingOnSever(
             this.portDetector.name,
@@ -235,7 +250,37 @@ class ReactantRouter extends BaseReactantRouter {
         }
       }
     );
-    return stopWatching;
+    const stopWatchingIsFirstRendering = watch(
+      this,
+      () => this.portDetector.lastAction.action,
+      () => {
+        if (!this.enableCacheRouting) {
+          stopWatchingIsFirstRendering();
+        }
+        const action = this.portDetector.lastAction
+          .action as any as LocationChangeAction;
+        if (
+          action.type === LOCATION_CHANGE &&
+          action.payload.isFirstRendering
+        ) {
+          stopWatchingIsFirstRendering();
+          if (!firstTrigger) {
+            firstTrigger = true;
+            return;
+          }
+          const router = this._routers[this.portDetector.name];
+          this._changeRoutingOnSever(
+            this.portDetector.name,
+            router ?? this.defaultHistory,
+            Date.now()
+          );
+        }
+      }
+    );
+    return () => {
+      stopWatchingRehydrated();
+      stopWatchingIsFirstRendering();
+    };
   }
 
   /**

diff --git a/packages/reactant-share/test/coworker.test.tsx b/packages/reactant-share/test/coworker.test.tsx
@@ -863,7 +863,6 @@ describe('base', () => {
     expect(onServerFn.mock.calls.length).toBe(0);
     expect(subscribeOnServerFn.mock.calls.length).toBe(0);
     expect(coworkerModuleSubscribe.mock.calls.map((item) => item[0])).toEqual([
-      'Coworker',
       'counterCoworker',
       'Coworker',
       'counterCoworker',
@@ -875,10 +874,11 @@ describe('base', () => {
       'counterCoworker',
       'Coworker',
       'counterCoworker',
-      'counterCoworker',
       'Coworker',
       'counterCoworker',
       'counterCoworker',
+      'Coworker',
+      'counterCoworker',
     ]);
     expect(nonCoworkerModuleSubscribe.mock.calls).toEqual(
       Array(10).fill([undefined])

diff --git a/packages/reactant-share/test/router.test.tsx b/packages/reactant-share/test/router.test.tsx
@@ -1809,22 +1809,22 @@ describe('base with storage and router', () => {
     expect(onClientFn.mock.calls.length).toBe(1);
     expect(subscribeOnClientFn.mock.calls.length).toBe(1);
     expect(onServerFn.mock.calls.length).toBe(1);
-    expect(subscribeOnServerFn.mock.calls.length).toBe(4);
+    expect(subscribeOnServerFn.mock.calls.length).toBe(2);
 
     await clientApp.bootstrap(clientContainer);
 
     expect(onClientFn.mock.calls.length).toBe(1);
     expect(subscribeOnClientFn.mock.calls.length).toBe(2);
     expect(onServerFn.mock.calls.length).toBe(1);
-    expect(subscribeOnServerFn.mock.calls.length).toBe(4);
+    expect(subscribeOnServerFn.mock.calls.length).toBe(2);
     expect(clientContainer.querySelector('#content')?.textContent).toBe('home');
 
     await Promise.resolve();
 
     expect(onClientFn.mock.calls.length).toBe(1);
     expect(subscribeOnClientFn.mock.calls.length).toBe(4);
     expect(onServerFn.mock.calls.length).toBe(1);
-    expect(subscribeOnServerFn.mock.calls.length).toBe(6);
+    expect(subscribeOnServerFn.mock.calls.length).toBe(4);
     expect(clientContainer.querySelector('#content')?.textContent).toBe('-0+');
 
     act(() => {
@@ -1838,7 +1838,7 @@ describe('base with storage and router', () => {
     expect(onClientFn.mock.calls.length).toBe(1);
     expect(subscribeOnClientFn.mock.calls.length).toBe(6);
     expect(onServerFn.mock.calls.length).toBe(1);
-    expect(subscribeOnServerFn.mock.calls.length).toBe(8);
+    expect(subscribeOnServerFn.mock.calls.length).toBe(6);
     expect(serverApp.instance.router.currentPath).toBe('/counter');
     expect(clientApp.instance.router.currentPath).toBe('/counter');
     expect(clientContainer.querySelector('#content')?.textContent).toBe('-0+');