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

Could you support Node V16 for v1.0.0 #3993

Open
mapuboy opened this issue Aug 12, 2024 · 14 comments
Open

Could you support Node V16 for v1.0.0 #3993

mapuboy opened this issue Aug 12, 2024 · 14 comments

Comments

@mapuboy
Copy link

mapuboy commented Aug 12, 2024

Thank the great lib Cheerio first.
Our VM is not support Nodejs V18, but the Cheerio is "engines": {
"node": ">=18.17"
},
so our installation encounter this
npm install
10:14:02 npm ERR! code EBADENGINE
10:14:02 npm ERR! engine Unsupported engine
10:14:02 npm ERR! engine Not compatible with your version of node/npm: [email protected]
10:14:02 npm ERR! notsup Not compatible with your version of node/npm: [email protected]
10:14:02 npm ERR! notsup Required: {"node":">=18.17"}
10:14:02 npm ERR! notsup Actual: {"npm":"8.5.5","node":"v16.15.0"}
10:14:02
10:14:02 npm ERR! A complete log of this run can be found in:
10:14:02 npm ERR! /root/.npm/_logs/2024-08-12T02_11_11_118Z-debug-0.log

So, could you continue to support Node V16?
Thanks again.

@pnappa
Copy link

pnappa commented Aug 12, 2024

Node v16 is unsupported, and does not even receive security updates anymore, and as such is unsafe. It's highly recommended you upgrade (I'm sure there's a way for your VM to use a more modern version of NodeJS) to a supported version. https://endoflife.date/nodejs

Looking at the dependencies, it's not up to cheerio to downgrade their required engines versions, they've also got three dependencies which require >=v18:

┌──────────────────────────────┬──────────────┐
│ Conflicting dependencies (3) │ engines.node │
├──────────────────────────────┼──────────────┤
│ undici                       │ >=18.17      │
├──────────────────────────────┼──────────────┤
│ whatwg-encoding              │ >=18         │
├──────────────────────────────┼──────────────┤
│ whatwg-mimetype              │ >=18         │
└──────────────────────────────┴──────────────┘

@mapuboy
Copy link
Author

mapuboy commented Aug 12, 2024

Thanks. I will find a way for our VM to use a more modern version of NodeJs.

@txw2018
Copy link

txw2018 commented Aug 12, 2024

package.json
"pnpm": { "overrides": { "cheerio": "1.0.0-rc.12" } }

That will solve it

@davidkovacstw
Copy link

Looking at the dependencies, it's not up to cheerio to downgrade their required engines versions, they've also got three dependencies which require >=v18:

In semantic versioning, if you have a breaking change, you should bump your major version too.

Many users of this library expected that this principle will be respected, so it's fine to use ^1.0.0 as a version number. Now releasing breaking changes without changing the major version broke all these users. Please note that some of these users are transitive ones, so they might not even have full control of the used node version.

So I would respectfully ask you to:

  • release a 1.0.1 fix that reverts all changes from 1.0.0
  • release the changes under a 2.0.0 version number

Thanks for your work.

@Alevale
Copy link

Alevale commented Aug 12, 2024

I really don't wanna be that guy, but when you package has ~8338955 weekly downloads you can't expect your community to address issues in an hour, or be all up to date with Node, nor have everything perfect as @pnappa is suggesting here.

To be fair, I think that some of the changes that have been part of the release candidate packages feel to me like they could be under testing packages.

Please, in this case as @davidkovacstw suggested, revert the changes from 1.0.0 as a new version 1.0.1 and keep the new changes (all of the breaking ones) under 2.0.0

Like dropping node, or removing the lib folder.

@pnappa
Copy link

pnappa commented Aug 12, 2024

@davidkovacstw I agree with the comment about semantic versioning! When you break consumers of the package, a major version should be published so as not to break things.

However, isn't the case here that cheerio updated major version to version 1.0.0, and thus releasing a new major version, meaning they can introduce breaking changes? If you review the release notes, this change is listed: https://github.com/cheeriojs/cheerio/releases/tag/v1.0.0

