raszi / node-tmp

Temporary file and directory creator for node.js
MIT License
736 stars 92 forks source link

breaking change in a patch #302

Closed ljharb closed 3 weeks ago

ljharb commented 1 month ago
$ npm show tmp@0.2 engines
tmp@0.2.0 { node: '>=8.17.0' }
tmp@0.2.1 { node: '>=8.17.0' }
tmp@0.2.2 { node: '>=14' }
tmp@0.2.3 { node: '>=14.14' }

In the npm ecosystem, versions are either v0.0.X, v0.X.Y, or vX.Y.Z, and the X is major, Y minor, Z patch.

That means, v0.2 is a "release line", and within 0.2, the engines requirements can't drop anything. So, v0.2.2 was a breaking change in a non-major version, and should have been a v0.3.

Is there any chance you could restore the engines compat in a v0.2.4? (if you want to publish a v0.3 that drops whatever you like that's perfectly fine ofc!)

silkentrance commented 1 month ago

0.x.x means that this is still not a production ready software - so breaking changes must be expected while it finally may develop towards a 1.0.0

ljharb commented 1 month ago

@silkentrance that's not how semver works in the node/npm ecosystem. you can see for yourself at https://semver.npmjs.org with a ^ range.

raszi commented 3 weeks ago

@ljharb That is exactly how Semver works, see the documentation:

https://semver.org/spec/v2.0.0.html#spec-item-4

The caret version dependency therefore should be discouraged for the non-final versions and explicit versions (=) should be used instead.

ljharb commented 3 weeks ago

@raszi that's how the spec works, but the spec isn't how npm works (and has literally always worked), and is thus irrelevant. check https://semver.npmjs.org, and https://github.com/semver/semver/pull/923

bvisness commented 3 weeks ago

Just because npm chooses to make some judgments about compatibility in the 0.x space does not mean package authors are obligated to avoid breakage on 0.x the same way they are on 1.x or greater. The npm docs link to the semver spec, which explicitly says that anything may change at any time and the public API should not be considered stable.

Until such a time as you actually get https://github.com/semver/semver/pull/923 merged, and get npm to officially adopt it, npm is under semver 2.0.0, which says that all versions (including 0.x) are of the form X.Y.Z, where X is major, Y is minor, and Z is patch. Your description of the status quo is incorrect, and conflates npm's resolution rules with the concept of major versions.

Also, it seems quite rude to impose this new burden on library authors who are quite literally following the letter of the law.

ljharb commented 3 weeks ago

@bvisness package authors aren't ever obligated to do anything. but npm and the package authors does get to decide the norms in its ecosystem, and that PR is the near universal consensus going back well over a decade, and it doesn't change just because some newer package authors aren't aware of it or agree with it.

raszi commented 3 weeks ago

The semver spec tells you how the versioning should be handled, and npm is an implementation of that with all its features.

You can use npm and the versioning constraints to keep in line with the semver spec and their idea behind the non-stable releases or you can use it differently, that is up to you.

You could use the = to pin it to the exact version as I suggested, you could use ~ and that could break since NPM implemented the non-stable versions differently.

You could also have used >=0.1.5 <2.0.0 and that could also potentially include breaking changes.

ljharb commented 3 weeks ago

@raszi at any rate you're welcome to decline my request and close the issue, but given that this is how ^ works (the default range indicator), this change objectively causes breakage (and caused breakage for me and my users), and i'm asking you to revert the breakage.

raszi commented 3 weeks ago

I am unsure that I completely understand your reasoning.

You have the option to pin to the version that satisfies your needs. On top of that, I believe we were historically quite generous with the engine compatibility. We are still compatible with the 14.14, even though the 14.x reached its end of life more than a year ago (2023-04-30).

What you technically say is that we should take care of older engine compatibility that reached their life more than two years ago. Is that a fair ask?

ljharb commented 3 weeks ago

I took that option already, in the linked commit shown above. However, users who don't have that commit will still be broken.

EOL status has no bearing on semver; it doesn't matter if it went EOL a decade ago or is still supported. Dropping an engine is always a breaking change.

I'm also not asking you to keep support of old engines - i'm just asking you to publish v0.2.1 directly as v0.2.4, and then publish v0.2.3 as v0.3.0 - which would keep your support burden identical and bring you back into compliance with the npm ecosystem's semver.

raszi commented 3 weeks ago

I am not questioning whether this change was breaking or not. The question we are discussing here is whether that should be allowed in a patch level change or not before the first stable version.

You are asking to support older engines since that is the breaking change that you want us to revert.

I don't think that your suggested steps could work, since versions are immutable. The only option would be to yank all the releases between the engine drop and now, and release a new version that still supports these engines, that they reached their end of life two years ago.

raszi commented 3 weeks ago

I realized that you also suggested to release a new version, but skipped the yanking part.

ljharb commented 3 weeks ago

You don't need to yank anything, because anyone who automatically updated to v0.2.3 would automatically update to v0.2.4.

raszi commented 3 weeks ago

If we consider these releases as broken then that would be the right thing to do.

ljharb commented 3 weeks ago

True, but npm's unpublish policy won't allow it, so npm deprecate would be the only option.

raszi commented 3 weeks ago

After considering the options, I decided to not do any rollbacks on the released versions for multiple reasons:

  1. NPM handles the pre-stable versions incorrectly, this should be not encouraged, and following a bad practice would lead to unwanted consequences, since then the pre-stable version would have no difference from a stable version and that makes zero sense.
  2. NPM should be handling the engine constraints with the version constraints together. Even if we accept that with the caret version selector, NPM picks up a newer version from the pre-stable semantic versions, it should also apply the engine availability and should be selecting a previous version that is available for the Node version of the system.
  3. Lastly, doing a "clean-up" on the released versions would create more mess than how big the problem is since the affected Node engines are long unsupported now.