chore(deps): update to React 18 #526

maxcao13 · 2022-09-29T20:23:18Z

Fixes #482
Depends on patternfly/patternfly-react#7142

What's been updated:

"react": "^17.0.2" => "^18.2.0",
"react-dom": "^17.0.2" => "react-dom": "^18.2.0",

@testing-library/dom: 8.11.3 => 8.18.1
@testing-library/jest-dom: 5.16.2 => 5.16.5
@testing-library/react: 12.1.4 => 13.4.0
@testing-library/user-event: 13.5.0 => 14.4.3

What's been added:

"@types/react": "^18.0.21", // typescript react types
"@types/react-dom": "^18.0.6",

"react-test-renderer": "^18.2.0",

Noteworthy: the update of @testing-library/user-event has made all their api calls to return promises, await and async should be used for those calls in our tests now. https://github.com/testing-library/user-event/releases/tag/v14.0.0

andrewazores · 2022-09-29T20:42:29Z

yarn format:apply should fix the CI failure, or at least let it advance to the next step.

Or npm run format:apply if you don' have yarn on your PATH.

maxcao13 · 2022-09-29T21:20:00Z

Should es-lint formatting be applied too? I noticed it's not included in the format command.

… including adding explicit children

andrewazores · 2022-09-29T22:12:50Z

Should es-lint formatting be applied too? I noticed it's not included in the format command.

The last time I tried it, which was quite a while ago, the auto-formatting it applied actually broke things and needed manual cleanup by hand, and it was quite a significant amount of busywork. I just tried it again now and it looks like it applies some safe fixes fine, though there are many other linting warnings still present after.

I think you can update the format:apply script to also run eslint:apply since it seems safe. Don't add eslint:check to format:check yet because there will still be a bunch of lint warnings emitted right now, and adding it to that script would cause builds to fail until we clean up the lint issues.

src/test/Recordings/Filters/DateTimePicker.test.tsx

tthvo · 2022-09-29T23:47:35Z

src/test/Recordings/ActiveRecordingsTable.test.tsx