Of which, bumping the minimum version of NodeJS as a result of bumping the undici dependency due to a low severity security vulnerability. GHSA-3g92-w8c5-73pq

Maintaining open source software is a thankless task, and we should be making their lives easier by not requesting they also make changes to support old & unsupported versions of software.

Thanks again to the cheerio devs for your software!

@Alevale
Copy link

Alevale commented Aug 13, 2024

@davidkovacstw I agree with the comment about semantic versioning! When you break consumers of the package, a major version should be published so as not to break things.

However, isn't the case here that cheerio updated major version to version 1.0.0, and thus releasing a new major version, meaning they can introduce breaking changes? If you review the release notes, this change is listed: https://github.com/cheeriojs/cheerio/releases/tag/v1.0.0

Of which, bumping the minimum version of NodeJS as a result of bumping the undici dependency due to a low severity security vulnerability. GHSA-3g92-w8c5-73pq

Yes and no.

I agree, that 1.0.0 wasn't final and there were RC packages till it became "stable enough" what I don't agree with is with the fact that it is acceptable to keep 1.0.0 as a similar or comparable version to what v1.0.0-rc.12 was previously.

1.0.0 deviated considerably from what was previously v1.0.0-rc.12 and it did so with the satement underneath (moving from Node 16 to Node 18)

The minimum NodeJS version is now 18.17 or higher #3959

It became a mandatory major by requiring a different major version of a previously used library/part of your system (Node in this case). By doing so you are breaking previous compatibility with any of your library prior clients, which in return should translate on you changing your package to become a major.

If you want to let them know that they need to updated you can deprecate or put a note on your current version, but you should not break semver and release a major on a minor.

There's no room for discussion, this is literally the definition of management of patches, minors and majors under semver.

Btw, I'm also grateful for all the amazing open source software and contributors. That is totally independent of this issue.

@pnappa
Copy link

pnappa commented Aug 13, 2024

Thanks for the insight @Alevale.

Treating semantic versioning as the letter of the law (of course, it's an entirely optional spec & the authors don't need to follow it), pre-release versions in semantic versioning have no bearing on compatibility across versions. As per https://semver.org/#spec-item-9, in bold:

  1. A pre-release version MAY be denoted by appending a hyphen and a series of dot separated identifiers immediately following the patch version. Identifiers MUST comprise only ASCII alphanumerics and hyphens [0-9A-Za-z-]. Identifiers MUST NOT be empty. Numeric identifiers MUST NOT include leading zeroes. Pre-release versions have a lower precedence than the associated normal version. A pre-release version indicates that the version is unstable and might not satisfy the intended compatibility requirements as denoted by its associated normal version. Examples: 1.0.0-alpha, 1.0.0-alpha.1, 1.0.0-0.3.7, 1.0.0-x.7.z.92, 1.0.0-x-y-z.--.

For the purpose of semantic versioning, the prior full release of 1.0.0 was 0.22.0 (released in 2016!). Unless I'm parsing the above paragraph incorrectly (or have missed something in the rest of the versioning spec, which is entirely possible!), 1.0.0 is a major version bump, and the release candidates have no bearing on compatibility guarantees between release candidates, or between the release candidate and the final version.

There was a mistake in the version tagging that did mark pre-release versions as latest, when they should have been next - presumably what's caused the implicit version update kerfuffle for consumers of this package.

@Alevale
Copy link

Alevale commented Aug 13, 2024

Thanks for sharing this @pnappa and you are right on that release candidates are indeed not stable by nature and anything can be dropped at any point.

My issue is that when you release once in a blue moon (or in the case of this package once every 2 years) I would argue that your RC packages should be treated as actual packages.

Keep in mind that the first release candidate was in 2017 and included a ton of breaking changes (which makes total sense cause it was different from 0.22 and it was the place to put them)

To be fair, there's also some instances where the RC could have been tagged as a major, but let's move on with the discussion.

My way of thinking is that if you can make things easier for the 8 million weekly downloads that should be the way to go, and if that means being in v11 that's fine.

I think it's not sensible to have to write down "cheerio": "=1.0.0-rc.12" in the package json of every single dependency of this, and resolutions or overrides for all the community...

If that's acceptable because someone was right I think we are failing as a community

@Alevale
Copy link

Alevale commented Aug 13, 2024

@matthewmueller @fb55 @jugglinmike In light of the discussion, and based on what is happening to everyone who trusted using your package for their builds, could you please do as suggested by this discussion and move the changes to 2.0.0 as suggested by @davidkovacstw ?

Semver is cool and shit, till you have a RC active for 7 years, 8 Million downloads, and people assumed you wouldn't change it much anymore.

@CodeMedic42
Copy link

I created a ticket(#4014) for something similar and @Alevale linked to this one there. I wanted to post what I have done to resolve this.

We use pnpm and I have added the following to our package.json

{
    "pnpm": {
        "overrides": {
            "cheerio": "1.0.0-rc.12"
        },
}

This obviously has a negative side effect for forcing ANYTHING using cheerio to use this version, so use carefully. Also it will always use this version until you remove it. Be sure to reevaluate this issue periodically to see if it has been resolved and you can remove this constraint. All very obvious statements but I like to be through.

@cossssmin
Copy link

We use cheerio in Juice (>1M downloads/week), which is now broken because of this unexpected breaking change release from an RC - we need to pin it to 1.0.0-rc.12 and release a patch ourselves now to fix this.

Please, in this case as @davidkovacstw suggested, revert the changes from 1.0.0 as a new version 1.0.1 and keep the new changes (all of the breaking ones) under 2.0.0

Yes, please.

@mapuboy
Copy link
Author

mapuboy commented Aug 16, 2024

package.json "pnpm": { "overrides": { "cheerio": "1.0.0-rc.12" } }

That will solve it

Yes. I have done that to resolve it for temp fixing.
But I upgraded our VMs too.

@kabbagepatch
Copy link

Thanks for the insight @Alevale.

Treating semantic versioning as the letter of the law (of course, it's an entirely optional spec & the authors don't need to follow it), pre-release versions in semantic versioning have no bearing on compatibility across versions. As per https://semver.org/#spec-item-9, in bold:

  1. A pre-release version MAY be denoted by appending a hyphen and a series of dot separated identifiers immediately following the patch version. Identifiers MUST comprise only ASCII alphanumerics and hyphens [0-9A-Za-z-]. Identifiers MUST NOT be empty. Numeric identifiers MUST NOT include leading zeroes. Pre-release versions have a lower precedence than the associated normal version. A pre-release version indicates that the version is unstable and might not satisfy the intended compatibility requirements as denoted by its associated normal version. Examples: 1.0.0-alpha, 1.0.0-alpha.1, 1.0.0-0.3.7, 1.0.0-x.7.z.92, 1.0.0-x-y-z.--.

For the purpose of semantic versioning, the prior full release of 1.0.0 was 0.22.0 (released in 2016!). Unless I'm parsing the above paragraph incorrectly (or have missed something in the rest of the versioning spec, which is entirely possible!), 1.0.0 is a major version bump, and the release candidates have no bearing on compatibility guarantees between release candidates, or between the release candidate and the final version.

There was a mistake in the version tagging that did mark pre-release versions as latest, when they should have been next - presumably what's caused the implicit version update kerfuffle for consumers of this package.

I think we all understand that technically the devs did nothing wrong. That said, we're all developers here and I like to think that our goal, in general, is to make people's lives easier. Going from a 1.0.0-rc that has millions of downloads a week to a 1.0.0 release that breaks a good chunk of those consumers' projects isn't making people's lives easier. On the other hand, changing the tag to 2.0.0 before releasing would've taken little to no effort. Just wanted to add my two cents

queicherius added a commit to gw2efficiency/scraping that referenced this issue Sep 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

8 participants