mime-types icon indicating copy to clipboard operation
mime-types copied to clipboard

engines field is not correct for 3.x

Open alexander-akait opened this issue 9 months ago • 6 comments

engines is https://github.com/jshttp/mime-types/blob/master/package.json#L37

But in code we have:

 if (
    exports.types[extension] !== 'application/octet-stream' &&
    (score0 > score1 ||
      (score0 === score1 &&
        exports.types[extension]?.slice(0, 12) === 'application/'))
  ) {
    return type0
  }

https://github.com/jshttp/mime-types/blob/master/index.js#L201C2-L208C4

Optional chaining is not available in [email protected]

Ref: https://github.com/webpack/webpack/pull/19366

alexander-akait avatar Mar 27 '25 11:03 alexander-akait

This is bug as we forgot to update the engine field (also by policy the field change is consided a semver-major), but optional chaining was included as we dropped support for old Node.js versions (Drop support for node <18) since [email protected] https://github.com/jshttp/mime-types/blob/master/HISTORY.md#300--2024-08-31

UlisesGascon avatar Mar 27 '25 13:03 UlisesGascon

Since the breaking change of dropping support for older Node versions is clearly documented in the 3.0.0 notes, I'm assuming we just want to go ahead and update the engines field retroactively?

dpopp07 avatar Mar 27 '25 14:03 dpopp07

Nope, we can't. Even though it was documented, it was decided not to update it. See https://github.com/expressjs/discussions/pull/289

bjohansebas avatar Mar 27 '25 14:03 bjohansebas

@bjohansebas In such case dependabot and other updating bots will spam this update, because they look at the engine field and compare it with your project (and based on this they decide whether to send updates or not.), I think you'll see more of these types of issues here in a little while if you will not fix it. This is not a serious problem, more of an inconvenience.

alexander-akait avatar Mar 27 '25 14:03 alexander-akait

@bjohansebas I remember that decision, and I agree it should be considered a breaking change, but that discussion states:

What will be done? ... Update the engines field in package.json for each package to reflect the minimum Node.js version supported by the corresponding CI configuration.

The minimum version in the CI configuration for this project is v18. So the CI, the code, and the documentation are all aligned that 0.6 is no longer supported, but the engines field is not consistent.

The three paths forward here, as I see them, are to 1) change the code, CI, and documentation to match what engines says, 2) update engines retroactively without a major release, or 3) update engines and release v4 (which seems heavy-handed IMO). None of the options are ideal but I personally don't think option 2 lies too far outside the boundaries of the policy.

dpopp07 avatar Mar 27 '25 14:03 dpopp07

We've been through this before and discussed at length previously.

tl;dr:

  1. We will update engines but this will require a major rev.
  2. We will not release a new major just for the engines change.

As an aside, last time I checked, dependabot does not take the engines field into consideration at all.

Let's leave this issue open until resolved.

ctcpip avatar Mar 27 '25 15:03 ctcpip