-
-
Notifications
You must be signed in to change notification settings - Fork 13
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
Update react from 17 to 18 #2791
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #2791 +/- ##
==========================================
- Coverage 72.75% 72.70% -0.05%
==========================================
Files 246 247 +1
Lines 9349 9344 -5
Branches 1077 1077
==========================================
- Hits 6802 6794 -8
- Misses 2222 2225 +3
Partials 325 325
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 14 of 42 files at r1, 6 of 14 files at r2, 1 of 3 files at r3, all commit messages.
Reviewable status: 21 of 50 files reviewed, all discussions resolved (waiting on @imnasnainaec)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 22 of 42 files at r1, 5 of 14 files at r2, 2 of 3 files at r3.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @imnasnainaec)
src/components/UserSettings/ClickableAvatar.tsx
line 8 at r3 (raw file):
import AvatarUpload from "components/UserSettings/AvatarUpload"; const avatarStyle = { height: 60, width: 60 };
I realize this is embedded in the original but shouldn't default styles be defined outside the code - perhaps a css or configuration file?
This kind of change would probably require many changes so if you agree that this would be a good change, we should create an issue for it.
Code quote:
const avatarStyle = { height: 60, width: 60 };
@jmgrady Yes, that is something we should do at some point: consolidate styles into the global theme. |
Also: * In `src/index.tsx` replace `import { render } from "react-dom";` with `import { createRoot } from "react-dom/client"` (per https://react.dev/blog/2022/03/08/react-18-upgrade-guide#updates-to-client-rendering-apis) * Update `@material-table/core` from 6.2 to 6.3 (not compatible with react 17) * Remove dependency `@mui/styles` (not compatible with react 18); replace with either `import { styled } from "@mui/system";` or direct `sx=` * Remove extra space from between audio components * Update `@microsoft/signalr` from 6 to 8 * Add dev dependency:`@babel/plugin-proposal-private-property-in-object` to deal with legacy warning * Migrate `@testing-library/react-hooks` usage to the corresponding parts in `@testing-library/react` * Remove unnecessary/problematic `await act(...` instances from `@testing-library/react tests` * Use `as any` to handle `TextFieldWithFont` type issues * Replace deprecated `tobeCalled` with `toHaveBeenCalled` and `tobeCalledTimes` with `toHaveBeenCalledTimes` * Remove extra space from between audio components
Also:
src/index.tsx
replaceimport { render } from "react-dom";
withimport { createRoot } from "react-dom/client";
per https://react.dev/blog/2022/03/08/react-18-upgrade-guide#updates-to-client-rendering-apis@material-table/core
from 6.2 to 6.3 (not compatible with react 17)@mui/styles
(not compatible with react 18)import { styled } from "@mui/system";
or directsx=
@microsoft/signalr
from 6 to 8@babel/plugin-proposal-private-property-in-object
to deal with legacy warning@testing-library/react-hooks
usage to the corresponding parts in@testing-library/react
await act(...
instances from@testing-library/react
testsas any
to handleTextFieldWithFont
type issuestobeCalled
withtoHaveBeenCalled
andtobeCalledTimes
withtoHaveBeenCalledTimes
This change is