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.

Sign up for GitHub

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

Upgrade to React 18 #1173

Merged
merged 1 commit into from
Feb 5, 2023
+98 −115
Merged

Upgrade to React 18 #1173

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
14 changes: 7 additions & 7 deletions packages/jaeger-ui/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,10 @@
"bluebird": "^3.5.0",
"customize-cra": "0.2.9",
"enzyme": "^3.8.0",
"enzyme-adapter-react-16": "^1.2.0",
"enzyme-to-json": "^3.3.0",
"@wojtekmaj/enzyme-adapter-react-17": "^0.8.0",
"enzyme-to-json": "^3.6.2",
"http-proxy-middleware": "^2.0.6",
"jest-junit": "^14.0.1",
"jest-junit": "^15.0.0",
"less": "3.13.1",
"less-loader": "4.1.0",
"less-vars-to-js": "^1.2.1",
Expand Down Expand Up @@ -85,10 +85,10 @@
"prop-types": "^15.5.10",
"query-string": "^6.3.0",
"raven-js": "^3.22.1",
"react": "^16.14.0",
"react-circular-progressbar": "^2.0.3",
"react-dimensions": "^1.3.0",
"react-dom": "^16.14.0",
"react": "^18.2.0",
Copy link
Member

Choose a reason for hiding this comment

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

Wow, that looks suspiciously easy, were there really no breaking changes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The breaking changes of lifecycle etc have been postponed to upcoming versions of React 18 and are still warnings afaik, and that is why its an easier upgrade. As far as I understand and can test the project still can use dependencies which are not yet fully React 18 compatible (like react-vis in this case).

Would love to hear how to move this PR forward and what we can do to test it, what edge cases of libs we think might break.

(I've also submitted changes to migrate from old lifecycle events in react-vis here uber/react-vis#1473)

"react-circular-progressbar": "^2.1.0",
"react-dimensions": "^1.3.1",
"react-dom": "^18.2.0",
"react-ga": "^3.3.1",
"react-helmet": "^6.1.0",
"react-icons": "2.2.7",
Expand Down
16 changes: 8 additions & 8 deletions packages/jaeger-ui/src/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,13 @@
// limitations under the License.

// site-prefix.js must be the first import of the main webpack entrypoint
// becaue it configures the webpack publicPath.
// because it configures the webpack publicPath.
/* eslint-disable import/first */
import './site-prefix';

import React from 'react';
import { BrowserRouter } from 'react-router-dom';
import ReactDOM from 'react-dom';
import { createRoot } from 'react-dom/client';
import { document } from 'global';

import JaegerUIApp from './components/App';
Expand All @@ -36,20 +36,20 @@ import 'u-basscss/css/typography.css';

const UI_ROOT_ID = 'jaeger-ui-root';

const root = createRoot(document.getElementById(UI_ROOT_ID));

if (trackingContext) {
trackingContext.context(() => {
ReactDOM.render(
root.render(
<BrowserRouter>
<JaegerUIApp />
</BrowserRouter>,
document.getElementById(UI_ROOT_ID)
</BrowserRouter>
);
});
} else {
ReactDOM.render(
root.render(
<BrowserRouter>
<JaegerUIApp />
</BrowserRouter>,
document.getElementById(UI_ROOT_ID)
</BrowserRouter>
);
}
2 changes: 1 addition & 1 deletion packages/jaeger-ui/src/setupTests.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ rafPolyfill();

/* eslint-disable import/no-extraneous-dependencies */
const Enzyme = require('enzyme');
const EnzymeAdapter = require('enzyme-adapter-react-16');
const EnzymeAdapter = require('@wojtekmaj/enzyme-adapter-react-17');
Copy link
Member

Choose a reason for hiding this comment

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

I can see this is another future PITA to be solved -- https://testing-library.com/docs/react-testing-library/migrate-from-enzyme/

const createSerializer = require('enzyme-to-json').createSerializer;

Enzyme.configure({ adapter: new EnzymeAdapter() });
Expand Down
Loading