From cec72ca7d6c18df82df834e48a512f80e705e8ff Mon Sep 17 00:00:00 2001 From: Chris Beer Date: Thu, 14 Dec 2023 09:10:51 -0800 Subject: [PATCH 1/3] Update to React 18 --- .eslintrc | 2 +- .../mirador/invalid-api-response.test.js | 1 + __tests__/src/actions/canvas.test.js | 2 +- __tests__/src/components/App.test.js | 4 +-- .../src/components/PrimaryWindow.test.js | 6 +++-- __tests__/src/lib/MiradorViewer.test.js | 10 ++++--- __tests__/utils/test-utils.js | 4 +-- package.json | 27 +++++++++---------- src/lib/MiradorViewer.js | 13 ++++----- src/state/createStore.js | 4 +-- src/state/selectors/manifests.js | 2 +- webpack.config.js | 4 --- 12 files changed, 39 insertions(+), 40 deletions(-) diff --git a/.eslintrc b/.eslintrc index 167c168ef..596e1115f 100644 --- a/.eslintrc +++ b/.eslintrc @@ -46,7 +46,7 @@ "react/react-in-jsx-scope": "off", "react-hooks/exhaustive-deps": "error", "testing-library/render-result-naming-convention": "off", - "testing-library/no-render-in-setup": [ + "testing-library/no-render-in-lifecycle": [ "error", { "allowTestingFrameworkSetupHook": "beforeEach" diff --git a/__tests__/integration/mirador/invalid-api-response.test.js b/__tests__/integration/mirador/invalid-api-response.test.js index d0edd9de1..a2f191d2a 100644 --- a/__tests__/integration/mirador/invalid-api-response.test.js +++ b/__tests__/integration/mirador/invalid-api-response.test.js @@ -1,6 +1,7 @@ describe('Mirador Invalid API Response Handler Test', () => { /** */ async function fetchManifest(uri) { + await expect(page).toMatchElement('button'); await page.evaluate(() => { document.querySelector('#addBtn').click(); }); diff --git a/__tests__/src/actions/canvas.test.js b/__tests__/src/actions/canvas.test.js index 4d6d92e4a..fead977ee 100644 --- a/__tests__/src/actions/canvas.test.js +++ b/__tests__/src/actions/canvas.test.js @@ -1,5 +1,5 @@ import configureMockStore from 'redux-mock-store'; -import thunk from 'redux-thunk'; +import { thunk } from 'redux-thunk'; import * as actions from '../../../src/state/actions'; import ActionTypes from '../../../src/state/actions/action-types'; diff --git a/__tests__/src/components/App.test.js b/__tests__/src/components/App.test.js index 868c73ad9..53a7236ff 100644 --- a/__tests__/src/components/App.test.js +++ b/__tests__/src/components/App.test.js @@ -16,8 +16,6 @@ describe('App', () => { createWrapper(); expect(screen.queryByRole('main')).not.toBeInTheDocument(); - await screen.findByText('welcome'); - - expect(screen.getByRole('main')).toBeInTheDocument(); + expect(await screen.findByRole('main')).toBeInTheDocument(); }); }); diff --git a/__tests__/src/components/PrimaryWindow.test.js b/__tests__/src/components/PrimaryWindow.test.js index ad78ebf4b..1eef93a4f 100644 --- a/__tests__/src/components/PrimaryWindow.test.js +++ b/__tests__/src/components/PrimaryWindow.test.js @@ -1,4 +1,4 @@ -import { render, screen } from 'test-utils'; +import { render, screen, waitFor } from 'test-utils'; import { PrimaryWindow } from '../../../src/components/PrimaryWindow'; /** create wrapper */ @@ -31,7 +31,9 @@ describe('PrimaryWindow', () => { it('should render if fetching is complete and view is gallery', async () => { createWrapper({ isFetching: false, view: 'gallery' }); await screen.findByTestId('test-window'); - expect(document.querySelector('#xyz-gallery')).toBeInTheDocument(); // eslint-disable-line testing-library/no-node-access + await waitFor(() => { + expect(document.querySelector('#xyz-gallery')).toBeInTheDocument(); // eslint-disable-line testing-library/no-node-access + }); }); it('should render and if manifest is collection and isCollectionDialogVisible', async () => { render(
); diff --git a/__tests__/src/lib/MiradorViewer.test.js b/__tests__/src/lib/MiradorViewer.test.js index d5e22093d..3103eea54 100644 --- a/__tests__/src/lib/MiradorViewer.test.js +++ b/__tests__/src/lib/MiradorViewer.test.js @@ -1,4 +1,4 @@ -import { render, screen } from 'test-utils'; +import { act, render, screen } from 'test-utils'; import MiradorViewer from '../../../src/lib/MiradorViewer'; jest.unmock('react-i18next'); @@ -23,7 +23,7 @@ describe('MiradorViewer', () => { expect(instance.store.dispatch).toBeDefined(); }); it('renders via ReactDOM', () => { - const instance = new MiradorViewer({ id: 'mirador' }); // eslint-disable-line no-unused-vars + act(() => { new MiradorViewer({ id: 'mirador' }); }); // eslint-disable-line no-new expect(screen.getByTestId('container')).not.toBeEmptyDOMElement(); }); @@ -137,9 +137,11 @@ describe('MiradorViewer', () => { describe('unmount', () => { it('unmounts via ReactDOM', () => { - const instance = new MiradorViewer({ id: 'mirador' }); + let instance; + + act(() => { instance = new MiradorViewer({ id: 'mirador' }); }); expect(screen.getByTestId('container')).not.toBeEmptyDOMElement(); - instance.unmount(); + act(() => { instance.unmount(); }); expect(screen.getByTestId('container')).toBeEmptyDOMElement(); }); }); diff --git a/__tests__/utils/test-utils.js b/__tests__/utils/test-utils.js index 7104dec6b..2e30b405c 100644 --- a/__tests__/utils/test-utils.js +++ b/__tests__/utils/test-utils.js @@ -2,7 +2,7 @@ import { Provider } from 'react-redux'; import { render } from '@testing-library/react'; import PropTypes from 'prop-types'; import { createStore, applyMiddleware } from 'redux'; -import thunkMiddleware from 'redux-thunk'; +import { thunk } from 'redux-thunk'; import { createTheme, ThemeProvider } from '@mui/material/styles'; import createRootReducer from '../../src/state/reducers/rootReducer'; import settings from '../../src/config/settings'; @@ -18,7 +18,7 @@ function renderWithProviders( { preloadedState = {}, // Automatically create a store instance if no store was passed in - store = createStore(rootReducer, preloadedState, applyMiddleware(thunkMiddleware)), + store = createStore(rootReducer, preloadedState, applyMiddleware(thunk)), ...renderOptions } = {}, ) { diff --git a/package.json b/package.json index d77b1f238..947f3680c 100644 --- a/package.json +++ b/package.json @@ -54,7 +54,7 @@ "openseadragon": "^2.4.2 || ^3.0.0 || ^4.0.0", "prop-types": "^15.6.2", "rdndmb-html5-to-touch": "^8.0.0", - "re-reselect": "^4.0.0", + "re-reselect": "^5.0.0", "react-copy-to-clipboard": "^5.0.1", "react-dnd": "^16.0.0", "react-dnd-html5-backend": "^16.0.0", @@ -65,17 +65,16 @@ "react-image": "^4.0.1", "react-intersection-observer": "^9.0.0", "react-mosaic-component": "^6.0.0", - "react-redux": "^7.1.0 || ^8.0.0", + "react-redux": "^8.0.0 || ^9.0.0", "react-resize-observer": "^1.1.1", "react-rnd": "^10.1", "react-sizeme": "^2.6.7 || ^3.0.0", "react-virtualized-auto-sizer": "^1.0.2", "react-window": "^1.8.5", - "redux": "^4.0.5", - "redux-devtools-extension": "^2.13.2", + "redux": "^5.0.0", "redux-saga": "^1.1.3", - "redux-thunk": "^2.3.0", - "reselect": "^4.0.0", + "redux-thunk": "^3.1.0", + "reselect": "^5.0.0", "stylis": "^4.3.0", "stylis-plugin-rtl": "^2.1.1", "url": "^0.11.0", @@ -94,10 +93,10 @@ "@pmmmwh/react-refresh-webpack-plugin": "^0.5.4", "@testing-library/dom": "^9.2.0", "@testing-library/jest-dom": "^6.1.5", - "@testing-library/react": "^12.1.5", + "@testing-library/react": "^14.1.2", "@testing-library/user-event": "^14.4.3", - "@typescript-eslint/eslint-plugin": "^5.15.0", - "@typescript-eslint/parser": "^5.15.0", + "@typescript-eslint/eslint-plugin": "^6.14.0", + "@typescript-eslint/parser": "^6.14.0", "babel-jest": "^29.3.1", "babel-loader": "^9.1.0", "babel-plugin-lodash": "^3.3.4", @@ -117,7 +116,7 @@ "eslint-plugin-jsx-a11y": "^6.4.1", "eslint-plugin-react": "^7.29.4", "eslint-plugin-react-hooks": "^4.6.0", - "eslint-plugin-testing-library": "^5.10.2", + "eslint-plugin-testing-library": "^6.2.0", "glob": "^10.3.0", "http-server": "^14.1.0", "jest": "^29.3.1", @@ -126,9 +125,9 @@ "jest-puppeteer": "^9.0.2", "jsdom": "^23.0.0", "puppeteer": "^21.0.0", - "react": "^17.0.0", + "react": "^18.0.0", "react-dnd-test-backend": "^16.0.1", - "react-dom": "^17.0.0", + "react-dom": "^18.0.0", "react-refresh": "^0.14.0", "redux-mock-store": "^1.5.1", "redux-saga-test-plan": "^4.0.0-rc.3", @@ -138,7 +137,7 @@ "webpack-dev-server": "^4.7.4" }, "peerDependencies": { - "react": "^17.0.0", - "react-dom": "^17.0.0" + "react": "^18.0.0", + "react-dom": "^18.0.0" } } diff --git a/src/lib/MiradorViewer.js b/src/lib/MiradorViewer.js index c6fcf583e..8c9c23bb7 100644 --- a/src/lib/MiradorViewer.js +++ b/src/lib/MiradorViewer.js @@ -1,4 +1,4 @@ -import ReactDOM from 'react-dom'; +import { createRoot } from 'react-dom/client'; import { Provider } from 'react-redux'; import HotApp from '../components/App'; import { @@ -20,10 +20,9 @@ class MiradorViewer { if (config.id) { this.container = document.getElementById(config.id); - config.id && ReactDOM.render( - this.render(), - this.container, - ); + this.root = createRoot(this.container); + + this.root.render(this.render()); } } @@ -42,7 +41,9 @@ class MiradorViewer { * Cleanup method to unmount Mirador from the dom */ unmount() { - this.container && ReactDOM.unmountComponentAtNode(this.container); + if (!this.root) return; + + this.root.unmount(); } } diff --git a/src/state/createStore.js b/src/state/createStore.js index c11585926..b593f54e9 100644 --- a/src/state/createStore.js +++ b/src/state/createStore.js @@ -3,7 +3,7 @@ // state normalisation // (normalizer library) -import thunkMiddleware from 'redux-thunk'; +import { thunk } from 'redux-thunk'; import createSagaMiddleware from 'redux-saga'; import { combineReducers, createStore, applyMiddleware } from 'redux'; import { composeWithDevTools } from '@redux-devtools/extension'; @@ -27,7 +27,7 @@ function configureStore(pluginReducers, pluginSagas = []) { const store = createStore( rootReducer, composeWithDevTools( - applyMiddleware(thunkMiddleware, sagaMiddleware), + applyMiddleware(thunk, sagaMiddleware), ), ); diff --git a/src/state/selectors/manifests.js b/src/state/selectors/manifests.js index 496d84562..aefe164ce 100644 --- a/src/state/selectors/manifests.js +++ b/src/state/selectors/manifests.js @@ -1,5 +1,5 @@ import { createSelector } from 'reselect'; -import createCachedSelector from 're-reselect'; +import { createCachedSelector } from 're-reselect'; import { PropertyValue, Utils, Resource } from 'manifesto.js'; import getThumbnail from '../../lib/ThumbnailFactory'; import asArray from '../../lib/asArray'; diff --git a/webpack.config.js b/webpack.config.js index f31af7318..6e3c1e838 100644 --- a/webpack.config.js +++ b/webpack.config.js @@ -45,10 +45,6 @@ const baseConfig = mode => ({ }), ], resolve: { - alias: { - 'react/jsx-dev-runtime': 'react/jsx-dev-runtime.js', - 'react/jsx-runtime': 'react/jsx-runtime.js', - }, extensions: ['.js'], }, }); From d6e71d650584f44bad93d739ac9e6e7a0b8be200 Mon Sep 17 00:00:00 2001 From: Chris Beer Date: Thu, 14 Dec 2023 12:47:42 -0800 Subject: [PATCH 2/3] Get rid of a function as an input to a selector --- src/state/selectors/layers.js | 21 +++++++++++++++------ src/state/selectors/viewer.js | 4 +--- 2 files changed, 16 insertions(+), 9 deletions(-) diff --git a/src/state/selectors/layers.js b/src/state/selectors/layers.js index e75c165b7..829e77ec3 100644 --- a/src/state/selectors/layers.js +++ b/src/state/selectors/layers.js @@ -17,16 +17,25 @@ export const getCanvasLayers = createSelector( }, ); +const EMPTY_ARRAY = Object.freeze([]); + +export const getLayersForWindow = createSelector( + [ + state => miradorSlice(state).layers, + (state, { windowId }) => windowId, + ], + (layers, windowId) => (layers ? (layers[windowId] || EMPTY_ARRAY) : EMPTY_ARRAY), +); + /** * Get the layer state for a particular canvas */ export const getLayers = createSelector( [ - state => miradorSlice(state).layers || {}, - (state, { windowId }) => windowId, + getLayersForWindow, (state, { canvasId }) => canvasId, ], - (layers, windowId, canvasId) => (layers[windowId] || {})[canvasId], + (layers, canvasId) => layers[canvasId], ); /** @@ -63,11 +72,11 @@ export const getSortedLayers = createSelector( export const getLayersForVisibleCanvases = createSelector( [ getVisibleCanvasIds, - (state, { windowId }) => (canvasId => getLayers(state, { canvasId, windowId })), + getLayersForWindow, ], - (canvasIds, getLayersForCanvas) => ( + (canvasIds, layers) => ( canvasIds.reduce((acc, canvasId) => { - acc[canvasId] = getLayersForCanvas(canvasId); + acc[canvasId] = layers[canvasId]; return acc; }, {}) ), diff --git a/src/state/selectors/viewer.js b/src/state/selectors/viewer.js index 27b599255..fc538a29b 100644 --- a/src/state/selectors/viewer.js +++ b/src/state/selectors/viewer.js @@ -7,8 +7,6 @@ import { getSequenceViewingDirection } from './sequences'; /** Instantiate a manifesto instance */ export const getCurrentCanvasWorld = createSelector( - getVisibleCanvases, - getLayersForVisibleCanvases, - getSequenceViewingDirection, + [getVisibleCanvases, getLayersForVisibleCanvases, getSequenceViewingDirection], (canvases, layers, viewingDirection) => new CanvasWorld(canvases, layers, viewingDirection), ); From df8f56f459e4edb3d6288f669f9bc3e0c64ebda8 Mon Sep 17 00:00:00 2001 From: Chris Beer Date: Thu, 14 Dec 2023 11:31:44 -0800 Subject: [PATCH 3/3] Don't construct new objects in selector inputs --- src/state/selectors/companionWindows.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/state/selectors/companionWindows.js b/src/state/selectors/companionWindows.js index 914c73a2f..c86d9de83 100644 --- a/src/state/selectors/companionWindows.js +++ b/src/state/selectors/companionWindows.js @@ -98,9 +98,9 @@ const getCompanionWindowIdsOfWindow = createSelector( export const getCompanionWindowsForPosition = createSelector( [ getCompanionWindowsOfWindow, - (state, { position }) => ({ position }), + (state, { position }) => (position), ], - (companionWindows, { position }) => companionWindows[position] || EMPTY_ARRAY, + (companionWindows, position) => companionWindows[position] || EMPTY_ARRAY, ); /** @@ -113,9 +113,9 @@ export const getCompanionWindowsForPosition = createSelector( export const getCompanionWindowsForContent = createSelector( [ getCompanionWindowsOfWindow, - (state, { content }) => ({ content }), + (state, { content }) => (content), ], - (companionWindows, { content }) => ( + (companionWindows, content) => ( [].concat(...Object.values(companionWindows)).filter(w => w.content === content) ), ); @@ -126,9 +126,9 @@ const EMPTY_ARRAY = []; export const getCompanionWindowIdsForPosition = createSelector( [ getCompanionWindowIdsOfWindow, - (state, { position }) => ({ position }), + (state, { position }) => (position), ], - (companionWindowIds, { position }) => companionWindowIds[position] || EMPTY_ARRAY, + (companionWindowIds, position) => companionWindowIds[position] || EMPTY_ARRAY, ); /**