Merge pull request dvajs#1204 from dvajs/fix-dva-loading

Fix dva-loading: dispatch multiple effects of the same model will break the model loading state
ming1014 · Sep 10, 2017 · a3a9c85 · a3a9c85
2 parents 092e503 + d6bc698
commit a3a9c85
Show file tree

Hide file tree

Showing 2 changed files with 63 additions and 17 deletions.
diff --git a/packages/dva-loading/src/index.js b/packages/dva-loading/src/index.js
@@ -7,10 +7,8 @@ function createLoading(opts = {}) {
   const initialState = {
     global: false,
     models: {},
+    effects: {},
   };
-  if (opts.effects) {
-    initialState.effects = {};
-  }
 
   const extraReducers = {
     [namespace](state = initialState, { type, payload }) {
@@ -22,24 +20,28 @@ function createLoading(opts = {}) {
             ...state,
             global: true,
             models: { ...state.models, [namespace]: true },
+            effects: { ...state.effects, [actionType]: true },
           };
-          if (opts.effects) {
-            ret.effects = { ...state.effects, [actionType]: true };
-          }
           break;
         case HIDE: // eslint-disable-line
-          const models = { ...state.models, [namespace]: false };
+          const effects = { ...state.effects, [actionType]: false };
+          const models = {
+            ...state.models,
+            [namespace]: Object.keys(effects).some((actionType) => {
+              const _namespace = actionType.split('/')[0];
+              if (_namespace !== namespace) return false;
+              return effects[actionType];
+            }),
+          };
           const global = Object.keys(models).some((namespace) => {
             return models[namespace];
           });
           ret = {
             ...state,
             global,
             models,
+            effects,
           };
-          if (opts.effects) {
-            ret.effects = { ...state.effects, [actionType]: false };
-          }
           break;
         default:
           ret = state;

diff --git a/packages/dva-loading/test/index-test.js b/packages/dva-loading/test/index-test.js
@@ -26,11 +26,19 @@ describe('dva-loading', () => {
     app.router(() => 1);
     app.start();
 
-    expect(app._store.getState().loading).toEqual({ global: false, models: {} });
+    expect(app._store.getState().loading).toEqual({ global: false, models: {}, effects: {} });
     app._store.dispatch({ type: 'count/addRemote' });
-    expect(app._store.getState().loading).toEqual({ global: true, models: { count: true } });
+    expect(app._store.getState().loading).toEqual({
+      global: true,
+      models: { count: true },
+      effects: { 'count/addRemote': true },
+    });
     setTimeout(() => {
-      expect(app._store.getState().loading).toEqual({ global: false, models: { count: false } });
+      expect(app._store.getState().loading).toEqual({
+        global: false,
+        models: { count: false },
+        effects: { 'count/addRemote': false },
+      });
       done();
     }, 200);
   });
@@ -78,7 +86,7 @@ describe('dva-loading', () => {
     });
     app.router(() => 1);
     app.start();
-    expect(app._store.getState().fooLoading).toEqual({ global: false, models: {} });
+    expect(app._store.getState().fooLoading).toEqual({ global: false, models: {}, effects: {} });
   });
 
   it('takeLatest', (done) => {
@@ -100,13 +108,49 @@ describe('dva-loading', () => {
     app.router(() => 1);
     app.start();
 
-    expect(app._store.getState().loading).toEqual({ global: false, models: {} });
+    expect(app._store.getState().loading).toEqual({ global: false, models: {}, effects: {} });
     app._store.dispatch({ type: 'count/addRemote' });
     app._store.dispatch({ type: 'count/addRemote' });
-    expect(app._store.getState().loading).toEqual({ global: true, models: { count: true } });
+    expect(app._store.getState().loading).toEqual({
+      global: true,
+      models: { count: true },
+      effects: { 'count/addRemote': true },
+    });
     setTimeout(() => {
-      expect(app._store.getState().loading).toEqual({ global: false, models: { count: false } });
+      expect(app._store.getState().loading).toEqual({
+        global: false,
+        models: { count: false },
+        effects: { 'count/addRemote': false },
+      });
       done();
     }, 200);
   });
+
+  it('multiple effects', (done) => {
+    const app = dva();
+    app.use(createLoading({ effects: true }));
+    app.model({
+      namespace: 'count',
+      state: 0,
+      effects: {
+        *a(action, { call }) {
+          yield call(delay, 100);
+        },
+        *b(action, { call }) {
+          yield call(delay, 500);
+        },
+      },
+    });
+    app.router(() => 1);
+    app.start();
+    app._store.dispatch({ type: 'count/a' });
+    app._store.dispatch({ type: 'count/b' });
+    setTimeout(() => {
+      expect(app._store.getState().loading.models.count).toEqual(true);
+    }, 200);
+    setTimeout(() => {
+      expect(app._store.getState().loading.models.count).toEqual(false);
+      done();
+    }, 800);
+  });
 });