Skip to content

Conversation

@UlisesGascon
Copy link
Member

@UlisesGascon UlisesGascon commented Oct 22, 2024

Copy link

@rxmarbles rxmarbles left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ctcpip
Copy link
Member

ctcpip commented Nov 5, 2024

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

@bjohansebas bjohansebas added meeting top priority Issues which the TC deem our current highest priorities for the project tc agenda labels Jan 13, 2025
@UlisesGascon
Copy link
Member Author

ping @ctcpip

Copy link
Member

@wesleytodd wesleytodd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@voxpelli
Copy link

voxpelli commented May 5, 2025

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
Copy link
Contributor

ljharb commented May 5, 2025

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
Copy link
Member

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.

@jonchurch
Copy link
Member

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.

@wesleytodd wesleytodd removed top priority Issues which the TC deem our current highest priorities for the project tc agenda labels Nov 10, 2025
@ctcpip
Copy link
Member

ctcpip commented Nov 10, 2025

relevant: jshttp/mime-types#136

@ljharb
Copy link
Contributor

ljharb commented Nov 11, 2025

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 :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Discussion: Using engines in the package.json

9 participants