classtranscribe / FrontEnd

The React + Redux Frontend for ClassTranscribe
https://classtranscribe.illinois.edu
Other
25 stars 27 forks source link

Override webpack crypto function by ejecting React Apps v4 - unblocking node version upgrades #812

Closed harsh183 closed 3 weeks ago

harsh183 commented 1 month ago

Successfully verified with Node versions 18, 20 and 22 (latest). I think we should be able to upgrade our node, react and any other major versions after this.

After a false start https://github.com/classtranscribe/FrontEnd/pull/810, I was able to use a minor version upgraded version of react scripts to eject. After ejecting, I was able to override the MD4 algorithm to SHA256 (stackoverflow).

The underlying issue in https://github.com/classtranscribe/FrontEnd/pull/789 was that openssl just permanently blocked MD4 (insecure since the 90s) while webpack 4 was using it. Upgrading to webpack 5 was a lot of work (https://github.com/classtranscribe/FrontEnd/pull/810) with dubious benefits, so a simpler solution was to patch the encryption algorithm used to something modern.

Edit: Don't forget to squash merge

harsh183 commented 1 month ago

Looks like yarn build works on both places, but yarn test breaks on CI but not locally due to path imports not working exactly the same. Hopefully a small-ish bug to resolve.

angrave commented 4 weeks ago

FYI (not relevant to the current problem .. just an FYI that might trip you up; be sure check the logs) "yarn test "on CI github actions will fail if there no tests. (this is currently true in our main branch and could be true in your PRs)

angrave commented 4 weeks ago

@harsh183 Maybe "/Users/harsh183/Experiments/FrontEnd/src" in package.json ....should just be 'src' ? :-)

"modulePaths": [
      "/Users/harsh183/Experiments/FrontEnd/src"
    ],
angrave commented 4 weeks ago

Strange that the Docker.yaml is failing. As a quick sanity test I created an almost empty PR based on staging - https://github.com/classtranscribe/FrontEnd/pull/814

~~Something that prevents yarn from contacting ct-dev.ncsa.illinois.edu to create public/config.js ? I expected to see- info: Config file downloaded and written for new backend server https://ct-dev.ncsa.illinois.edu> (But I can't reproduce this locally; i.e. I can remove config/config.jsand running "yarn" will happily recreate it.)~~ Update: the log line error: Error: ENOENT: no such file or directory, open './public/config.js' is not a real error, and can be ignored.

angrave commented 4 weeks ago

I was able to reproduce the github actions build error by trying to create the docker image locally, docker build -t whatever . Cause: The Dockerfile needed to be updated to copy the new directories (build/ and config/) into the image.

angrave commented 4 weeks ago

I'm okay merging this in - assuming there's no other changes you want to add at this time.

harsh183 commented 3 weeks ago

Oh, I had no idea it would need Docker changes, thanks for debugging! In theory, create react scripts is not supposed to change anything functionally but just expose the abstraction out. Glad we don't have the starter template things anymore now

harsh183 commented 3 weeks ago

@harsh183 Maybe "/Users/harsh183/Experiments/FrontEnd/src" in package.json ....should just be 'src' ? :-)

"modulePaths": [
      "/Users/harsh183/Experiments/FrontEnd/src"
    ],

I love create-react-app facebook devs not understanding the differences between filepaths