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

Bump node target #4257

Closed
wants to merge 5 commits into from
Closed

Bump node target #4257

wants to merge 5 commits into from

Conversation

jtoar
Copy link
Contributor

@jtoar jtoar commented Jan 24, 2022

Both Netlify and Vercel's default Node.js version is v14.x.x.12 Currently, v14.19.0 is the latest v14.x release.

To bump the Node.js version we target, this PR changes two parameters: TARGETS_NODE, which is passed to Babel's preset-env, and ESBuild's target parameter for its build API.

TARGETS_NODE mainly affects how the packages in the Redwood framework are built, which indirectly affects the Redwood projects that use them. I.e., TARGETS_NODE doesn't really affect how redwood projects are built (when users run yarn rw build). Instead, the target param for ESBuild matters.

But TARGETS_NODE does directly affect CLI Commands that use registerApiSideBabelHook:

  • yarn rw console
  • yarn rw exec
  • yarn rw data-migrate up

Footnotes

  1. https://docs.netlify.com/functions/build-with-javascript/#runtime-settings

  2. https://vercel.com/docs/runtimes#official-runtimes/node-js/node-js-version

@jtoar jtoar self-assigned this Jan 24, 2022
babel.config.js Outdated
@@ -4,7 +4,7 @@ const packageJSON = require(path.join(__dirname, 'package.json'))

// RedwoodJS targets Node.js 12.x because this is the default version
// for Netlify's functions.
const TARGETS_NODE = '12.16'
const TARGETS_NODE = '14.18.3'
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
const TARGETS_NODE = '14.18.3'
const TARGETS_NODE = '14.18'

Is it valid to omit the semver patch?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We used to omit it so I imagine it is, but there should be an official answer somewhere; I'll try to find it.

Copy link
Contributor Author

@jtoar jtoar Jan 25, 2022

Choose a reason for hiding this comment

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

I couldn't find any docs but it looks like this is what babel does with the value: https://github.com/babel/babel/blob/e45d86c33380e2e7e98b5713443b8e20658494a9/packages/babel-helper-compilation-targets/src/utils.ts#L19-L37.

Basically, passing what we currently have ('12.16') ends up being '12.16.0'. We should probably either 1) pin the version explicitly or 2) use legit semver, like '^14.18.3'.

Netlify and Vercel both say that their default is 14.x.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor Author

@jtoar jtoar Jan 25, 2022

Choose a reason for hiding this comment

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

@thedavidprice whatever we pass goes through semver.valid which only seems to like something like '14.18.3'—a semver without any of the range specifiers. If we leave off the patch it's filled in for us with a 0.

I'd need to dig through babel more to understand the full implications, but one thing to keep in mind is that this isn't the the runtime version. It seems like that number is up to netlify, vercel, and to some degree, the user.

babel.config.js Outdated Show resolved Hide resolved
@thedavidprice
Copy link
Contributor

@jtoar Great to see this one started. We should update here as well (correct?): https://github.com/redwoodjs/redwood/blob/94c13c78dd8a6eb77337708cbceba49fbf82fb71/packages/internal/src/build/babel/api.ts

My questions about this change are (no surprises):

  1. is this backward compatible?
  2. how and what do we need to communicate about this change in the release notes?

What if we added custom babel.config.js to my deploy test project and confirmed working?

@jtoar
Copy link
Contributor Author

jtoar commented Jan 25, 2022

We should update here as well (correct?)

Ah yeah great catch!

is this backward compatible?

I want to say that since we've got redwood projects configured to use most of the JS latest and greatest via babel, this isn't breaking, but I need to confirm.

how and what do we need to communicate about this change in the release notes?

We need to communicate that we did it, and also that most users should experience no difference. If someone was trying to use a specific node.js feature (not javascript, but node.js explicitly), they have access to it now. And I can look at the breaking changes from 12 to 14.

What if we added custom babel.config.js to my deploy test project and confirmed working?

That'd be a great way to test!

@jtoar jtoar marked this pull request as draft February 7, 2022 17:31
@thedavidprice
Copy link
Contributor

@jtoar in parallel to this, do we need to take a look at our Browserslist settings?

@thedavidprice
Copy link
Contributor

@jtoar let me know if there's anything I can test here (e.g. deployments) before next CT meeting

@jtoar
Copy link
Contributor Author

jtoar commented Mar 13, 2022

in parallel to this, do we need to take a look at our Browserslist settings?

I need to read-up on how browserlist queries work, but when I glanced at them, they looked a lot more up to date than our node target.

@jtoar
Copy link
Contributor Author

jtoar commented Mar 15, 2022

  • try deploying
  • review how building the framework changes
  • review how building redwood apps change
  • investigate if/how this affects other babel packages (e.g. runtime)

@jtoar jtoar added release:breaking This PR is a breaking change and removed release:feature This PR introduces a new feature v2 labels May 5, 2022
@netlify
Copy link

netlify bot commented Jul 13, 2022

Deploy Preview for redwoodjs-docs canceled.

Name Link
🔨 Latest commit 8b7841e
🔍 Latest deploy log https://app.netlify.com/sites/redwoodjs-docs/deploys/62cee3d319e48b0009e6a8bf

@jtoar
Copy link
Contributor Author

jtoar commented Jul 21, 2022

I'm going to combine this with #5405. The work will be done there.

@jtoar jtoar closed this Jul 21, 2022
@jtoar jtoar deleted the ds-bump-node-target branch January 22, 2023 20:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release:breaking This PR is a breaking change topic/core
Projects
Status: Archived
Development

Successfully merging this pull request may close these issues.

None yet

3 participants