Fix isLoading for multiple async calls

goatslacker · May 25, 2015 · 6e4ed23 · 6e4ed23
1 parent e45c615
commit 6e4ed23
Show file tree

Hide file tree

Showing 2 changed files with 99 additions and 37 deletions.
diff --git a/src/alt/store/StoreMixin.js b/src/alt/store/StoreMixin.js
@@ -22,39 +22,45 @@ const StoreMixin = {
   },
 
   exportAsync(asyncMethods) {
-    let isLoading = false
-    let hasError = false
+    this.registerAsync(asyncMethods)
+  },
+
+  registerAsync(asyncDef) {
+    let loadCounter = 0
+
+    const asyncMethods = fn.isFunction(asyncDef)
+      ? asyncDef(this.alt)
+      : asyncDef
 
     const toExport = Object.keys(asyncMethods).reduce((publicMethods, methodName) => {
-      const asyncSpec = asyncMethods[methodName](this)
+      const desc = asyncMethods[methodName]
+      const spec = fn.isFunction(desc) ? desc(this) : desc
 
       const validHandlers = ['success', 'error', 'loading']
       validHandlers.forEach((handler) => {
-        if (asyncSpec[handler] && !asyncSpec[handler][Sym.ACTION_KEY]) {
+        if (spec[handler] && !spec[handler][Sym.ACTION_KEY]) {
           throw new Error(`${handler} handler must be an action function`)
         }
       })
 
       publicMethods[methodName] = (...args) => {
         const state = this.getInstance().getState()
-        const value = asyncSpec.local && asyncSpec.local(state, ...args)
-        const shouldFetch = asyncSpec.shouldFetch ? asyncSpec.shouldFetch(state, ...args) : !value
+        const value = spec.local && spec.local(state, ...args)
+        const shouldFetch = spec.shouldFetch ? spec.shouldFetch(state, ...args) : !value
 
         // if we don't have it in cache then fetch it
         if (shouldFetch) {
-          isLoading = true
-          hasError = false
+          loadCounter += 1
           /* istanbul ignore else */
-          if (asyncSpec.loading) asyncSpec.loading()
-          asyncSpec.remote(state, ...args)
+          if (spec.loading) spec.loading()
+          spec.remote(state, ...args)
             .then((v) => {
-              isLoading = false
-              asyncSpec.success(v)
+              loadCounter -= 1
+              spec.success(v)
             })
             .catch((v) => {
-              isLoading = false
-              hasError = true
-              asyncSpec.error(v)
+              loadCounter -= 1
+              spec.error(v)
             })
         } else {
           // otherwise emit the change now
@@ -67,8 +73,7 @@ const StoreMixin = {
 
     this.exportPublicMethods(toExport)
     this.exportPublicMethods({
-      isLoading: () => isLoading,
-      hasError: () => hasError
+      isLoading: () => loadCounter > 0
     })
   },
 

diff --git a/test/async-test.js b/test/async-test.js
@@ -39,25 +39,33 @@ const StargazerSource = {
       error: StargazerActions.failed
     }
   },
-  alwaysFetchUsers() {
-    return {
-      remote,
-      local: () => true,
-      loading: StargazerActions.fetchingUsers,
-      success: StargazerActions.usersReceived,
-      error: StargazerActions.failed,
-      shouldFetch: () => true
-    }
+
+  alwaysFetchUsers: {
+    remote,
+    local: () => true,
+    loading: StargazerActions.fetchingUsers,
+    success: StargazerActions.usersReceived,
+    error: StargazerActions.failed,
+    shouldFetch: () => true
   },
-  neverFetchUsers() {
-    return {
-      remote,
-      local: () => false,
-      loading: StargazerActions.fetchingUsers,
-      success: StargazerActions.usersReceived,
-      error: StargazerActions.failed,
-      shouldFetch: () => false
-    }
+
+  neverFetchUsers: {
+    remote,
+    local: () => false,
+    loading: StargazerActions.fetchingUsers,
+    success: StargazerActions.usersReceived,
+    error: StargazerActions.failed,
+    shouldFetch: () => false
+  },
+
+  fetchRepos: {
+    remote() {
+      return new Promise((resolve, reject) => {
+        setTimeout(() => resolve('TESTTEST'), 200)
+      })
+    },
+    success: StargazerActions.usersReceived,
+    error: StargazerActions.failed
   }
 }
 
@@ -165,7 +173,6 @@ export default {
           count()
           test()
           assert.ok(local.calledOnce)
-          assert.notOk(StargazerStore.hasError(), 'no errors')
           assert.notOk(StargazerStore.isLoading())
           assert(remote.callCount === 0)
           done()
@@ -185,7 +192,6 @@ export default {
           assert(state.users.length === 0)
         } else if (spy.callCount === 2) {
           assert.match(state.errorMessage, /things broke/)
-          assert.ok(StargazerStore.hasError(), 'we have an error')
           count()
           test()
           assert.notOk(StargazerStore.isLoading())
@@ -210,5 +216,56 @@ export default {
       assert.notOk(StargazerStore.isLoading())
       assert(remote.callCount === 0)
     },
+
+    'multiple loads'(done) {
+      const unsub = StargazerStore.listen((state) => {
+        if (state.users === 'TESTTEST') {
+          assert.notOk(StargazerStore.isLoading())
+          unsub()
+          done()
+        } else {
+          assert.ok(StargazerStore.isLoading())
+        }
+      })
+
+      StargazerStore.fetchUsers()
+      StargazerStore.fetchRepos()
+      assert.ok(StargazerStore.isLoading())
+    },
+
+    'as a function'() {
+      const FauxSource = sinon.stub().returns({})
+
+      @datasource(FauxSource)
+      class FauxStore {
+        static displayName = 'FauxStore'
+      }
+
+      const store = alt.createStore(FauxStore)
+
+      assert(FauxSource.firstCall.args[0] === alt)
+      assert.isFunction(store.isLoading)
+    },
+
+    'as an object'() {
+      const actions = alt.generateActions('test')
+
+      const PojoSource = {
+        justTesting: {
+          success: actions.test,
+          error: actions.test,
+        }
+      }
+
+      @datasource(PojoSource)
+      class MyStore {
+        static displayName = 'MyStore'
+      }
+
+      const store = alt.createStore(MyStore)
+
+      assert.isFunction(store.justTesting)
+      assert.isFunction(store.isLoading)
+    },
   }
 }