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

BREAKING CHANGE(engines): engines support for npm 9.0.0 #519

Closed
5 tasks done
Tracked by #443
lukekarrys opened this issue Jun 13, 2022 · 9 comments
Closed
5 tasks done
Tracked by #443

BREAKING CHANGE(engines): engines support for npm 9.0.0 #519

lukekarrys opened this issue Jun 13, 2022 · 9 comments
Assignees
Labels
Milestone

Comments

@lukekarrys
Copy link
Contributor

lukekarrys commented Jun 13, 2022

Summary

We want to ensure that the CLI's engines field is as up-to-date as possible & reflects a realistic set of node versions that it will work on & that our team cares to "support".

Details

  • Use the engines spec: ^12.22.0 || ^14.17.0 || >=16.0.0
  • Do not error but warn on undocumented node versions

Exit Criteria


Notes & References

Proposed major version support for npm 9.

  • 12
    • npm 8: ^12.13.0
    • npm 9: support for node 12 will be dropped
  • 14
    • npm 8: ^14.15.0
    • npm 9: ^14.15.0
    • I don't see a reason to change the supported versions of node 14. I couldn't find any features we would be unable to use by changing node 14 versions.
  • 16:
    • npm 8: >=16
    • npm 9: ^16.13.0
    • In the past we've backstopped major version support at the version where it entered Active LTS support, which in this case is 16.13.0. This made sense for us but caused problems if other parts of the ecosystem settled on a different minor/patch version for LTS. For example, npm@8 used ^12.13.0 and node-gyp@9 used ^12.22.0 which made upgrading difficult. We should collaborate with other packages to see what version of node 16 we should support.
  • 18:
    • npm 8: technically supported via >=16
    • npm 9: >=18
    • This drops support for node 17 but allows us to support node 19 as it is developed.
  • Odd numbered major versions:
    • npm 8: ^17 and ^19 supported via >=16
    • npm 9: All non-EOL odd versions with the caveat that dropping support for EOL odd versions is NOT a breaking change
    • We want to support all non-EOL odd versions, but dropping support for them (eg when going from >=16 to >=18) is a breaking change in our current support contract. The proposed solution above would be documented in a SUPPORT.md file in all relevant repos.
@lukekarrys
Copy link
Contributor Author

Alternatively, we can set node 18 support as ^18. This would allow us to add future major versions (^20 slated for 05-2023) without it being a breaking change.

Roadblocks to this are code that @wraithgar added that makes npm refuse to install itself globally if engines doesn't match. We could special case odd node versions and document/log our support for them, which would basically be "it should work, but is not officially supported"

@wraithgar
Copy link
Member

In hindsight the support levels being set at "when it went lts" were kind of arbitrary, and absent any features in 14 we want to start using, I don't think it warrants pinning anything higher. Functionally, only the lowest semver major in the matrix really defines the bounds because nodejs doesn't typically introduce the same feature in multiple semver major versions. esm would have been a good thing for us to identify in v12 and pin it when that was introduced, but we didn't.

Going forward I think that unless there are features in the lowest semver major that we think we want to start using in the next year, leaving it as ^n is fine.

As far as odd numbered nodejs versions, I think that the current engines check warning can be tweaked to say something slightly different if it detects your nodejs version is an odd number. It can explain that npm will probably work but remind folks that odd numbered nodejs is not stable, etc.

@nlf
Copy link

nlf commented Jun 14, 2022

some features that we may want to use and the minimum node versions to support them:

private class methods (i.e. no more symbols): ^14.19.3
optional chaining (foo?.bar?.baz): ^14.5.0
nullish coalescing operator (false ?? 42 === false, null ?? 42 === 42): ^14.5.0

@wraithgar
Copy link
Member

I can see us using private class methods once available. that will clean up a LOT of hard to read code, which is very important to me.

@wraithgar
Copy link
Member

wraithgar commented Jun 14, 2022

After a bit of discussion we have identified a root problem here being that we are conflating support and compatibility. Engines denotes compatibility. This is distinct from a "support" contract. We need to come up with our "support" contract that explains things like our intention to align w/ the support roadmap of nodejs versions. Once we've done that our engines field can become something much simpler like >=14.19.3

@lukekarrys
Copy link
Contributor Author

some features that we may want to use and the minimum node versions to support them

we discussed this a little further and because it might be important to keep engines support wide for ecosystem compatability i wrote this and found 14.6.0 to be the min version that supports all three of those features

