React 18 upgrade (#633)

* React 17 -> 18.3.1 upgrade * Fix failing tests and update docs for language prop in MediaPlayer * Fixes after rebase * Upgrade react-styleguidist to latest, compatible with React 18 * Fix dynamic import path for video.js language files
samvera-labs · Sep 6, 2024 · 1e6528c · 1e6528c
1 parent bc10eca
commit 1e6528c
Show file tree

Hide file tree

Showing 21 changed files with 486 additions and 917 deletions.
diff --git a/demo/index.js b/demo/index.js
@@ -1,5 +1,5 @@
 import React from 'react';
-import ReactDOM from 'react-dom';
+import * as ReactDOMClient from 'react-dom/client';
 import App from './app';
 import config from './config';
 
@@ -12,6 +12,7 @@ const manifestURL = () => {
   return url;
 };
 
-ReactDOM.render(<App
-  manifestURL={manifestURL()}
-/>, document.getElementById('root'));
+const container = document.getElementById('root');
+const root = ReactDOMClient.createRoot(container);
+
+root.render(<App manifestURL={manifestURL} />);
diff --git a/package.json b/package.json
@@ -47,10 +47,11 @@
     "@rollup/plugin-commonjs": "^15.0.0",
     "@rollup/plugin-node-resolve": "^10.0.0",
     "@rollup/plugin-replace": "^2.3.2",
+    "@testing-library/dom": "^10.4.0",
     "@testing-library/jest-dom": "^5.11.4",
-    "@testing-library/react": "^11.1.0",
+    "@testing-library/react": "^16.0.1",
     "@types/jest": "^26.0.12",
-    "@types/react": "^18.2.79",
+    "@types/react": "18.3.5",
     "babel-jest": "^26.3.0",
     "babel-loader": "^8.1.0",
     "babel-plugin-module-alias": "^1.6.0",
@@ -64,9 +65,9 @@
     "path-browserify": "^1.0.1",
     "postcss-svg": "^3.0.0",
     "prop-types": "^15.7.2",
-    "react": "^17.0.1",
-    "react-dom": "^17.0.1",
-    "react-styleguidist": "^11.0.8",
+    "react": "^18.3.1",
+    "react-dom": "^18.3.1",
+    "react-styleguidist": "13.1.3",
     "rollup": "^2.26.9",
     "rollup-plugin-cleaner": "^1.0.0",
     "rollup-plugin-postcss": "^3.1.8",
@@ -94,8 +95,8 @@
     "videojs-markers-plugin": "^1.0.2"
   },
   "peerDependencies": {
-    "react": "^17.0.1",
-    "react-dom": "^17.0.1",
+    "react": "^18.3.1",
+    "react-dom": "^18.3.1",
     "video.js": "^8.10.0"
   },
   "repository": {

diff --git a/src/components/IIIFPlayer/IIIFPlayer.js b/src/components/IIIFPlayer/IIIFPlayer.js
@@ -47,4 +47,3 @@ IIIFPlayer.propTypes = {
   startCanvasTime: PropTypes.number,
 };
 
-IIIFPlayer.defaultProps = {};
diff --git a/src/components/IIIFPlayerWrapper.js b/src/components/IIIFPlayerWrapper.js
@@ -26,47 +26,51 @@ export default function IIIFPlayerWrapper({
 
   const { showBoundary } = useErrorBoundary();
 
-  React.useEffect(async () => {
+  // AbortController for Manifest fetch request
+  let controller;
+
+  const fetchManifest = async (url) => {
+    controller = new AbortController();
+    let requestOptions = {
+      // NOTE: try thin in Avalon
+      //credentials: 'include',
+      // headers: { 'Avalon-Api-Key': '' },
+    };
+    /**
+     * Sanitize manifest urls of query or anchor fragments included in the
+     * middle of the url: hhtp://example.com/endpoint?params/manifest
+     */
+    const sanitizedUrl = url.replace(/[\?#].*(?=\/)/i, '');
+    try {
+      await fetch(sanitizedUrl, requestOptions, { signal: controller.signal })
+        .then((result) => {
+          if (result.status != 200 && result.status != 201) {
+            throw new Error('Failed to fetch Manifest. Please check again.');
+          } else {
+            return result.json();
+          }
+        })
+        .then((data) => {
+          if (!data) {
+            throw new Error(GENERIC_ERROR_MESSAGE);
+          }
+          setManifest(data);
+        })
+        .catch((error) => {
+          console.log('Error fetching manifest, ', error);
+          throw new Error('Failed to fetch Manifest. Please check again.');
+        });
+    } catch (error) {
+      showBoundary(error);
+    }
+  };
+
+  React.useEffect(() => {
     setAppErrorMessage(customErrorMessage);
     setAppEmptyManifestMessage(emptyManifestMessage);
 
-    // AbortController for Manifest fetch request
-    let controller;
-
     if (!manifest && manifestUrl) {
-      controller = new AbortController();
-      let requestOptions = {
-        // NOTE: try thin in Avalon
-        //credentials: 'include',
-        // headers: { 'Avalon-Api-Key': '' },
-      };
-      /**
-       * Sanitize manifest urls of query or anchor fragments included in the
-       * middle of the url: hhtp://example.com/endpoint?params/manifest
-       */
-      const sanitizedUrl = manifestUrl.replace(/[\?#].*(?=\/)/i, '');
-      try {
-        await fetch(sanitizedUrl, requestOptions, { signal: controller.signal })
-          .then((result) => {
-            if (result.status != 200 && result.status != 201) {
-              throw new Error('Failed to fetch Manifest. Please check again.');
-            } else {
-              return result.json();
-            }
-          })
-          .then((data) => {
-            if (!data) {
-              throw new Error(GENERIC_ERROR_MESSAGE);
-            }
-            setManifest(data);
-          })
-          .catch((error) => {
-            console.log('Error fetching manifest, ', error);
-            throw new Error('Failed to fetch Manifest. Please check again.');
-          });
-      } catch (error) {
-        showBoundary(error);
-      }
+      fetchManifest(manifestUrl);
     }
 
     // Cleanup Manifest fetch request on component unmount

diff --git a/src/components/MarkersDisplay/MarkerUtils/CreateMarker.js b/src/components/MarkersDisplay/MarkerUtils/CreateMarker.js
@@ -10,6 +10,14 @@ const CreateMarker = ({ newMarkerEndpoint, canvasId, handleCreate, getCurrentTim
   const [saveError, setSaveError] = React.useState(false);
   const [errorMessage, setErrorMessage] = React.useState('');
   const [markerTime, setMarkerTime] = React.useState();
+  let controller;
+
+  // Remove all fetch requests on unmount
+  React.useEffect(() => {
+    return () => {
+      controller?.abort();
+    };
+  }, []);
 
   const handleAddMarker = () => {
     const currentTime = timeToHHmmss(getCurrentTime(), true, true);
@@ -44,7 +52,8 @@ const CreateMarker = ({ newMarkerEndpoint, canvasId, handleCreate, getCurrentTim
       body: JSON.stringify(annotation)
     };
     if (csrfToken !== undefined) { requestOptions.headers['X-CSRF-Token'] = csrfToken; };
-    fetch(newMarkerEndpoint, requestOptions)
+    controller = new AbortController();
+    fetch(newMarkerEndpoint, requestOptions, { signal: controller.signal })
       .then((response) => {
         if (response.status != 201) {
           throw new Error();

diff --git a/src/components/MarkersDisplay/MarkerUtils/MarkerRow.js b/src/components/MarkersDisplay/MarkerUtils/MarkerRow.js
@@ -19,10 +19,13 @@ const MarkerRow = ({
   const [deleting, setDeleting] = React.useState(false);
   const [saveError, setSaveError] = React.useState(false);
   const [errorMessage, setErrorMessage] = React.useState('');
+  let controller;
 
-  // Remove all subscriptions on unmount
+  // Remove all fetch requests on unmount
   React.useEffect(() => {
-    return {};
+    return () => {
+      controller?.abort();
+    };
   }, []);
 
   React.useEffect(() => {
@@ -81,7 +84,8 @@ const MarkerRow = ({
       body: JSON.stringify(annotation)
     };
     if (csrfToken !== undefined) { requestOptions.headers['X-CSRF-Token'] = csrfToken; };
-    fetch(marker.id, requestOptions)
+    controller = new AbortController();
+    fetch(marker.id, requestOptions, { signal: controller.signal })
       .then((response) => {
         if (response.status != 201) {
           throw new Error();
@@ -124,7 +128,8 @@ const MarkerRow = ({
     };
     if (csrfToken !== undefined) { requestOptions.headers['X-CSRF-Token'] = csrfToken; };
     // API call for DELETE
-    fetch(marker.id, requestOptions)
+    controller = new AbortController();
+    fetch(marker.id, requestOptions, { signal: controller.signal })
       .then((response) => {
         if (response.status != 200) {
           throw new Error();

diff --git a/src/components/MarkersDisplay/MarkersDisplay.test.js b/src/components/MarkersDisplay/MarkersDisplay.test.js
@@ -45,6 +45,12 @@ describe('MarkersDisplay component', () => {
     });
 
     describe('editing markers', () => {
+      /** Reference: https://dev.to/pyyding/comment/mk2n  */
+      const abortCall = jest.fn();
+      global.AbortController = class {
+        signal = 'test-signal';
+        abort = abortCall;
+      };
       let firstEditButton, secondEditButton,
         firstDeleteButton, secondDeleteButton;
       beforeEach(() => {
@@ -154,7 +160,11 @@ describe('MarkersDisplay component', () => {
         fireEvent.click(screen.getByTestId('delete-confirm-button'));
 
         expect(deleteFetchSpy).toHaveBeenCalled();
-        expect(deleteFetchSpy).toHaveBeenCalledWith("http://example.com/playlists/1/canvas/3/marker/4", deleteOptions);
+        expect(deleteFetchSpy).toHaveBeenCalledWith(
+          "http://example.com/playlists/1/canvas/3/marker/4",
+          deleteOptions,
+          { signal: 'test-signal' },
+        );
       });
 
       test('user actions have csrf token in header when it is present in DOM', () => {
@@ -175,7 +185,11 @@ describe('MarkersDisplay component', () => {
         fireEvent.click(screen.getByTestId('delete-confirm-button'));
 
         expect(deleteFetchSpy).toHaveBeenCalled();
-        expect(deleteFetchSpy).toHaveBeenCalledWith("http://example.com/playlists/1/canvas/3/marker/4", deleteOptions);
+        expect(deleteFetchSpy).toHaveBeenCalledWith(
+          "http://example.com/playlists/1/canvas/3/marker/4",
+          deleteOptions,
+          { signal: 'test-signal' },
+        );
       });
     });
   });

diff --git a/src/components/MediaPlayer/MediaPlayer.js b/src/components/MediaPlayer/MediaPlayer.js
@@ -13,6 +13,8 @@ import {
 } from '../../context/player-context';
 import { useErrorBoundary } from "react-error-boundary";
 import { IS_ANDROID, IS_MOBILE, IS_SAFARI, IS_TOUCH_ONLY } from '@Services/browser';
+// Default language for Video.js
+import en from 'video.js/dist/lang/en.json';
 
 const PLAYER_ID = "iiif-media-player";
 
@@ -22,6 +24,7 @@ const MediaPlayer = ({
   enablePlaybackRate = false,
   enableTitleLink = false,
   withCredentials = false,
+  language = 'en'
 }) => {
   const manifestState = useManifestState();
   const playerState = usePlayerState();
@@ -76,11 +79,28 @@ const MediaPlayer = ({
   const trackScrubberRef = React.useRef();
   const timeToolRef = React.useRef();
 
+  let videoJSLangMap = React.useRef('{}');
+
   let canvasMessageTimerRef = React.useRef(null);
 
+  // FIXME:: Dynamic language imports break with rollup configuration when packaging
+  // Using dynamic imports to enforce code-splitting in webpack
+  // https://webpack.js.org/api/module-methods/#dynamic-expressions-in-import
+  const loadVideoJSLanguageMap = React.useMemo(() =>
+    async () => {
+      try {
+        const resources = await import(`../../../node_modules/video.js/dist/lang/${language}.json`);
+        videoJSLangMap.current = JSON.stringify(resources);
+      } catch (e) {
+        console.warn(`${language} is not available, defaulting to English`);
+        videoJSLangMap.current = JSON.stringify(en);
+      }
+    }, [language]);
+
   React.useEffect(() => {
     if (manifest) {
       try {
+        loadVideoJSLanguageMap();
         /*
           Always start from the start time relevant to the Canvas only in playlist contexts,
           because canvases related to playlist items always start from the given start.
@@ -286,7 +306,7 @@ const MediaPlayer = ({
         poster: isVideo ? playerConfig.poster : null,
         controls: true,
         fluid: true,
-        language: "en", // TODO:: fill this information from props
+        language: language,
         controlBar: {
           // Define and order control bar controls
           // See https://docs.videojs.com/tutorial-components.html for options of what
@@ -384,7 +404,6 @@ const MediaPlayer = ({
     setOptions(videoJsOptions);
   }, [ready, cIndex, srcIndex, canvasIsEmpty, currentTime]);
 
-
   if ((ready && options != undefined) || canvasIsEmpty) {
     return (
       <div
@@ -405,6 +424,7 @@ const MediaPlayer = ({
           loadPrevOrNext={switchPlayer}
           lastCanvasIndex={lastCanvasIndex}
           enableTitleLink={enableTitleLink}
+          videoJSLangMap={videoJSLangMap.current}
           options={options}
         />
       </div>
@@ -418,6 +438,9 @@ MediaPlayer.propTypes = {
   enableFileDownload: PropTypes.bool,
   enablePIP: PropTypes.bool,
   enablePlaybackRate: PropTypes.bool,
+  enableTitleLink: PropTypes.bool,
+  withCredentials: PropTypes.bool,
+  language: PropTypes.string,
 };
 
 export default MediaPlayer;
diff --git a/src/components/MediaPlayer/MediaPlayer.md b/src/components/MediaPlayer/MediaPlayer.md
@@ -7,6 +7,7 @@ MediaPlayer component provides a player that facilitates both audio and video me
 - `enablePlaybackRate`: accepts a Boolean value, which has a default value of `false` and is _not required_. When this is set to `true`, it adds an icon to the player's control bar which provides a menu to select a different playback speed for the media. The available speed options are 0.5x, 0.75x, 1x, 1.5x, and 2x. This icon is a VideoJS component.
 - `enableTitleLink`: accepts a Boolean value, which has a default value of `false` and is _not required_. When this is set to `true`, it adds a title bar to the video player which displays `Manifest Label - Active Canvas Label` with an href attribute linking to the URL in the active canvas's `id`. This is a custom VideoJS component added to the VideoJS instance in Ramp.
 - `withCredentials`: accepts a Boolean value, which has a default value of `false` and is _not required_. Once this is set to `true` it causes the VideoJS component to include any available `Authentication` and `Cookie` headers with [XHR requests](https://developer.mozilla.org/en-US/docs/Web/API/XMLHttpRequest/withCredentials). There are special server-side CORS requirements that go along with this option – specifically, the streaming server should include an appropriate [`Access-Control-Allow-Credentials`](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Access-Control-Allow-Credentials) header, and a non-wildcard [`Access-Control-Allow-Origin`](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Access-Control-Allow-Origin) specifying the server originating the request.
+- `language`: accepts a String value for a [standard language code](https://www.iana.org/assignments/language-subtag-registry/language-subtag-registry), which has a default value of `en` and is _not required_. If the given language code doesn't match with any of the existing language file with `.json` extension, Ramp defaults to English language.
 
 To import and use this component from the library;
 ```js static
Original file line number	Diff line number	Diff line change
Expand Up		@@ -47,4 +47,3 @@ IIIFPlayer.propTypes = {
		startCanvasTime: PropTypes.number,
		};

		IIIFPlayer.defaultProps = {};