classtranscribe / FrontEnd

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

UpdateToNode18 #789

Closed angrave closed 3 weeks ago

harsh183 commented 1 month ago

Just hit update to see where the CI is failing. A new node version is desperately needed but we can slowly upgrade to the latest version (v22)

harsh183 commented 1 month ago

Across a lot of different node versions I tried (18, 19, 20), the solution to the current build failing is to update "react-scripts": "5.0.1". However, this upgrades to a version of webpack that doesn't have many of the core library polyfills: https://github.com/facebook/create-react-app/issues/11756.

Module not found: Error: Can't resolve 'https' in '/Users/harsh183/Experiments/FrontEnd/src/utils/cthttp'
BREAKING CHANGE: webpack < 5 used to include polyfills for node.js core modules by default.
This is no longer the case. Verify if you need this module and configure a polyfill for it.

If you want to include a polyfill, you need to:
    - add a fallback 'resolve.fallback: { "https": require.resolve("https-browserify") }'
    - install 'https-browserify'
If you don't want to include a polyfill, you can use an empty module like this:
    resolve.fallback: { "https": false }

To apply the polyfills ourselves we can either 'eject' the create react apps config (one way irreversible) to access the config or override the existing config via craco/react-app-rewired.

harsh183 commented 1 month ago

@angrave After doing some more reading, I think the best option is to eject the create react app config. The app has been stably running for the last few years so we're past the initial prototyping phase where we wanted to hide away the config details.

angrave commented 1 month ago

@harsh183 ((The last merged PR was an improved build action by Rob Kooper - so maybe the Docker build error is due to that instead??)

I googled the error below "Error: error:0308010C:digital envelope routines::unsupported" https://stackoverflow.com/questions/69692842/error-message-error0308010cdigital-envelope-routinesunsupported

extract below from build, https://github.com/classtranscribe/FrontEnd/actions/runs/9244786161/job/25430487004?pr=789

[frontend 8/8] RUN yarn build
#7 0.298 yarn run v1.22.19
#7 0.328 $ react-scripts build
#7 2.243 Creating an optimized production build...
#7 2.581 Error: error:0308010C:digital envelope routines::unsupported
#7 2.581     at new Hash (node:internal/crypto/hash:69:19)
#7 2.581     at Object.createHash (node:crypto:133:10)
#7 2.581     at module.exports (/frontend/node_modules/webpack/lib/util/createHash.js:135:53)
#7 2.581     at NormalModule._initBuildHash (/frontend/node_modules/webpack/lib/NormalModule.js:417:16)
#7 2.581     at handleParseError (/frontend/node_modules/webpack/lib/NormalModule.js:471:10)
#7 2.581     at /frontend/node_modules/webpack/lib/NormalModule.js:503:5
#7 2.581     at /frontend/node_modules/webpack/lib/NormalModule.js:358:12
#7 2.581     at /frontend/node_modules/loader-runner/lib/LoaderRunner.js:373:3
#7 2.581     at iterateNormalLoaders (/frontend/node_modules/loader-runner/lib/LoaderRunner.js:214:10)
#7 2.581     at iterateNormalLoaders (/frontend/node_modules/loader-runner/lib/LoaderRunner.js:221:10)
#7 2.581     at /frontend/node_modules/loader-runner/lib/LoaderRunner.js:236:3
#7 2.581     at runSyncOrAsync (/frontend/node_modules/loader-runner/lib/LoaderRunner.js:130:11)
#7 2.581     at iterateNormalLoaders (/frontend/node_modules/loader-runner/lib/LoaderRunner.js:232:2)
#7 2.581     at Array.<anonymous> (/frontend/node_modules/loader-runner/lib/LoaderRunner.js:205:4)
#7 2.581     at Storage.finished (/frontend/node_modules/enhanced-resolve/lib/CachedInputFileSystem.js:55:16)
#7 2.581     at /frontend/node_modules/enhanced-resolve/lib/CachedInputFileSystem.js:91:9
#7 2.627 /frontend/node_modules/react-scripts/scripts/build.js:19
#7 2.627   throw err;
harsh183 commented 1 month ago

@angrave yeah I was also getting this error locally. Basically this is caused by an old version of react scripts (0.4) currently and moving up to 0.5 will fix it, but that upgrade is another can of worms with react scripts hiding away the launch config.

We can also just allow unsafe openssl, but that feels like a bad short term solution over just fixing the configuration long term (react scripts eject)

angrave commented 1 month ago

Okay, I'll ignore this PR for now, but will leave it open so the comments are handy. (I think we probably should have put these comments & discussion in a github issue instead of a PR, ... but whatever). FYI One idea I had was starting from a blank slate (e.g. latest react (5?) project) and slowly re-adding required projects., starting with latest versions .I didn't do this because i) we don't have a good testing framework. ii) dva is obviously a blocking component. Looking at this today, dva seems to be used a small shim around redux, (to make it easy to have scope/namespaces) . So maybe updating piecemeal to use modern redux is one way to tear out dva... https://redux.js.org/usage/migrating-to-modern-redux

harsh183 commented 1 month ago

@angrave starting over will definitely take much longer, especially when it's already being used by lots of users currently. I think we can incrementally remove dvaJs as needed. Let's continue this discussion here: https://github.com/classtranscribe/FrontEnd/issues/768

angrave commented 1 month ago

Please merge/rebase latest changes in staging and re push.

harsh183 commented 3 weeks ago

@angrave the build is passing 🥳 , I can update the documentation once we merge to now have node 18.