Add policy to align engines fields with ci

[UlisesGascon]

• closes: Discussion: Using engines in the package.json #286
• related: Re-thinking the discussion process #285

[rxmarbles]

LGTM

[ctcpip]

we need a note in here about the possibility of continuing to run CI in older unsupported node versions so we know if/when a change clearly breaks back-compat. (we do this for express itself, for example)

I'll try to come up with good wording for this

[UlisesGascon]

ping @ctcpip

[wesleytodd]

While I have general concerns about the decision here, I would much prefer this decision over the current state. The only thing I think is missing is the call-out that all more restrictive engines changes can only land with a MAJOR. And that we agreed major versions should not be cut just to change node support.

This means that this policy will take a while to roll out. I do not consider this blocking to merge this ADR, but I think it is something we had agreed on in a few discussions so just wanted to call it out since I didn't see it mentioned in the text of the ADR.

[blakeembrey]

As mentioned in jshttp/content-disposition#79 (comment), this proposal doesn’t cover what happens when the engines is incorrect or needs to be added for the first time. I don’t think that belongs in this ADR specifically since it’s about versioning itself vs engines, but it would help to clarify how engines impacts versioning. I.e. if we are clarifying an existing engines support, is it a breaking change?

If we’re releasing majors for only engines changes, this doubles maintenance overhead if there happens to be any issues that need to be back ported to the version with and without engines.

I can think of two options here:

1. Consider fixing the engines field a non-major change (it’s a fix after all)
2. Document the process of adding it to the “next major”

IIRC we discussed that adjusting engines itself should not be a reason to do a major release, but I can’t recall where that conversation is now.

[ljharb]

If a thing worked before, and stops working, it's a breaking change. Adding engines can be a breaking change even if it's exactly matching the previously working range, because there's tools that check for it in a graph and can fail CI.

[blakeembrey]

Fair, and that’s a strict take. If you have code that only runs on node 20+ or already has dependencies that restrict engines to node 20+ it seems like it should be safe to add engines documenting this.

That’s why the other option I see is deleting engines when it’s wrong. Or are you saying that’s also a breaking change to also remove engines here?

[voxpelli]

I think the pragmatic thing is to see the "engines" as a best effort description at describing the intended compatibility of the code at the point of release.

If the description is out of sync with reality then either reality has to adapt to the description or the other way around.

If the change of the "engines" range can be seen as a bug fix – reality was clearly different to it – then I think it's correct to fix, else I think a new major or a widened reality is needed.

Bug fixes are always breaking for those reliant on the bugged behavior and it's always a balance of factors where it's primarily a fix or a breaking change.

[ljharb]

Removing engines entirely would be explicitly declaring that you support * (the documented default), so I'm not sure why that'd be desirable, but certainly that's not a breaking change.

[blakeembrey]

I'm not sure why that'd be desirable, but certainly that's not a breaking change.

It’s not, but it’s less desirable to try and release new major versions to fix the supported version range.

@voxpelli I agree, it’s the main point of contention. We need to document this somewhere.