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

Update client dependencies #4150

Merged
merged 80 commits into from
Mar 6, 2022
Merged

Update client dependencies #4150

merged 80 commits into from
Mar 6, 2022

Conversation

zhuhanming
Copy link
Member

@zhuhanming zhuhanming commented Oct 27, 2021

Summary

Update React to v17 and Webpack to v5, as well as other relevant dependencies.

Basically upgraded whatever I could that didn't require intensive refactoring/rewriting of our critical components. This is why the following dependencies were untouched:

  • @date-io/moment: Kept at v1 because @material-ui/pickers v3 will break with @date-io/moment v2. Will eventually upgrade once we shift to MUI v5.
  • redux-form and react-redux: Upgrading redux-form basically breaks all our assessment components. react-redux is also retained since redux-form will break when it's upgraded any further. This should be done as a separate effort.
  • react-intl and babel-plugin-react-intl: The former has a rather significant change in its interface. This should be done as a separate effort.
  • axios: Upgraded, but it got another update since, which was quite disruptive to its TypeScript usage. Luckily, we're not using TypeScript, but I held back on upgrading this for now. Should be low-effort to upgrade this separately later.
  • immutable: A lot of breaking changes. This should be done as a separate effort.
  • react-dropzone: Some breaking changes, since we're on v5 and the latest is v11, which is largely based on hooks. This should be done as a separate effort.
  • eslint: Jumped from v7 to v8, not a high priority. Can be upgraded when we feel like it.
  • react-router and react-router-dom: Has some breaking changes going from v4 to v5, and even more from v5 to v6. Will require some effort to migrate.
  • react-chartjs-2: Has breaking changes.

In addition to the above, the following dependencies have also been explicitly deprecated:

  • @babel/polyfill: Deprecated in favour of separate inclusion of a polyfill and regenerator-runtime.
  • babel-plugin-react-intl: Renamed to babel-plugin-formatjs (which is actually not really the same)
  • material-ui: Now known as @material-ui/core for v4 or @mui/material for v5 (will be upgraded in a subsequent PR)
  • react-intl: Not really a deprecation, but it is part of the formatjs library now.

Some deps to look into removing in the future:

  • redux-promise: Doesn't seem to be used in the app, but might be a peer dependency for some other dependency.
  • jquery and jquery-ui: Not used in React app, and Rails side seems to have its own gems for it. Need to check if it's needed.
  • @wojtekmaj/enzyme-adapter-react-17: To be removed once Enzyme supports React 17 officially.

Some other notes:

  • chartjs: Not directly used, but is a peer dependency for react-chartjs-2.

Might've missed out a few points here and there, since the initial part of this commit was done a while back.

Test Plan

See the CI. 🙏🏼

@zhuhanming
Copy link
Member Author

MUI migration will be in a separate PR.

@ekowidianto
Copy link
Member

@ekowidianto Can I also get your help to check the submission table related files? I think I had to rebase it multiple times since changes were being made frequently to that file previously, just want to make sure I didn't rebase things wrongly.

Hey sorry for the late reply. Checked the code and tried it locally and it seems to be working fine!

@ekowidianto ekowidianto mentioned this pull request Feb 14, 2022
4 tasks
@ekowidianto
Copy link
Member

ekowidianto commented Feb 24, 2022

EDIT: After restarting my comp (note in the future similar errors may happen and restarting should make things work), the following are not issues anymore

Still known issues:

  • Narrow editor cant be opened (Warning: Function components cannot be given refs. Attempts to access this ref will fail. Did you mean to use React.forwardRef()?)

  • Material summernote airmode toolbar is still not working properly

  • When creating a survey question
    image

  • Suggests that hot reload is disabled - encountered some problems with react lifecycle (eg when an unrelated change is made in a submission edit, i get the following errors: Uncaught TypeError: postPacks.map is not a function, Warning: Can't perform a React state update on an unmounted component. This is a no-op, but it indicates a memory leak in your application. To fix, cancel all subscriptions and asynchronous tasks in the componentWillUnmount method., element is null, etc)

  • Video player is not showing (Uncaught (in promise) ChunkLoadError: Loading chunk vendors~video failed.)

  • Video stats in show not working (eg http://localhost:5000/courses/2/videos/1)
    image

  • Scribing question in submission edit cant be loaded (stuck with the loading indicator) (Uncaught ReferenceError: fabric is not defined)

  • The following appears when a client sc file is updated and saved (also seems like hot loading doesnt work for me)

image

@zhuhanming
Copy link
Member Author

I've managed to get the UI back to what it looked like before, but now there's an issue with the behaviour when you press ENTER.

For some reason, it will append two paragraphs instead of one. It also disrupts adding of new list items for ordered and unordered lists.

@ekowidianto
Copy link
Member

Downgraded mui chip input as there are breaking changes. I will replace this with autocomplete in mui v4 upgrade.

@ekowidianto
Copy link
Member

ekowidianto commented Mar 4, 2022

I've managed to get the UI back to what it looked like before, but now there's an issue with the behaviour when you press ENTER.

For some reason, it will append two paragraphs instead of one. It also disrupts adding of new list items for ordered and unordered lists.

FYI this issue also persists for summernote in the rails page. Also, found another issue for rails summernote and the border is lightly visible:

image

@ekowidianto
Copy link
Member

ekowidianto commented Mar 4, 2022

Helping to take a look at the summernote issue here..
@zhuhanming Can take a look at this commit (4536835) ?

I figured the forked repo (https://github.com/noesya/summernote-rails) does not have double return issue and it fixes the airmode toolbar issue in react view..

For this issue (#4150 (comment)), I fixed it by doing some adjustments in layout.scss

@zhuhanming
Copy link
Member Author

Helping to take a look at the summernote issue here.. @zhuhanming Can take a look at this commit (4536835) ?

I figured the forked repo (https://github.com/noesya/summernote-rails) does not have double return issue and it fixes the airmode toolbar issue in react view..

For this issue (#4150 (comment)), I fixed it by doing some adjustments in layout.scss

Quite interesting, because my summernote-rails was actually forked from that very repo you linked.

@zhuhanming
Copy link
Member Author

Finally...

itsdone

@zhuhanming zhuhanming merged commit aefea45 into master Mar 6, 2022
@zhuhanming zhuhanming deleted the update-client-deps-2 branch March 6, 2022 14:43
@ekowidianto
Copy link
Member

Another note: we will need to update the Node version used in production to v14.18.2 after merging this PR.

@cysjonathan fyi

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants