Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Yuhsuan/react 18 upgrade #2426

Open
wants to merge 16 commits into
base: dev
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
193 changes: 92 additions & 101 deletions package-lock.json

Large diffs are not rendered by default.

19 changes: 9 additions & 10 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,17 +10,16 @@
"@blueprintjs/select": "^5.2.2",
"@blueprintjs/table": "^5.1.8",
"@testing-library/jest-dom": "^6.4.8",
"@testing-library/react": "^12.1.5",
"@testing-library/react": "^16.0.1",
"@testing-library/user-event": "^14.5.2",
"@types/jest": "^29.5.12",
"@types/jquery": "^3.5.30",
"@types/lodash": "^4.17.7",
"@types/node": "^20.14.13",
"@types/plotly.js": "^2.33.3",
"@types/prismjs": "^1.26.4",
"@types/react": "^17.0.80",
"@types/react-color": "^3.0.12",
"@types/react-dom": "^17.0.25",
"@types/react": "^18.3.12",
"@types/react-dom": "^18.3.1",
"@types/react-plotly.js": "^2.6.3",
"@types/react-virtualized-auto-sizer": "^1.0.4",
"@types/react-window": "^1.8.8",
Expand All @@ -42,7 +41,7 @@
"idb": "^8.0.0",
"jest-canvas-mock": "^2.5.2",
"jwt-decode": "^4.0.0",
"konva": "^8.4.3",
"konva": "^9.3.16",
"lodash": "^4.17.15",
"mnemonist": "^0.39.8",
"mobx": "^6.13.1",
Expand All @@ -58,12 +57,12 @@
"protobufjs": "^7.3.2",
"protobufjs-cli": "1.1.2",
"raw-loader": "^4.0.2",
"react": "^17.0.2",
"react": "^18.3.1",
"react-chartjs-2": "^5.2.0",
"react-color": "^2.17.3",
"react-dom": "^17.0.2",
"react-dom": "^18.3.1",
"react-iframe": "^1.8.5",
"react-konva": "^17.0.2-6",
"react-konva": "^18.2.10",
"react-plotly.js": "^2.6.0",
"react-resize-detector": "^9.1.1",
"react-rnd": "^10.4.11",
Expand Down Expand Up @@ -204,8 +203,8 @@
"usehooks-ts": "^3.1.0"
},
"overrides": {
"react": "^17.0.2",
"react-dom": "^17.0.2",
"react": "^18.3.1",
"react-dom": "^18.3.1",
"typescript": "^5.5.4",
"fork-ts-checker-webpack-plugin": "^6.5.3",
"@typescript-eslint/typescript-estree": "^7.15.0",
Expand Down
24 changes: 24 additions & 0 deletions patches/golden-layout+1.5.9.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
diff --git a/node_modules/golden-layout/dist/goldenlayout.js b/node_modules/golden-layout/dist/goldenlayout.js
index 736f9ec..9c59284 100644
--- a/node_modules/golden-layout/dist/goldenlayout.js
+++ b/node_modules/golden-layout/dist/goldenlayout.js
@@ -5265,7 +5265,9 @@ lm.utils.copy( lm.utils.ReactComponentHandler.prototype, {
* @returns {void}
*/
_render: function() {
- this._reactComponent = ReactDOM.render( this._getReactComponent(), this._container.getElement()[ 0 ] );
+ this._reactRoot = createRoot( this._container.getElement()[ 0 ] );
+ this._reactRoot.render( this._getReactComponent() );
+ this._reactComponent = this._getReactComponent();
this._originalComponentWillUpdate = this._reactComponent.componentWillUpdate || function() {
};
this._reactComponent.componentWillUpdate = this._onUpdate.bind( this );
@@ -5281,7 +5283,7 @@ lm.utils.copy( lm.utils.ReactComponentHandler.prototype, {
* @returns {void}
*/
_destroy: function() {
- ReactDOM.unmountComponentAtNode( this._container.getElement()[ 0 ] );
+ this._reactRoot.unmount();
this._container.off( 'open', this._render, this );
this._container.off( 'destroy', this._destroy, this );
},
20 changes: 20 additions & 0 deletions patches/react-split-pane+0.1.92.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
diff --git a/node_modules/react-split-pane/index.d.ts b/node_modules/react-split-pane/index.d.ts
index d116f54..54452b7 100644
--- a/node_modules/react-split-pane/index.d.ts
+++ b/node_modules/react-split-pane/index.d.ts
@@ -5,6 +5,7 @@ export type Size = string | number;
export type Split = 'vertical' | 'horizontal';

export type SplitPaneProps = {
+ children: React.ReactNode[];
allowResize?: boolean;
className?: string;
primary?: 'first' | 'second';
@@ -59,6 +60,7 @@ declare class SplitPane extends React.Component<
export default SplitPane;

export type PaneProps = {
+ children: React.ReactNode;
className?: string;
size?: Size;
split?: Split;
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import {getColorForTheme, SWATCH_COLORS} from "utilities";

import "./CatalogOverlayPlotSettingsPanelComponent.scss";

const IconWrapper = (path: React.SVGProps<SVGPathElement>, color: string, fill: boolean, strokeWidth = 2, viewboxDefault = 16) => {
const IconWrapper = (path: React.ReactNode, color: string, fill: boolean, strokeWidth = 2, viewboxDefault = 16) => {
let fillColor = color;
if (!fill) {
fillColor = "none";
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// Based on code from https://github.com/palantir/blueprint/issues/336
import * as React from "react";
import * as ReactDOM from "react-dom";
import {createRoot} from "react-dom/client";
import {ResizeEnable, Rnd} from "react-rnd";
import {Button, Classes, Dialog, DialogProps} from "@blueprintjs/core";
import {observer} from "mobx-react";
Expand All @@ -19,6 +19,7 @@ export class ResizableDialogComponentProps {
helpType?: HelpType;
onResizeStop?: (newWidth: number, newHeight: number) => void;
dialogId: string;
children?: React.ReactNode;
}

@observer
Expand All @@ -41,7 +42,10 @@ export class DraggableDialogComponent extends React.Component<ResizableDialogCom
const helpButton = <Button icon="help" minimal={true} onClick={this.onClickHelpButton} />;
const helpButtonDiv = document.createElement("div") as HTMLDivElement;
helpButtonDiv.setAttribute("class", "help-button");
ReactDOM.render(helpButton, helpButtonDiv);

const root = createRoot(helpButtonDiv);
root.render(helpButton);

const closeButton = this.dd.current.getElementsByClassName(Classes.DIALOG_CLOSE_BUTTON);
if (closeButton.length > 0) {
closeButton[0].before(helpButtonDiv);
Expand Down
11 changes: 5 additions & 6 deletions src/components/ImageView/RegionView/RegionViewComponent.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -758,7 +758,6 @@ class RegionComponents extends React.Component<{frame: FrameStore; regions: Regi
const regionSet = this.props.frame?.regionSet;
return regions.map(r => {
const commonProps = {
key: r.regionId,
region: r,
frame: this.props.frame,
layerWidth: this.props.width,
Expand All @@ -770,11 +769,11 @@ class RegionComponents extends React.Component<{frame: FrameStore; regions: Regi
};

if (r.regionType === CARTA.RegionType.POINT || r.regionType === CARTA.RegionType.ANNPOINT) {
return <PointRegionComponent {...commonProps} />;
return <PointRegionComponent {...commonProps} key={r.regionId} />;
} else if (r.regionType === CARTA.RegionType.ANNCOMPASS) {
return <CompassAnnotation {...commonProps} />;
return <CompassAnnotation {...commonProps} key={r.regionId} />;
} else if (r.regionType === CARTA.RegionType.ANNRULER) {
return <RulerAnnotation {...commonProps} />;
return <RulerAnnotation {...commonProps} key={r.regionId} />;
} else {
const allProps = {
...commonProps,
Expand All @@ -788,9 +787,9 @@ class RegionComponents extends React.Component<{frame: FrameStore; regions: Regi
r.regionType === CARTA.RegionType.ANNLINE ||
r.regionType === CARTA.RegionType.ANNVECTOR ||
r.regionType === CARTA.RegionType.ANNPOLYLINE ? (
<LineSegmentRegionComponent {...allProps} />
<LineSegmentRegionComponent {...allProps} key={r.regionId} />
) : (
<SimpleShapeRegionComponent {...allProps} />
<SimpleShapeRegionComponent {...allProps} key={r.regionId} />
);
}
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ export const PointShapeSelectComponent = observer((props: {handleChange: (pointS
const appStore = AppStore.Instance;
const preference = appStore.preferenceStore;

const iconWrapper = (path: React.SVGProps<SVGPathElement>, color: string, fill: boolean, strokeWidth = 2, viewboxDefault = 16) => {
const iconWrapper = (path: React.ReactNode, color: string, fill: boolean, strokeWidth = 2, viewboxDefault = 16) => {
let fillColor = color;
if (!fill) {
fillColor = "none";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ export interface PopoverSettingsComponentProps {
contentWidth: number;
onShowClicked?: () => void;
onHideClicked?: () => void;
children?: React.ReactNode;
}

export class PopoverSettingsComponent extends React.Component<PopoverSettingsComponentProps> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,14 @@ export class SpectralSettingsComponent extends React.Component<{
const filteredSpectralTypes = this.props.disableChannelOption ? spectralTypes.filter(type => type !== "Channel") : spectralTypes;
const spectralCoordinateOptions: OptionProps[] = filteredSpectralTypes.map((coord: string) => {
if (coord === SPECTRAL_TYPE_STRING.get(SpectralType.NATIVE)) {
return {value: coord, label: nativeSpectralCoordinate + " (Native WCS)"};
return {value: coord, label: nativeSpectralCoordinate + " (Native WCS)", key: coord};
}
return {value: coord, label: coord === nativeSpectralCoordinate ? coord + " (Native WCS)" : coord};
return {value: coord, label: coord === nativeSpectralCoordinate ? coord + " (Native WCS)" : coord, key: coord};
});
const spectralSystemOptions: OptionProps[] =
frame?.spectralSystemsSupported?.length > 0
? frame.spectralSystemsSupported.map(system => {
return {value: system, label: system};
return {value: system, label: system, key: system};
})
: [{value: frame?.spectralAxis?.specsys, label: frame?.spectralAxis?.specsys}];
const hasFrameCoordinateSetting = frame?.isSpectralChannel;
Expand Down
2 changes: 1 addition & 1 deletion src/components/Shared/Tables/FilterableTableComponent.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -353,7 +353,7 @@ export class FilterableTableComponent extends React.Component<FilterableTableCom
return (
<Table2
className={className}
ref={table.updateTableRef ? ref => table.updateTableRef(ref) : null}
ref={table.updateTableRef ?? null}
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From React documentation:

React will also call your ref callback whenever you pass a different ref callback. In the above example, (node) => { ... } is a different function on every render. When your component re-renders, the previous function will be called with null as the argument, and the next function will be called with the DOM node.

Therefore, FilterableTableComponent updates the ref whenever the table re-renders because there is an additional arrow function in the ref callback.

Moreover, the ref is set to null and then to the DOM node in the process. Before react 18, we use the ref after the two calls are complete. With react 18, the re-render timing somehow changes and we use the ref between the two calls —— the ref is (temporarily) null and the auto scroll is not triggered.

I remove the additional arrow function to avoid unnecessary reset of the ref, avoiding the entire issue.

numRows={table.numVisibleRows}
renderMode={RenderMode.BATCH}
enableRowReordering={false}
Expand Down
25 changes: 20 additions & 5 deletions src/components/Shared/toaster.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,26 @@
import {IconName, Position, Toaster, ToastProps} from "@blueprintjs/core";
import {createRoot} from "react-dom/client";
import {IconName, OverlayToaster, Position, ToastProps} from "@blueprintjs/core";

import {copyToClipboard} from "utilities";

export const AppToaster = Toaster.create({
className: "app-toaster",
position: Position.BOTTOM
});
const toaster = OverlayToaster.createAsync(
{
className: "app-toaster",
position: Position.BOTTOM
},
{
domRenderer: (toaster, containerElement) => createRoot(containerElement).render(toaster)
}
);

export const AppToaster = {
show: async (toast: ToastProps) => {
(await toaster).show(toast);
},
clear: async () => {
(await toaster).clear();
}
};

export function SuccessToast(icon: IconName, message: string, timeout?: number): ToastProps {
return {
Expand Down
11 changes: 6 additions & 5 deletions src/index.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 {createRoot} from "react-dom/client";
import {FocusStyleManager, OverlaysProvider} from "@blueprintjs/core";
import axios from "axios";

Expand All @@ -26,7 +26,7 @@ FocusStyleManager.onlyShowFocusOnTabs();

// GoldenLayout requires these in the global namespace
window["React"] = React; // tslint:disable-line
window["ReactDOM"] = ReactDOM; // tslint:disable-line
window["createRoot"] = createRoot; // tslint:disable-line

async function fetchConfig() {
const baseUrl = window.location.href.replace(window.location.search, "").replace("index.html", "");
Expand All @@ -39,11 +39,12 @@ async function fetchConfig() {
ApiService.SetRuntimeConfig({});
}

ReactDOM.render(
const container = document.getElementById("root") as HTMLElement;
const root = createRoot(container);
root.render(
<OverlaysProvider>
<App />
</OverlaysProvider>,
document.getElementById("root") as HTMLElement
</OverlaysProvider>
);
}

Expand Down
Loading