Make a bit more progress towards upgrading to React 18
This supersedes !450 (closed).
I spent the last couple of days trying, once again, to upgrade to React 18... but once again I can't get passed all the failing tests. I tried setting up Jest independently from Create React App in order to use the latest version; I tried installing all the latest version of the Testing Library packages; I tried refactoring various things in the app; ... I tried sooo many workarounds, but every time I change something, something else breaks for no clear reason.
There's definitely a combination of:
- Jest/Testing Library/React struggling to handle asynchronicity in a JSDOM environment — one example I encountered is components rendered with
React.lazy()
getting stuck on "Loading..." - tests leaking into one another — i.e. tests that pass when run on their own but not together (maybe something to do with all the singletons used for rendering the app, like
Start
andRestService
and/or with the Redux/zustand stores orlocal/sessionStorage
not being cleared properly)
In this MR, I bring some of the changes from !450 (closed) and fix various issues (notably with typing) that popped up at various times along my quest. I also upgrade two libraries mentioned in #103 that are now compatible with React 18.
We should have a chat about where to go next. We need to get past this one way or another so we can move forward in H5Web, but also for the long-term health of the Daiquiri UI project.
Merge request reports
Activity
requested review from @sfisher
assigned to @bocciare
mentioned in issue #103
removed review request for @sfisher
added 1 commit
- dd6fd643 - Make a bit more progress towards upgrading to React 18
- Resolved by Stuart Fisher
- Resolved by Stuart Fisher
- Resolved by Stuart Fisher
- Resolved by Stuart Fisher
92 90 } 93 91 } 94 92 95 let Wrap: ReactNode = Fragment; I've seen this pattern in a few places in the codebase. React 18 complained about this one, but generally-speaking, it is cleaner to either:
- have two
return
statements and just duplicate the JSX that needs to be conditionnaly (like I did in my refactor); or - use composition by extracting the wrapping logic into a dedicated component and pass the JSX hat needs to be conditionally wrapped as
children
.
- have two
18 18 Start.start(store, layout); 19 19 } 20 20 21 const view = render(<App />, { store }); 21 const view = await render(<App />, { store }); 22 22 23 await waitFor(() => 24 expect(session.selector('auth', store.getState())).toEqual(true) 25 ); 23 await waitFor(() => { 24 expect(screen.queryByText('Starting Up')).not.toBeInTheDocument(); requested review from @sfisher
mentioned in merge request !450 (closed)
mentioned in merge request !640 (closed)
added 64 commits
-
dd6fd643...e2009421 - 63 commits from branch
main
- 476e584a - Make a bit more progress towards upgrading to React 18
-
dd6fd643...e2009421 - 63 commits from branch