Fix race condition between script load and rerenders

package.json

@@ -55,8 +55,5 @@
         "rimraf": "^3.0.2",
         "ts-jest": "^25.3.0",
         "typescript": "^3.8.3"
-    },
-    "dependencies": {
-        "react-is-mounted-hook": "^1.0.3"
     }
 }

src/__tests__/use-script.test.tsx

@@ -1,4 +1,4 @@
-import { renderHook } from '@testing-library/react-hooks';
+import { act, renderHook } from '@testing-library/react-hooks';
 
 import useScript from '../use-script';
 
@@ -57,10 +57,51 @@ describe('useScript', () => {
     it('should render a script only once', () => {
         expect(document.querySelectorAll('script').length).toBe(0);
 
-        renderHook(() => useScript({ src: 'http://scriptsrc/' }));
+        const props = { src: 'http://scriptsrc/' };
+        const handle = renderHook((p) => useScript(p), {
+            initialProps: props,
+        });
         expect(document.querySelectorAll('script').length).toBe(1);
 
-        renderHook(() => useScript({ src: 'http://scriptsrc/' }));
+        handle.rerender();
         expect(document.querySelectorAll('script').length).toBe(1);
     });
+
+    it('should set loading false on load', async () => {
+        const props = { src: 'http://scriptsrc/' };
+        const handle = renderHook((p) => useScript(p), {
+            initialProps: props,
+        });
+
+        const [loading, error] = handle.result.current;
+        expect(loading).toBe(true);
+        expect(error).toBe(null);
+
+        act(() => {
+            const el = document.querySelector('script');
+            if (el) {
+                el.dispatchEvent(new Event('load'));
+            }
+        });
+
+        const [loadingAfter, errorAfter] = handle.result.current;
+        expect(loadingAfter).toBe(false);
+        expect(errorAfter).toBe(null);
+    });
+
+    it('should not cause issues on unmount', async () => {
+        const props = { src: 'http://scriptsrc/' };
+        const handle = renderHook((p) => useScript(p), {
+            initialProps: props,
+        });
+
+        handle.unmount();
+
+        act(() => {
+            const el = document.querySelector('script');
+            if (el) {
+                el.dispatchEvent(new Event('load'));
+            }
+        });
+    });
 });

src/use-script.tsx

@@ -1,10 +1,7 @@
 import { useState, useEffect } from 'react';
-import useIsMounted from 'react-is-mounted-hook';
 
 export interface ScriptProps {
     src: HTMLScriptElement['src'];
-    onload?: HTMLScriptElement['onload'];
-    onerror?: HTMLScriptElement['onerror'];
     [key: string]: any;
 }
 
@@ -14,20 +11,12 @@ export default function useScript({
     src,
     ...attributes
 }: ScriptProps): [boolean, ErrorState] {
-    const isMounted = useIsMounted();
     const [loading, setLoading] = useState(true);
     const [error, setError] = useState<ErrorState>(null);
 
     useEffect(() => {
         if (!isBrowser) return;
 
-        if (document.querySelector(`script[src="${src}"]`)) {
-            if (isMounted()) {
-                setLoading(false);
-            }
-            return;
-        }
-
         const scriptEl = document.createElement('script');
         scriptEl.setAttribute('src', src);
 
@@ -40,14 +29,10 @@ export default function useScript({
         });
 
         const handleLoad = () => {
-            if (isMounted()) {
-                setLoading(false);
-            }
+            setLoading(false);
         };
         const handleError = (error: ErrorEvent) => {
-            if (isMounted()) {
-                setError(error);
-            }
+            setError(error);
         };
 
         scriptEl.addEventListener('load', handleLoad);
@@ -59,7 +44,9 @@ export default function useScript({
             scriptEl.removeEventListener('load', handleLoad);
             scriptEl.removeEventListener('error', handleError);
         };
-    }, [src, attributes, isMounted]);
+        // we need to ignore the attributes as they're a new object per call, so we'd never skip an effect call
+        // eslint-disable-next-line react-hooks/exhaustive-deps
+    }, [src]);
 
     return [loading, error];
 }