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

Document minimum node v12 & v14 versions in README. #1469

Closed

Conversation

delhanty
Copy link

@delhanty delhanty commented May 17, 2021

Further to my earlier comment:

Edit: looks like the minimum versions are v12.20 and v14.13.1. I will make a pull request to document this in the README.

#1000 (comment)

That references:

Node.js 14.13.0 added support for named CJS exports, see #35249 and this change has also been backported to Node.js v12.20.0.

nodejs/node#32137 (comment)

and

Yep, it was fixed after downloading 14.13.1, thank you

nodejs/node#35249 (comment)

Before submitting the PR, please make sure you do the following

  • It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
  • This message body should clearly illustrate what problems it solves.
  • Ideally, include a test that fails without this PR but passes with it.

Tests

  • Run the tests with pnpm test and lint the project with pnpm lint

Changesets

  • If your PR makes a change that should be noted in one or more packages' changelogs, generate a changeset by running pnpx changeset and following the prompts

sveltejs#1000).

See my comment

sveltejs#1000 (comment)

references

>Node.js 14.13.0 added support for named CJS exports, see #35249 and this change has also been backported to Node.js v12.20.0.

nodejs/node#32137 (comment)

and

>Yep, it was fixed after downloading 14.13.1, thank you

nodejs/node#35249 (comment)
@benmccann
Copy link
Member

Rather than documenting this, we've enforced it here:

"node": ">= 12.17.0"

It looks like that line should be changed to >= 12.17.0 || >= 14.13.1

@delhanty
Copy link
Author

delhanty commented May 17, 2021

Rather than documenting this, we've enforced it here:

"node": ">= 12.17.0"

It looks like that line should be changed to >= 12.17.0 || >= 14.13.1

Ah great!

A friend (@a-ogilvie below) suggested pretty much that to me just now

"engines": {
  "node": "^12.20 || ^14.13.1 || ^16"
}

but I'm much happier with you messing with the package.json than me!

I note in Discord in the #sveltekit channel there are people trying to build with really ancient versions of node, so hopefully this should save on support time.

@delhanty delhanty closed this May 17, 2021
@a-ogilvie
Copy link

It looks like that line should be changed to >= 12.17.0 || >= 14.13.1

@benmccann Will this pattern also match against 14.0 since it's "greater than" 12.17?

@delhanty delhanty deleted the doc-readme-minimum-node-version branch May 17, 2021 03:04
@delhanty delhanty restored the doc-readme-minimum-node-version branch May 17, 2021 03:06
@delhanty
Copy link
Author

delhanty commented May 17, 2021

It looks like that line should be changed to >= 12.17.0 || >= 14.13.1

@benmccann for v12, the minimum version of node required is v12.20.0.

I guess I close this pull once engines gets updated in master ...

@benmccann
Copy link
Member

Closing in favor of #1470

@benmccann benmccann closed this May 17, 2021
@delhanty delhanty deleted the doc-readme-minimum-node-version branch May 17, 2021 03:40
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.

3 participants