discussions
discussions copied to clipboard
expressjs
Add policy to align engines fields with ci
- closes: https://github.com/expressjs/discussions/issues/286
- related: https://github.com/expressjs/discussions/issues/285
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
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.
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?
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.
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.
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.
Notes for me summarizing while ulises talks about this bc it has been so long and I have to do this everytime:
We have engines field in our packages. We have a challenge w/ engines, the problem is if we forget to update the engines field (this has happened, I jon have done this), do we do a new major to fix the engines field? That was an open question, which has ended up being discussed in this ADR as well.
Its turned into a discussion into whether or not adding/fixing engines is always a major.
Im taking an action item to work with Ulises to caught back up on this and get a clear next step.
Fixing engines is not a major if the implementation was already incompatible with the engines being excluded.
If that's the case, it's a patch! If that's not the case, then I'd suggest reconsidering changing engines in the first place :-)