Skip to content

Commit

Permalink
Migrate annotations to react18 (#716)
Browse files Browse the repository at this point in the history
* feat(react18): Update to React 18, replace ReactDOM with ReactDOM.client

Migrated codebase from React 17 to React 18. Updated `ReactDOM` imports and usage across multiple files to use `ReactDOM.client`. This included changes to various test files, components, utilities, and the package dependencies.

* feat(react18): refactor imports and remove unnecessary dependencies

Simplified import syntax across multiple components, moving from wildcard imports to direct imports. Removed unnecessary `usePrevious` hooks to streamline popup update logic and updated `yarn.lock` to newer versions of several dependencies.

* feat(react18): refactor imports and remove unnecessary dependencies

Refactored components to improve readability and consistency. Updated package dependencies and ESLint configurations for better code quality and compatibility with the latest libraries. Adjustments include moving to "react-jsx," improving TypeScript definitions, and simplifying utility functions.

* feat(react18): Mock react-redux in RegionAnnotation tests

Mocking the entire react-redux module to control useSelector behavior. This ensures test consistency by avoiding real state dependencies.

* feat(react18): Refactor PopupReply imports

Replaced `ReactRedux.useSelector` with `useSelector` in PopupReply for consistency. Also, removed unused and commented-out code in global type definitions to improve code clarity.

* feat(react18): Disable ESLint for undefined variables in popperMock

Added a directive to disable ESLint checks for undefined variables in the popperMock.js script. This change ensures that the createPopper mock function can be defined without ESLint errors during Jest testing.

* feat(react18): Add back tests, remove unused files

* In BaseManager, added back deleting the root react node on destroy
* Removed usePrevious files - no longer used
* Added mocks of a mangager to fix ImageAnnotator unit tests of getReference and init functions
* Minor style changes

* feat(react18): add optional chaining to ensure not null

* feat(react18): Remove whitespace

* feat(react18): Add useMemo to avoid re-renders and fix eslint issue

---------

Co-authored-by: Patrick Murphy <[email protected]>
  • Loading branch information
mickr and patlm authored Dec 4, 2024
1 parent a19740e commit cf6a34c
Show file tree
Hide file tree
Showing 55 changed files with 3,170 additions and 1,767 deletions.
11 changes: 7 additions & 4 deletions .eslintrc.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
const eslintrc = require.resolve('@box/frontend/eslint/eslintrc.js');

module.exports = {
extends: [eslintrc],
extends: [
require.resolve('@box/frontend/eslint/base'),
require.resolve('@box/frontend/eslint/react'),
require.resolve('@box/frontend/eslint/typescript'),
],
rules: {
'class-methods-use-this': 0, // fixme
'flowtype/no-types-missing-file-annotation': 'off', // Allows types in TS files
Expand All @@ -22,8 +24,9 @@ module.exports = {
{
files: ['**/__mocks__/*', '**/__tests__/*'],
rules: {
'@typescript-eslint/camelcase': 'off',
'@typescript-eslint/naming-convention': 'off',
'@typescript-eslint/no-non-null-assertion': 'off',
'import/no-import-module-exports': 'off'
},
},
{
Expand Down
70 changes: 37 additions & 33 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -16,18 +16,20 @@
"yarn": ">=1.10.x"
},
"devDependencies": {
"@babel/cli": "^7.8.4",
"@babel/core": "^7.9.0",
"@babel/plugin-proposal-class-properties": "^7.8.3",
"@babel/plugin-transform-flow-strip-types": "^7.9.0",
"@babel/plugin-transform-object-assign": "^7.8.3",
"@babel/plugin-transform-runtime": "^7.9.0",
"@babel/polyfill": "^7.8.7",
"@babel/preset-env": "^7.9.5",
"@babel/preset-react": "^7.9.4",
"@babel/preset-typescript": "^7.9.0",
"@box/frontend": "^6.4.0",
"@babel/cli": "^7.25.9",
"@babel/core": "^7.25.9",
"@babel/eslint-parser": "^7.24.7",
"@babel/plugin-proposal-class-properties": "^7.18.6",
"@babel/plugin-proposal-object-rest-spread": "^7.20.7",
"@babel/plugin-transform-flow-strip-types": "^7.25.9",
"@babel/plugin-transform-object-assign": "^7.25.9",
"@babel/plugin-transform-runtime": "^7.25.9",
"@babel/preset-env": "^7.25.9",
"@babel/preset-react": "^7.25.9",
"@babel/preset-typescript": "^7.25.9",
"@box/frontend": "^10.0.0",
"@box/languages": "^1.1.0",
"@cfaester/enzyme-adapter-react-18": "^0.8.0",
"@commitlint/cli": "^8.3.5",
"@commitlint/config-conventional": "^8.2.0",
"@formatjs/intl-pluralrules": "^1.5.5",
Expand All @@ -39,13 +41,12 @@
"@types/enzyme": "^3.10.5",
"@types/jest": "^24.9.0",
"@types/lodash": "^4.14.150",
"@types/react": "^17.0.2",
"@types/react-dom": "^17.0.1",
"@types/react-redux": "^7.1.16",
"@types/react": "18.3.12",
"@types/react-dom": "18.3.1",
"@types/react-redux": "^7.1.33",
"@types/uuid": "^8.3.0",
"@typescript-eslint/eslint-plugin": "^2.29.0",
"@typescript-eslint/parser": "^2.29.0",
"@wojtekmaj/enzyme-adapter-react-17": "^0.4.1",
"@typescript-eslint/eslint-plugin": "^7.3.1",
"@typescript-eslint/parser": "^7.3.1",
"autoprefixer": "^9.7.6",
"axios": "^0.24.0",
"babel-eslint": "^10.1.0",
Expand All @@ -59,26 +60,26 @@
"circular-dependency-plugin": "^5.2.0",
"classnames": "^2.2.5",
"conventional-github-releaser": "^3.1.3",
"core-js": "^3.6.5",
"core-js": "^3.38.1",
"css-loader": "^3.5.2",
"cypress": "^4.4.1",
"draft-js": "0.10.5",
"enzyme": "^3.11.0",
"enzyme-to-json": "^3.4.4",
"eslint": "^6.7.2",
"eslint-config-airbnb": "^18.1.0",
"eslint-config-prettier": "^6.11.0",
"eslint": "^8.57.0",
"eslint-config-airbnb": "^19.0.4",
"eslint-config-prettier": "^9.1.0",
"eslint-plugin-babel": "^5.3.0",
"eslint-plugin-cypress": "^2.10.3",
"eslint-plugin-flowtype": "^4.7.0",
"eslint-plugin-formatjs": "^1.5.4",
"eslint-plugin-import": "^2.20.2",
"eslint-plugin-flowtype": "^8.0.3",
"eslint-plugin-formatjs": "^4.13.3",
"eslint-plugin-import": "^2.26.0",
"eslint-plugin-jest": "^23.8.2",
"eslint-plugin-jsx-a11y": "^6.2.3",
"eslint-plugin-lodash": "^6.0.0",
"eslint-plugin-prettier": "^3.1.3",
"eslint-plugin-react": "^7.19.0",
"eslint-plugin-react-hooks": "^1.7.0",
"eslint-plugin-lodash": "^7.4.0",
"eslint-plugin-prettier": "^5.2.1",
"eslint-plugin-react": "^7.34.3",
"eslint-plugin-react-hooks": "^4.6.0",
"formik": "^2.1.4",
"husky": "^3.1.0",
"jest": "^24.9.0",
Expand All @@ -92,13 +93,14 @@
"postcss-loader": "^3.0.0",
"prettier": "^1.19.1",
"raw-loader": "^4.0.1",
"react": "^17.0.1",
"react-dom": "^17.0.1",
"react": "18.3.1",
"react-dom": "18.3.1",
"react-intl": "6.4.2",
"react-redux": "^7.2.2",
"react-redux": "^9.1.2",
"react-tether": "1.0.5",
"react-textarea-autosize": "^7.1.2",
"redux": "^4.0.5",
"regenerator-runtime": "^0.13.9",
"sass": "^1.64.1",
"sass-loader": "^8.0.2",
"scroll-into-view-if-needed": "^2.2.24",
Expand All @@ -109,7 +111,7 @@
"stylelint-order": "^3.1.1",
"tabbable": "^1.1.3",
"terser-webpack-plugin": "^4.2.3",
"typescript": "^3.8.3",
"typescript": "4.9.5",
"uuid": "^8.3.1",
"wait-on": "^4.0.2",
"webpack": "^4.47.0",
Expand Down Expand Up @@ -153,6 +155,8 @@
}
},
"resolutions": {
"**/react-intl/**/@types/react": "^17.0.2"
"**/react-intl/**/@types/react": "^18.3.12",
"**/@types/react": "^18.3.12",
"**/@types/react-dom": "^18.2.0"
}
}
2 changes: 1 addition & 1 deletion scripts/jest/enzyme-adapter.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import 'core-js/es/map';
import 'core-js/es/set';
import Enzyme, { mount, shallow } from 'enzyme';
import Adapter from '@wojtekmaj/enzyme-adapter-react-17';
import Adapter from '@cfaester/enzyme-adapter-react-18';

Enzyme.configure({ adapter: new Adapter() });

Expand Down
1 change: 1 addition & 0 deletions scripts/jest/popperMock.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/* eslint-disable no-undef */
const createPopper = jest.fn(() => ({
destroy: jest.fn(),
forceUpdate: jest.fn(),
Expand Down
5 changes: 1 addition & 4 deletions src/@types/global.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,5 @@
/* eslint-disable @typescript-eslint/no-explicit-any, @typescript-eslint/no-namespace */
declare namespace Intl {
const RelativeTimeFormat: {};
}

// eslint-disable-next-line @typescript-eslint/no-unused-vars
declare namespace NodeJS {
interface Global {
window: any;
Expand Down
16 changes: 10 additions & 6 deletions src/common/BaseManager.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import * as ReactDOM from 'react-dom';
import * as ReactDOM from 'react-dom/client';
import { IntlShape } from 'react-intl';
import { Store } from 'redux';
import { applyResinTags } from '../utils/resin';
Expand Down Expand Up @@ -28,13 +28,17 @@ export default class BaseManager implements Manager {

reactEl: HTMLElement;

root: ReactDOM.Root | null = null;

constructor({ location = 1, referenceEl, resinTags = {} }: Options) {
this.location = location;
this.reactEl = this.insert(referenceEl, {
...resinTags,
feature: 'annotations',
});

this.root = ReactDOM.createRoot(this.reactEl);

this.decorate();
}

Expand All @@ -43,11 +47,11 @@ export default class BaseManager implements Manager {
}

destroy(): void {
ReactDOM.unmountComponentAtNode(this.reactEl);

const parentEl = this.reactEl.parentNode;
if (parentEl) {
parentEl.removeChild(this.reactEl);
if (this.root) {
this.root.unmount();
}
if (this.reactEl.parentNode) {
this.reactEl.parentNode.removeChild(this.reactEl);
}
}

Expand Down
9 changes: 6 additions & 3 deletions src/common/DeselectManager.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import * as React from 'react';
import * as ReactDOM from 'react-dom';
import * as ReactDOM from 'react-dom/client';
import { Provider } from 'react-redux';
import { Store } from 'redux';
import BaseManager from './BaseManager';
Expand All @@ -20,11 +20,14 @@ export default class DeselectManager extends BaseManager {
}

render({ store }: Props): void {
ReactDOM.render(
if (!this.root) {
this.root = ReactDOM.createRoot(this.reactEl);
}

this.root.render(
<Provider store={store}>
<DeselectListener />
</Provider>,
this.reactEl,
);
}
}
11 changes: 8 additions & 3 deletions src/common/__tests__/BaseManager-test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,12 @@
import * as ReactDOM from 'react-dom';
import BaseManager, { Options, Props } from '../BaseManager';

jest.mock('react-dom');
jest.mock('react-dom/client', () => ({
createRoot: jest.fn().mockReturnValue({
render: jest.fn(),
unmount: jest.fn(),
}),
}));

class TestBaseManager extends BaseManager {
// eslint-disable-next-line @typescript-eslint/no-empty-function
decorate(): void {}
Expand Down Expand Up @@ -47,7 +52,7 @@ describe('BaseManager', () => {
const wrapper = getWrapper();
wrapper.destroy();

expect(ReactDOM.unmountComponentAtNode).toHaveBeenCalledWith(wrapper.reactEl);
expect(wrapper.root?.unmount).toHaveBeenCalled();
expect(removeChildSpy).toHaveBeenCalledWith(wrapper.reactEl);
});
});
Expand Down
14 changes: 8 additions & 6 deletions src/common/__tests__/DeselectManager-test.tsx
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
import ReactDOM from 'react-dom';
import ReactDOM from 'react-dom/client';
import DeselectManager from '../DeselectManager';
import { createStore } from '../../store';
import { Options } from '../BaseManager';

jest.mock('react-dom', () => ({
render: jest.fn(),
unmountComponentAtNode: jest.fn(),
jest.mock('react-dom/client', () => ({
createRoot: jest.fn().mockReturnValue({
render: jest.fn(),
unmount: jest.fn(),
}),
}));

describe('DeselectManager', () => {
Expand Down Expand Up @@ -34,10 +36,10 @@ describe('DeselectManager', () => {
describe('render()', () => {
test('should format the props and pass them to the underlying components', () => {
const wrapper = getWrapper();

const root = ReactDOM.createRoot(rootEl);
wrapper.render({ store: createStore() });

expect(ReactDOM.render).toHaveBeenCalled();
expect(root.render).toHaveBeenCalled();
});
});
});
30 changes: 0 additions & 30 deletions src/common/__tests__/usePrevious-test.tsx

This file was deleted.

2 changes: 1 addition & 1 deletion src/common/useAutoScroll/__tests__/useAutoScroll-test.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import * as React from 'react';
import React from 'react';
import { mount, ReactWrapper } from 'enzyme';
import useAutoScroll from '../useAutoScroll';

Expand Down
2 changes: 1 addition & 1 deletion src/common/useAutoScroll/useAutoScroll.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import * as React from 'react';
import React from 'react';
import noop from 'lodash/noop';
import { getScrollParent } from './util';

Expand Down
13 changes: 0 additions & 13 deletions src/common/usePrevious.ts

This file was deleted.

18 changes: 18 additions & 0 deletions src/components/Popups/Popper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,3 +31,21 @@ export default function create(
): Instance {
return popper.createPopper(reference, popup, mergeWith({}, defaults, options, merger) as Options);
}

export function clientBoundingRect(height: number, width: number, x: number, y: number): DOMRect {
const rect = {
bottom: y + height,
height,
left: x,
right: x + width,
top: y,
width,
x,
y,
};

return {
...rect,
toJSON: () => ({ ...rect }),
};
}
2 changes: 1 addition & 1 deletion src/components/Popups/PopupBase.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import * as React from 'react';
import React from 'react';
import classNames from 'classnames';
import createPopper, { Instance, Options, PopupReference } from './Popper';
import './PopupBase.scss';
Expand Down
Loading

0 comments on commit cf6a34c

Please sign in to comment.