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

Node 14 #1050

Merged
merged 34 commits into from
Apr 30, 2021
Merged

Node 14 #1050

merged 34 commits into from
Apr 30, 2021

Conversation

Stephen-ONeil
Copy link
Contributor

@Stephen-ONeil Stephen-ONeil commented Apr 28, 2021

Move entire project to Node 14, both as the expected dev environment and as the intended target for Google Cloud Function deploys. Move node-targeted code to esModules where possible.

Have yet to see client builds hitting memory limits after switching to Node 14 🤞. Edit: still true locally, where it seems the problem had always been the baked in memory limits of the older V8 I was still on. Still very easy to burn through the CI container's 4GB, but I've re-re-fixed the prod build memory capping in this branch and I think I've got CI surviving the build jobs consistently.

…ct and deployment of the google cloud functions
… transpillation and babel from the project (note: Jest doesn't support ecma modules well yet, still requires babel as a peer dependency for tests to work)
…mation bugs (might not have been worth the conversion, but for now I'm leaning towards consistency)
…nd server babel set up to include babel-plugin-transform-import-meta
…e, not something I want to tangle with in this branch
@Stephen-ONeil
Copy link
Contributor Author

New CI images a much larger, fingers crossed the impact on CI job spin up isn't too great.

…rom local node_modules rather than requiring it to be installed globally
…kage.json. Didn't want to, since they're really only needed for the deploy scripts (which aren't run through package.json), but it's the most trustworthy way to run the local bode_modules babel in CI (since npx isn't installed)
…r step, so that the function will redeploy on transiplation config change
…ud sdk image, shouldn't be trying to run babel in there. Lift server transpile up in to the main build job, persist to the workspace with everything else
… plugin-transform-modules-commonjs is part of preset-env by default
…o declare that the modules are CommonJS rather than esModules
…ack in to bash scripts alongside the other deploy-specific scripts for those projects
…int for it (and catch some other misc unlinted lines in client/src)
@Stephen-ONeil
Copy link
Contributor Author

Stephen-ONeil commented Apr 30, 2021

... had hoped this would be the time we could fully standardize on esModules, but:

  • in /server and /email_backend:
    • even with Node 14... turns out GCF specifically do not support esModules*, so even if we can now run locally from src without babel we still need to transpile pre-deploy
    • Jest only semi-supports esModules, and doesn't do a good enough job of it to work out of the box. Fine though, since it transpiles using your local .babelrc by default anyway (and if we still need to transpile for GCF, it's preferable to run tests against the transpiled version anyway)
  • in /client:
    • while I got builds running with all /build_code moved to esModules, the resulting site had some confusing React errors (some components containing Elements of invalid type; object instead of a string/class/function), didn't dig too deep in to that because of the following bullet
    • Storybook doesn't support an esModule .storybook/main.js (issue) Extra painful, because our storybook config imports from build_code/webpack_common.js to share webpack rules... So for now Storybook also forces our webpack_common.js to remain CommonJS one way or another (tried some workarounds, e.g., but they failed to work-around)
  • buried in a notice at the very bottom of their GCF Node runtime docs, no explanation given 🤷
    image

…See PR 1050 for notes on what prevents this from being an option
…odules. See PR 1050 for notes on what prevents this from being an option"

This reverts commit 2c4b038.
@Stephen-ONeil
Copy link
Contributor Author

Well, minor victory, since the local server and email_backend can be run without transpilation, debugging them will be a nicer experience!

Copy link
Contributor

@ktw1016 ktw1016 left a comment

Choose a reason for hiding this comment

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

It's a shame that storybook doesn't support es moudle.. but I'm a fan that server code support fully es module

@Stephen-ONeil Stephen-ONeil merged commit d14a975 into master Apr 30, 2021
@Stephen-ONeil Stephen-ONeil deleted the node-14 branch April 30, 2021 20:29
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