const privateMethod = () => {
  class C {
    #x() { return 42; }
    x() {
      return this.#x();
    }
  }
  return new C().x() === 42;
}

const optionalChaining = () => {
  const x = {}
  return x?.y?.z === undefined
}

const nullishCoalescing = () => {
  return (null ?? 42) === 42
}
              
console.log(
  privateMethod(),
  optionalChaining(),
  nullishCoalescing()
)
❯ node --version && node index.js
v14.5.0
/Users/lukekarrys/Documents/npm-issues/statusboard-519/index.js:3
    #x() { return 42; }
      ^

SyntaxError: Unexpected token '('
❯ node --version && node index.js
v14.6.0
true true true

@wesleytodd
Copy link

wesleytodd commented Jun 15, 2022

This might be a useful starting place: https://github.com/nodejs/package-maintenance/blob/main/docs/PACKAGE-SUPPORT.md

I think there are issues and genuine concerns which folks have brought up in the past, including @darcyclarke. But, if you are looking to try and separate out the "compatible" and "supported" this was our attempt to decouple them while also giving it structure.

EDIT: also here is what I think was the last conversation related to that: nodejs/package-maintenance#192

@lukekarrys
Copy link
Contributor Author

We use readable-stream in the cli (only via npmlog which will be dropped sometime in the future, but maybe not before npm9?) and the recent major version uses ^12.22.0 || ^14.17.0 || >=16.0.0 so I think that would be a point towards using ^14.17.0 or higher for the node 14 version.

@darcyclarke darcyclarke changed the title BREAKING CHANGE(engines): engines support for npm9 BREAKING CHANGE(engines): engines support for npm 9.0.0 Aug 23, 2022
@lukekarrys lukekarrys added this to the v9.0.0 milestone Sep 2, 2022
lukekarrys added a commit to npm/cli that referenced this issue Sep 6, 2022
BREAKING CHANGE: `npm` is now compatible with the following semver range
for node: `^14.17.0 || ^16.0.0 || >=18.0.0`

Ref: npm/statusboard#519
lukekarrys added a commit to npm/cli that referenced this issue Sep 6, 2022
BREAKING CHANGE: `npm` is now compatible with the following semver range
for node: `^14.17.0 || ^16.0.0 || >=18.0.0`

Ref: npm/statusboard#519
lukekarrys added a commit to npm/cli that referenced this issue Sep 7, 2022
This also replaces the previous check for known broken versions of node
with an exception handler for syntax errors in order to try and give a
nicer error message when attempting to run npm on older node versions.

BREAKING CHANGE: `npm` is now compatible with the following semver range
for node: `^14.17.0 || ^16.13.0 || >=18.0.0`

Ref: npm/statusboard#519
wraithgar added a commit to npm/cli that referenced this issue Sep 8, 2022
This also replaces the previous check for known broken versions of node
with an exception handler for syntax errors in order to try and give a
nicer error message when attempting to run npm on older node versions.

BREAKING CHANGE: `npm` is now compatible with the following semver range
for node: `^14.17.0 || ^16.13.0 || >=18.0.0`

Ref: npm/statusboard#519
wraithgar added a commit to npm/cli that referenced this issue Sep 8, 2022
This also replaces the previous check for known broken versions of node
with an exception handler for syntax errors in order to try and give a
nicer error message when attempting to run npm on older node versions.

BREAKING CHANGE: `npm` is now compatible with the following semver range
for node: `^14.17.0 || ^16.13.0 || >=18.0.0`

Ref: npm/statusboard#519
@wraithgar
Copy link
Member

@lukekarrys can this be closed?

feelepxyz added a commit to sigstore/sigstore-js that referenced this issue Oct 24, 2022
Update supported node versions to match npm/cli v9: `^14.17.0 || ^16.0.0 || >=18.0.0`

References
- [BREAKING CHANGE(engines): engines support for npm 9.0.0](npm/statusboard#519)
- [Related npm/cli change](npm/cli@457d388)

Signed-off-by: Philip Harrison <philip@mailharrison.com>
feelepxyz added a commit to sigstore/sigstore-js that referenced this issue Oct 24, 2022
Update supported node versions to match npm/cli v9: `^14.17.0 || ^16.0.0 || >=18.0.0`

References
- [BREAKING CHANGE(engines): engines support for npm 9.0.0](npm/statusboard#519)
- [Related npm/cli change](npm/cli@457d388)

Signed-off-by: Philip Harrison <philip@mailharrison.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants