Fix/is loading/multi render behaviour weirdness (#99)

* fix(isLoading): added catch for post render edge case

* test(isLoading): added tests

* chore(clean-up): removed numOfRenders from isLoading function

* chore(sync): added code to sync changes with pr#98

* feat(forceInitialFetch): added forceInitialFetch functionality

* refactor(pr-comments): acconting for regression

* test(queryHelpers): added extra tests and edge cases

* chore(clean-up): updated tests to be grouped together

Author: DyBev

packages/fetchye/__tests__/queryHelpers.spec.js

@@ -19,30 +19,118 @@ import {
 } from '../src/queryHelpers';
 
 describe('isLoading', () => {
-  it('should return true if loading cache true', () => {
-    expect(isLoading({
-      loading: true, data: undefined, numOfRenders: 2, options: {},
-    })).toEqual(true);
-  });
-  it('should return true if first render is true', () => {
-    expect(isLoading({
-      loading: false, data: undefined, numOfRenders: 1, options: { },
-    })).toEqual(true);
-  });
-  it('should return true if first render and forceInitialFetch option is true', () => {
-    expect(isLoading({
-      loading: false, data: undefined, numOfRenders: 1, options: { forceInitialFetch: true },
-    })).toEqual(true);
-  });
-  it('should return false if first render is true and defer is true', () => {
-    expect(isLoading({
-      loading: false, data: undefined, numOfRenders: 1, options: { defer: true },
-    })).toEqual(false);
+  describe('should return false', () => {
+    it('if defer is true, and not already loading', () => {
+      expect(isLoading({
+        loading: false,
+        data: undefined,
+        options: {
+          defer: true,
+        },
+        error: undefined,
+        refs: {},
+      })).toEqual(false);
+    });
+
+    it('if forceInitialFetch is true, but call is defered', () => {
+      expect(isLoading({
+        loading: false,
+        data: {},
+        options: {
+          defer: true,
+        },
+        error: undefined,
+        refs: {
+          forceInitialFetch: true,
+        },
+      })).toEqual(false);
+    });
+
+    it('if data is undefined and no error, but call is defered', () => {
+      expect(isLoading({
+        loading: false,
+        data: undefined,
+        options: {
+          defer: true,
+        },
+        error: undefined,
+        refs: {
+          forceInitialFetch: false,
+        },
+      })).toEqual(false);
+    });
+
+    it('if there are errors present', () => {
+      expect(isLoading({
+        loading: false,
+        options: {
+          defer: false,
+        },
+        error: {},
+        refs: {},
+      })).toEqual(false);
+    });
+
+    it('if there is data present', () => {
+      expect(isLoading({
+        loading: false,
+        data: { },
+        options: {
+          defer: false,
+        },
+        refs: {},
+      })).toEqual(false);
+    });
   });
-  it('should return false if all args are false', () => {
-    expect(isLoading({
-      loading: false, numOfRenders: 2, options: { defer: false },
-    })).toEqual(false);
+
+  describe('should return true', () => {
+    it('if fetchye is loading', () => {
+      expect(isLoading({
+        loading: true,
+        data: undefined,
+        options: {},
+        error: undefined,
+        refs: {},
+      })).toEqual(true);
+    });
+
+    it('if fetchye is loading, even if defered is true', () => {
+      expect(isLoading({
+        loading: true,
+        data: undefined,
+        options: {
+          defered: true,
+        },
+        error: undefined,
+        refs: {},
+      })).toEqual(true);
+    });
+
+    it('if there is no data, no error and not defered', () => {
+      expect(isLoading({
+        data: undefined,
+        loading: false,
+        options: {
+          defer: false,
+        },
+        error: undefined,
+        refs: {},
+      })).toEqual(true);
+    });
+
+    it('if forceInitialFetch is true, even if data exists', () => {
+      expect(isLoading({
+        loading: false,
+        data: {},
+        options: {
+          defer: false,
+        },
+        error: undefined,
+        refs: {
+          forceInitialFetch: true,
+        },
+      })).toEqual(true);
+    });
   });
 });
 

packages/fetchye/src/queryHelpers.js

@@ -15,31 +15,23 @@
  */
 
 export const isLoading = ({
-  loading, data, numOfRenders, options,
+  loading, data, options, error, refs,
 }) => {
-  // If first render
-  if (numOfRenders === 1) {
-    // and defer is true
-    if (options.defer) {
-      // isLoading should be false
-      return false;
-    }
-    // and no data exists and presume loading state isn't present
-    if (!data) {
-      // isLoading should be true
-      return true;
-    }
-    // when we force fetch isLoading is always going to be true on first render
-    if (options.forceInitialFetch) {
-      // isLoading should be true
-      return true;
-    }
-  }
-  // If not on first render and loading from cache is true
+  // If loading from cache is true
   if (loading) {
     // isLoading should be true
     return true;
   }
+
+  // will fetch if no data and no error
+  // will fetch if forceInitialFetch is true
+  const willFetchIfNotDefered = (!data && !error)
+    || (refs.forceInitialFetch === true);
+
+  if (willFetchIfNotDefered && !options.defer) {
+    return true;
+  }
+
   // else isLoading should be false
   return false;
 };

packages/fetchye/src/useFetchye.js

@@ -88,8 +88,11 @@ const useFetchye = (
     isLoading: isLoading({
       loading: selectorState.current.loading,
       data: selectorState.current.data || options.initialData?.data,
-      numOfRenders: numOfRenders.current,
       options,
+      error: selectorState.current.error,
+      refs: {
+        forceInitialFetch: forceInitialFetch.current,
+      },
     }),
     error: passInitialData(
       {