[NO-ISSUE] Fixed issue with FetchingProvider remounting children (#14)

* There was an issue with FetchingProvider that it rendered it's
children initially, before fetching was started, then rendering null and
finally rendering children again after fetching. This happened when
blocking was set to true (which is default). This was causing multiple
mounts of children, affecting performance.
* The issue was caused by misuse of "isFetching" flag in rendering logic
since it was initially false, before fetching has even started.
* Replacing "isFetching" with "isFetched" is restoring original purpose
of "blocking" flag, since rendering should or shouldn't be blocked until
data is fetched, not while it is fetching as it was implemented so far.

Author: ivanCosic

src/lib/FetchingProvider.test.tsx

@@ -3,9 +3,21 @@ import * as RTL from '@testing-library/react';
 import { FetchingProvider } from './FetchingProvider';
 import { useMessageSource } from './useMessageSource';
 
+const noop = () => {};
+
 describe('FetchingProvider', () => {
-  function Spy() {
+  type SpyProps = {
+    onMount?: Function,
+  };
+
+  function Spy({ onMount = noop }: SpyProps) {
     const { getMessage } = useMessageSource();
+
+    // Run onMount only once, when component is mounted initially.
+    React.useEffect(() => {
+      onMount();
+    }, []);
+
     return <span>{getMessage('hello.world')}</span>;
   }
 
@@ -98,4 +110,37 @@ describe('FetchingProvider', () => {
     expect(faultyFetch).toHaveBeenCalledTimes(1);
     expect(onFetchingError).toHaveBeenCalledTimes(1);
   });
+
+  it('mounts children only once', async () => {
+    let timesChildrenAreMounted = 0;
+    const increaseChildrenMountedNumber = () => {
+      timesChildrenAreMounted += 1;
+    };
+
+    const transform = jest.fn(x => x);
+    const onFetchingStart = jest.fn();
+    const onFetchingEnd = jest.fn();
+
+    function TestComponent() {
+      return (
+        <FetchingProvider
+          url="http://any.uri/texts?lang=en"
+          transform={transform}
+          onFetchingStart={onFetchingStart}
+          onFetchingEnd={onFetchingEnd}
+        >
+          <Spy onMount={increaseChildrenMountedNumber} />
+        </FetchingProvider>
+      );
+    }
+
+    const { getByText } = RTL.render(<TestComponent />);
+    const spyNode = await RTL.waitForElement(() => getByText('Hello world'));
+
+    expect(spyNode).toBeDefined();
+    expect(spyNode.innerHTML).toBe('Hello world');
+    // @ts-ignore
+    expect(global.fetch).toHaveBeenCalledTimes(1);
+    expect(timesChildrenAreMounted).toBe(1);
+  });
 });

src/lib/FetchingProvider.tsx

@@ -50,12 +50,12 @@ export interface FetchingProviderApi {
 
 type State = {
   translations: MessageSourceContextShape,
-  isFetching: boolean,
+  isFetched: boolean,
 };
 
 const initialState: State = {
   translations: {},
-  isFetching: false,
+  isFetched: false,
 };
 
 /**
@@ -73,12 +73,12 @@ export function FetchingProvider(props: FetchingProviderApi) {
     onFetchingError = noop,
   } = props;
 
-  const [{ translations, isFetching }, setState] = React.useState<State>(initialState);
+  const [{ translations, isFetched }, setState] = React.useState<State>(initialState);
 
   React.useEffect(() => {
     let isStillMounted = true;
 
-    setState(state => ({ ...state, isFetching: true }));
+    setState((state: State) => ({ ...state, isFetched: false }));
     onFetchingStart();
 
     fetch(url)
@@ -87,7 +87,7 @@ export function FetchingProvider(props: FetchingProviderApi) {
         if (isStillMounted) {
           setState({
             translations: transform(response),
-            isFetching: false,
+            isFetched: true,
           });
           onFetchingEnd();
         }
@@ -99,6 +99,6 @@ export function FetchingProvider(props: FetchingProviderApi) {
     };
   }, [url]); // re-fetch only when url changes
 
-  const shouldRenderSubtree = !blocking || (blocking && !isFetching);
+  const shouldRenderSubtree = !blocking || (blocking && isFetched);
   return <Provider value={translations}>{shouldRenderSubtree ? children : null}</Provider>;
 }