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

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

Merged
merged 19 commits into from
Jun 7, 2024

Conversation

harsh183
Copy link
Member

@harsh183 harsh183 commented Jun 4, 2024

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 #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 #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 (#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 harsh183 requested a review from angrave June 4, 2024 02:34
@harsh183
Copy link
Member Author

harsh183 commented Jun 4, 2024

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
Copy link
Collaborator

angrave commented Jun 4, 2024

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
Copy link
Collaborator

angrave commented Jun 4, 2024

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

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

@angrave
Copy link
Collaborator

angrave commented Jun 4, 2024

Strange that the Docker.yaml is failing. As a quick sanity test I created an almost empty PR based on staging -
#814

  • which builds okay.

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.js and 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
Copy link
Collaborator

angrave commented Jun 4, 2024

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
Copy link
Collaborator

angrave commented Jun 4, 2024

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

Copy link
Collaborator

@angrave angrave left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

In the future, if we add Babel configs etc, be sure to update Dockerfile to copy them into the image

@angrave angrave merged commit 0f03139 into staging Jun 7, 2024
2 checks passed
@angrave angrave deleted the hd/eject-react-scripts-v4.0.3 branch June 7, 2024 19:42
@harsh183
Copy link
Member Author

harsh183 commented Jun 7, 2024

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
Copy link
Member Author

harsh183 commented Jun 8, 2024

@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

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.

2 participants