MinimumDurationLoader mounts twice if loading finishes before MinDura…

…tion has passed (#213)

Fixes a bug that happens when isLoaded is set to true before minDuration has passed. In this scenario, the children will get mounted twice.

Wraps jest.advanceTimersByTime with "act()" to clear warnings in the tests

Prepares v2.6.2

CHANGELOG.md

@@ -1,5 +1,9 @@
 ## Changelog
 
+## 2.6.2
+
+- [bug] `MinimumDurationLoader` mounts the underlying components twice if `minDuration` is reached before `isLoaded` is set to `true`.
+
 ## 2.6.1
 
 - [fix] Incorrect directory was published to npm as 2.6.0

package.json

@@ -1,6 +1,6 @@
 {
   "name": "@mapbox/mr-ui",
-  "version": "2.6.1",
+  "version": "2.6.2",
   "description": "UI components for Mapbox projects",
   "main": "index.js",
   "homepage": "./",

src/components/minimum-duration-loader/minimum-duration-loader.test.tsx

@@ -1,5 +1,5 @@
 import React from 'react';
-import { render, screen } from '@testing-library/react';
+import { render, screen, act } from '@testing-library/react';
 import MinimumDurationLoader from './minimum-duration-loader';
 
 const children = <span data-testid='test-content'>Content</span>;
@@ -37,7 +37,7 @@ describe('MinimumDurationLoader', () => {
       expect(screen.getByTestId('minimum-duration-loader')).toBeInTheDocument();
       expect(screen.queryByTestId('test-content')).not.toBeInTheDocument();
 
-      jest.advanceTimersByTime(1000);
+     act(() => jest.advanceTimersByTime(1000));
 
       expect(screen.queryByTestId('minimum-duration-loader')).not.toBeInTheDocument();
       expect(screen.getByTestId('test-content')).toBeInTheDocument();
@@ -46,11 +46,11 @@ describe('MinimumDurationLoader', () => {
     test.skip('does not render the content if isLoaded becomes `false` after having been `true`', () => {
       const { rerender } = render(<MinimumDurationLoader {...{...props, isLoaded: true}} />)
       expect(screen.getByTestId('minimum-duration-loader')).toBeInTheDocument();
-      jest.advanceTimersByTime(1000);
+      act(() => jest.advanceTimersByTime(1000));
       expect(screen.getByTestId('test-content')).toBeInTheDocument();
       rerender(<MinimumDurationLoader {...props } />)
       expect(screen.getByTestId('minimum-duration-loader')).toBeInTheDocument();
-      jest.advanceTimersByTime(1000);
+      act(() => jest.advanceTimersByTime(1000));
       expect(screen.getByTestId('test-content')).toBeInTheDocument();
     });
   });
@@ -104,11 +104,11 @@ describe('MinimumDurationLoader', () => {
       // Loader should exist
       expect(screen.getByTestId('minimum-duration-loader')).toBeInTheDocument();
       expect(screen.queryByTestId('test-content')).not.toBeInTheDocument();
-      jest.advanceTimersByTime(1000);
+      act(() => jest.advanceTimersByTime(1000));
       // Loader should still exist because we exceeded the minimum duration
       expect(screen.getByTestId('minimum-duration-loader')).toBeInTheDocument();
       expect(screen.queryByTestId('test-content')).not.toBeInTheDocument();
-      jest.advanceTimersByTime(4000);
+      act(() => jest.advanceTimersByTime(4000));
       // We've exceeded the minimum duration, so the loader should not exist
       expect(screen.queryByTestId('minimum-duration-loader')).not.toBeInTheDocument();
       expect(screen.getByTestId('test-content')).toBeInTheDocument();
@@ -164,4 +164,102 @@ describe('MinimumDurationLoader', () => {
       expect(baseElement).toMatchSnapshot();
     });
   });
+
+  describe('always mounts the content exactly once', () => {
+    beforeEach(() => {
+      jest.useFakeTimers();
+    });
+
+    afterEach(() => {
+      jest.useRealTimers();
+    });
+
+    let mountedCount = 0;
+
+    const Counter: React.FC = () => {
+      React.useEffect(() => {
+        mountedCount++
+      }, []);
+
+      return <span data-testid="mounted-count">{mountedCount}</span>;
+    }
+
+    afterEach(() => {
+      mountedCount = 0;
+    })
+
+    test('when isLoaded = true before minDuration has passed', () => {
+      const MIN_DURATION = 100;
+      const { rerender } = render(
+        <MinimumDurationLoader minDuration={MIN_DURATION} isLoaded={false}>
+          <Counter />
+        </MinimumDurationLoader>
+      )
+
+      expect(mountedCount).toBe(0)
+
+      act(() => jest.advanceTimersByTime(60));
+      rerender(
+        <MinimumDurationLoader minDuration={MIN_DURATION} isLoaded={true}>
+          <Counter />
+        </MinimumDurationLoader>
+      )
+
+      // Does not mount yet since minDuration has not been reached
+      expect(mountedCount).toBe(0)
+      rerender(
+        <MinimumDurationLoader minDuration={MIN_DURATION} isLoaded={true}>
+          <Counter />
+        </MinimumDurationLoader>
+      )
+
+      act(() => jest.advanceTimersByTime(60));
+
+      // Mounts since minDuration has passed and isLoaded = true
+      expect(mountedCount).toBe(1)
+
+      rerender(
+        <MinimumDurationLoader minDuration={MIN_DURATION} isLoaded={true}>
+          <Counter />
+        </MinimumDurationLoader>
+      )
+
+      // Confirm that the component has not been mounted again if we move forward in time
+      act(() => jest.advanceTimersByTime(1));
+      expect(mountedCount).toBe(1)
+    });
+
+    test('when isLoaded = true after minDuration has passed', () => {
+      const MIN_DURATION = 100;
+      const { rerender } = render(
+        <MinimumDurationLoader minDuration={MIN_DURATION} isLoaded={false}>
+          <Counter />
+        </MinimumDurationLoader>
+      )
+
+      expect(mountedCount).toBe(0)
+
+      // Move forward in time more than minDuration before setting isLoaded = true
+      act(() => jest.advanceTimersByTime(110));      
+      rerender(
+        <MinimumDurationLoader minDuration={MIN_DURATION} isLoaded={true}>
+          <Counter />
+        </MinimumDurationLoader>
+      )
+
+      // Mounts since minDuration has passed and isLoaded = true
+      expect(mountedCount).toBe(1)
+
+      // Confirm that the component has not been mounted again if we move forward in time
+      act(() => jest.advanceTimersByTime(1));
+      rerender(
+        <MinimumDurationLoader minDuration={MIN_DURATION} isLoaded={true}>
+          <Counter />
+        </MinimumDurationLoader>
+      )
+
+
+      expect(mountedCount).toBe(1)
+    });
+  })
 });

src/components/minimum-duration-loader/minimum-duration-loader.tsx

@@ -65,7 +65,7 @@ export default function MinimumDurationLoader({
     isLoaded
   ]);
 
-  const shouldRenderContent = isLoaded && (wasLoaded || delayedLoaded);
+  const shouldRenderContent = isLoaded && (wasLoaded || delayedLoaded || prevIsLoaded);
 
   if (shouldRenderContent) {
     return (