expressjs / discussions

Public discussions for the Express.js organization
62 stars 14 forks source link

Discussion: Using engines in the package.json #286

Open UlisesGascon opened 6 days ago

UlisesGascon commented 6 days ago

Currently some packages has drop support to Node.js prior to v18, seems like we didn't upgrade the engines field in the package.json discordantly.

So, I will like to discuss if this a policy that we should follow as an organization to all the packages (so in that case we can also document it).

:warning: So far seems like Express actually updated the package (ref) but not in others like finalhandler (ref)

Ref:

cc: @expressjs/express-tc @Phillip9587

UlisesGascon commented 6 days ago

IMO, we should align the engines to the CI versions we test for each release branch. Otherwise, we risk introducing breaking changes without realizing it.

If the package happens to work in older versions of Node.js that we haven't explicitly defined as supported, it should be considered an "unsupported feature." We should encourage end users to use at least the minimum supported version for that release branch. Otherwise, we'll end up "supporting" unintended scenarios based on environments we don't explicitly endorse, leading to issues like unexpected bug reports and slower development due to legacy version compatibility problems.

Additionally, npm is clear in its documentation: unless the user has set the engine-strict config flag, the engines field is advisory only and will simply produce warnings when your package is installed as a dependency. This reinforces the need to clearly define supported versions, ensuring that users are aware of the minimum requirements. (documentation)

ctcpip commented 6 days ago

agree, engines should be added where missing, and should match our CI matrix

Phillip9587 commented 6 days ago

I completely agree with the proposed solution to establish a consistent policy for updating the engines field across all packages. Ensuring that the Node.js version support is clearly defined and aligned accross the packages makes sense from a maintenance and user experience perspective.

I’m happy to contribute and help move this forward by submitting the necessary PRs to update the relevant packages. Let me know how best to coordinate on this!

ljharb commented 6 days ago

100% that CI should match whatever engines says.

(Changing engines in a way that reduces support is a breaking change, even if the software didn't work on the removed envs, ftr)

inigomarquinez commented 2 days ago

I completely agree with the proposal!

bjohansebas commented 1 day ago

I totally agree on keeping the CI in line with what the engines say

UlisesGascon commented 21 hours ago

Proposal for adr created https://github.com/expressjs/discussions/pull/289