-      preloadState: preloadedState,
-      history: history,
+  it('removes a recording after receiving a notification', async () => {
+    await act(async () => {


Some reasons behind wrapping these calls in act?

There was a lot of console warnings with the tests before adding that, but other than that not actually sure of the underlying reason why that fixes it.

Pretty odd! This only happens with *RecordingsTable components :D

Seems like using iframe for ReportView causes this issue. It uses onLoad callback that updates React states outside React callstack, which throws a warning in tests. Surprisingly, this didn't get caught in old versions of React testing.

cryostat-web/src/app/Recordings/ReportFrame.tsx

Line 83 in 19f353f

<iframe title="Automated Analysis" srcDoc={report} {...rest} onLoad={onLoad} hidden={!(loaded && isExpanded)} />

src/test/Recordings/ActiveRecordingsTable.test.tsx

maxcao13 · 2022-09-30T00:02:08Z

There are some rendering problems when actually running the web-client with React 18, it just seems a lot more clunky and slower to use. Currently investigating the reason.

maxcao13 · 2022-09-30T18:12:46Z

There are some rendering problems when actually running the web-client with React 18, it just seems a lot more clunky and slower to use. Currently investigating the reason.

After a lot of digging and testing, I'm unable to pinpoint the root of the problems and there doesn't seem to be many problems from users online with React 18 in general with what is going on here - Thuan's PR does seems to speed up rendering but the main problem is that intermediate and default states seems to flicker in between rendering. The most obvious replication of this is if you click on the Recordings tab with an Active Recording and click to a different tab and keep clicking back and forth. There are hacks that can be used to solve these problems, but obviously this seems to be part of a bigger issue.

So I'm beginning to wonder if it is a Patternfly issue or not since they do not fully support React 18 yet, and won't do so until their next release in early 2023. patternfly/patternfly-react#7142 (comment). If so, this should be put off until then, but for now it's unclear. Updating to React 18 isn't a priority in anyway either.

github-actions · 2022-12-13T01:00:56Z

This PR/issue depends on:

Core - Add support for React 18 patternfly/patternfly-react#7142
By Dependent Issues (🤖). Happy coding!

andrewazores · 2022-12-13T16:05:09Z

This PR/issue depends on:

Not actually resolved, just autoclosed due to inactivity.

maxcao13 · 2023-07-31T07:08:42Z

Patternfly 5 has been released with full support of React 18 https://medium.com/patternfly/patternflys-major-release-everything-you-need-to-know-62f7c890ff57

Here's a full upgrade article: https://www.patternfly.org/get-started/upgrade/

maxcao13 · 2023-08-21T20:05:58Z

Probably be better to start from scratch here.

maxcao13 added dependencies Pull requests that update a dependency file build labels Sep 29, 2022

maxcao13 changed the title ~~build(deps): update to React 18~~ chore(deps): update to React 18 Sep 29, 2022

andrewazores added the chore Refactor, rename, cleanup, etc. label Sep 29, 2022

maxcao13 requested review from andrewazores and tthvo and removed request for andrewazores September 29, 2022 21:53

maxcao13 added 6 commits September 29, 2022 17:53

update to react 18.2

4c52118

update client rendering

ccd58da

added @types/react and @types/react-dom, and fixed compilation issues…

528fa85

… including adding explicit children

fixed tests, but still have warnings

eb723f7

fixed warnings by wrapping with act

02cacdc

yarn format:applied

9d675e4

maxcao13 force-pushed the react-18 branch from 175e97f to 9d675e4 Compare September 29, 2022 21:58

maxcao13 marked this pull request as ready for review September 29, 2022 22:12

updated format:apply with eslint:apply and applied it

f1d3d53

andrewazores reviewed Sep 29, 2022

View reviewed changes

src/test/Recordings/Filters/DateTimePicker.test.tsx Outdated Show resolved Hide resolved

maxcao13 added 3 commits September 29, 2022 18:37

use advanceTimers config for userEvent in test

14efd3b

prettier

5aaca5a

remove comment

a792b5a

tthvo reviewed Sep 29, 2022

View reviewed changes

src/test/Recordings/ActiveRecordingsTable.test.tsx Show resolved Hide resolved

andrewazores mentioned this pull request Sep 30, 2022

[Task] Update useSubscriptions hook to reduce re-rendering #528

Closed

andrewazores added the blocked label Sep 30, 2022

github-actions bot added the dependent label Sep 30, 2022

andrewazores mentioned this pull request Nov 15, 2022

build(deps): bump react and react-dom #658

Closed

andrewazores mentioned this pull request Dec 9, 2022

build(deps-dev): bump @testing-library/react from 12.1.5 to 13.4.0 #744

Closed

mergify bot added the safe-to-test label Dec 12, 2022

github-actions bot removed the dependent label Dec 13, 2022

github-actions bot added the dependent label Dec 14, 2022

maxcao13 closed this Aug 21, 2023

maxcao13 deleted the react-18 branch August 21, 2023 20:06

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore(deps): update to React 18 #526

chore(deps): update to React 18 #526

maxcao13 commented Sep 29, 2022 •

edited by andrewazores

Loading

andrewazores commented Sep 29, 2022 •

edited

Loading

maxcao13 commented Sep 29, 2022 •

edited

Loading

andrewazores commented Sep 29, 2022

tthvo Sep 29, 2022

maxcao13 Sep 30, 2022

tthvo Sep 30, 2022

tthvo Sep 30, 2022

maxcao13 commented Sep 30, 2022 •

edited

Loading

maxcao13 commented Sep 30, 2022 •

edited

Loading

github-actions bot commented Dec 13, 2022 •

edited

Loading

andrewazores commented Dec 13, 2022

maxcao13 commented Jul 31, 2023

maxcao13 commented Aug 21, 2023

chore(deps): update to React 18 #526

chore(deps): update to React 18 #526

Conversation

maxcao13 commented Sep 29, 2022 • edited by andrewazores Loading

andrewazores commented Sep 29, 2022 • edited Loading

maxcao13 commented Sep 29, 2022 • edited Loading

andrewazores commented Sep 29, 2022

tthvo Sep 29, 2022

Choose a reason for hiding this comment

maxcao13 Sep 30, 2022

Choose a reason for hiding this comment

tthvo Sep 30, 2022

Choose a reason for hiding this comment

tthvo Sep 30, 2022

Choose a reason for hiding this comment

maxcao13 commented Sep 30, 2022 • edited Loading

maxcao13 commented Sep 30, 2022 • edited Loading

github-actions bot commented Dec 13, 2022 • edited Loading

andrewazores commented Dec 13, 2022

maxcao13 commented Jul 31, 2023

maxcao13 commented Aug 21, 2023

maxcao13 commented Sep 29, 2022 •

edited by andrewazores

Loading

andrewazores commented Sep 29, 2022 •

edited

Loading

maxcao13 commented Sep 29, 2022 •

edited

Loading

maxcao13 commented Sep 30, 2022 •

edited

Loading

maxcao13 commented Sep 30, 2022 •

edited

Loading

github-actions bot commented Dec 13, 2022 •

edited

Loading