Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: move mime-db to peerDependencies (plus musings on how semver applies) #114

Closed
wants to merge 0 commits into from

Conversation

broofa
Copy link
Member

@broofa broofa commented Dec 10, 2023

Currently, it's difficult for downstream libs to pin mime-types to a specific version of mime-db. To do so, they have to (indirectly) do that by pinning to a specific version of mime-types. But that's tricky, if for no other reason than the two projects use different versioning schemes. E.g. mime-db@latest is v1.52.0, while mime-types@latest is v2.1.35.

Switching mime-db to a peer dependency has the following benefits:

  • Gives users direct control over mime-db version, and does so in an easy, intuitive manner (see below)
  • Allows users to update (or regress) mime-types w/out having to take the correspond change to mime-db version
  • Removes the need to release a new version of mime-types with every mime-db release.

To Do:

  • Edit README to add instructions for how to "update" mime-types by running npm install mime-db@latest

Expected behavior (enabled by this PR)

For example, if we wanted to pin mime-db to a version prior to the introduction of the geojson type (for whatever reason), the logical way of doing this would be npm install mime-db@<desired version>...

# With `mime-types` already installed ...

$ npm install mime-db@1.0.0  # Going way back in time, just for giggles

$ npm ls -a
foo@ /private/tmp/foo
+-- mime-db@1.0.0
`-- mime-types@2.1.35
  `-- mime-db@1.0.0 deduped  # <-- Expected: `mime-types` uses "our" version 馃帀

$ node -e "console.log(require('mime-types').lookup('geojson'))"
false  # Expected, as "geojson" wasn't introduced until mime-db@1.26.0 馃帀馃帀馃帀

Actual behavior

# With `mime-types` already installed ...

$ npm install mime-db@1.0.0

$ npm ls -a
foo@ /private/tmp/foo
+-- mime-db@1.0.0
`-- mime-types@2.1.35
  `-- mime-db@1.52.0  # <-- Not expected: `mime-types` using its own version 馃槩

$ node -e "console.log(require('mime-types').lookup('geojson'))"
application/geo+json  # Nope, not what we want. 馃槩馃槩馃槩

Why?

Making this change decouples mime-types from mime-db. Users can install mime-types and then, as needed, npm update mime-db@<version> to update their type mappings.

But seriously ... why??? 馃槙

This PR is a result of my attempt to respond to this comment thread. I didn't want to spam that issue with what seemed likely to turn into a debate on the merits of mime-db and mime-types versioning strategies, so I'm just going to include my response here for posterity, as it's the basis of why I created this PR. I'll invite @Krinkle to respond here.

Briefly, @Krinkle asks that changes to @mime-db be surfaced in mime-types as major-version bumps since they risk breaking downstream libraries. I disagree with that. Not only does this fly in the face of mime-db and mime-types release history (historically such changes are minor-version bumps), it renders semver a bit meaningless since even the most trivial of changes (such as adding extensions to the text/javascript type) risk breaking downstream code.

There are two issues in play here:

How should changes to mime-db be reflected in its version?

Applying semver to data structures is tricky. Semver is specifically about "APIs", and data structures don't really have that. So "it's debatable". With that said, there's really only a couple ways of looking at this:

  • Consider the data itself as the "API"
  • Consider the "shape" of the data (i.e. the data types) to be the API.

I very much prefer the latter perspective, as that's what downstream libs like mime-types and mime are most concerned with . As the maintainer of mime, I don't really care all that much if a new MIME type is added or modified. I need to roll a new version that incorporates that change, sure, but it doesn't affect my build script, or mime's API. Thus, such changes are trivial where mime is concerned and I'd like to see that conveyed in mime-db's versioning strategy. So the current practice of minor-version bumps has been fine for me.

What does matter - what will absolutely break the world of the mime module - is the shape of mime-db data - i.e. the structures documented in @types/mime-db. And I think it appropriate to expect mime-db's versioning strategy to reflect that.

The problem with treating "trivial" changes to the data as breaking changes is that there's nowhere to go if/when the structure of the data changes. Semver doesn't have a "No, seriously, we really, really mean breaking change this time!" field.

With that in mind, when I think about how semver should apply to data - and mime-db data, specifically - from a first-principles approach, I expect something along these lines:

  • Major version: Bumps when fields in the existing data structure are modified or removed .
    • E.g. Structure: renaming or removing values from the MimeSource type
    • E.g. Structure: removing the compressible field, or changing it's type.
  • Minor version: Bumps when fields are added to the data structure, or existing data is modified or removed.
    • E.g. Structure: adding a a value to the MimeSource type
    • E.g. Structure: adding a new field to the MimeEntry type.
    • E.g. Data: changing extensions for the application/javascript entry
  • Patch version: Bumps when new data is added.
    • E.g. Data: Adding extensions to the text/javascript entry

Basically changes to structure = "at least minor", while changes to data = "at most minor"

How should mime-types incorporate mime-db's semver changes?

Answer: It shouldn't. Doing so is a losing proposition because the two projects use semver in different ways, and trying to conflate the two will always be a source of confusion. mime-db uses semver to describe the db.json data, while mime-types is (or at least, should) uses it to describe the API.

Hence, this PR. Allowing users to directly specify both the mime-db and mime-types versions gives them explicit control over both the API and the data. And it allows project maintainers to be clear and unambiguous about what users can expect when the version(s) change.

cc: @ljharb, as I've seen him chime in on a few discussions like this on the SemVer repo and he may find this an interesting use case. 'Not sure if he'll agree or disagree with me on my rational, above. :-)

Copy link

@ljharb ljharb left a comment

Choose a reason for hiding this comment

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

This seems fine, modulo using ^ (deps should never be pinned anywhere but in a lockfile), and noting that adding a peer dep is a breaking change.

package.json Outdated
@@ -13,7 +13,7 @@
"types"
],
"repository": "jshttp/mime-types",
"dependencies": {
"peerDependencies": {
"mime-db": "1.52.0"
Copy link

Choose a reason for hiding this comment

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

is the intention here really to pin? if so, i'd use an explicit =, but using ^ is the better approach here so that security updates and bugfixes can be used without use of overrides.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ugh, good catch. I'd actually changed this to "1.x" ... 'just forgot to push it. 馃槢

Copy link

Choose a reason for hiding this comment

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

I would strongly suggest avoiding that syntax, and instead using ^1.52.0 or ^1.0.0 if that's really what you want.

Copy link
Member Author

Choose a reason for hiding this comment

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

Interesting. How come? Is it because "1.x" doesn't let you specify a minimum version other than the implied "1.0.0"?

@ljharb
Copy link

ljharb commented Dec 10, 2023

I'm also not sure why "different versioning schemes" makes anything tricky.

@broofa
Copy link
Member Author

broofa commented Dec 10, 2023

I'm also not sure why "different versioning schemes" makes anything tricky.

'Just referring to the fact there's no direct relationship between mime-db version and mime-types version. E.g. if you want to use types defined in mime-db@1.26.0 you don't install mime-types@1.26.0, you install a different version.
To figure out which version is right you have to sift through HISTORY.md or git log.

@ljharb
Copy link

ljharb commented Dec 10, 2023

Ah, then yes, peer deps should always have been the mechanism used here :-)

@dougwilson
Copy link
Contributor

I don't mind using peer deps in principle, though I am not super familiar with them since I don't really encounter them normally. I wonder how does it work with a sub dep, like if you use foo lib that is requires mime-types, does foo add the mime-db dep or your top lib?

@ljharb
Copy link

ljharb commented Dec 11, 2023

Generally, the top lib - peer dep requirements are "hoisted".

Declaring mime-db as a peer dep is effectively stating that there should only ever be one copy/version of it in your entire application.

@dougwilson
Copy link
Contributor

Gorcha. Just thinking though what it would mean when mime-db 2 comes out and users have various versions of mime-types in their module tree. I would suppose that mime-types would probably need to (if possible, even) supoort both major versions of mime-db such that the user can have whatever the lowesr common major would be for their installed tree.

@broofa
Copy link
Member Author

broofa commented Dec 11, 2023

It's worth noting that npm install doesn't install peerDependencies. So the "normal" way of installing mime-types would become npm install mime-types mime-db.

when mime-db 2 comes out

Then mime-types would need a new release that declares "mime-db": ^2.0.0 in peerDependencies

Basically peerDependencies pushes responsibility for declaring a mime-db dependency "up" the dependency tree with the proviso that, wherever it's declared, it has to satisfy the specified version range. And then npm install will "dedupe" the actual installed versions to whatever extent is possible.

@dougwilson
Copy link
Contributor

Right, I get it would bump the peer dep req, but what happens if there is still a sub sub dep in your tree with mime-types 1? Is the only thing people can do is bug one module after the other to bump each other to get to all mime-types 2 in the tree before they can upgrade any of them? That's what I am asking, I guess.

@ljharb
Copy link

ljharb commented Dec 11, 2023

@broofa npm v7+ (and < 3) does indeed attempt to install peer deps automatically, and the install will fail if the peer dep can't be satisfied.

@dougwilson yes, for anything that's a peer dep, you can't upgrade anything from a v1 to a v2 unless everything already supports it.

@broofa
Copy link
Member Author

broofa commented Dec 11, 2023

@broofa npm v7+ (and < 3) does indeed attempt to install peer deps automatically, and the install will fail if the peer dep can't be satisfied.

Ahh, I was wondering about that. When I was playing with this yesterday I could have sworn npm was installing the dependencies for me. But this morning it wasn't... I guess because I'd been doing some testing with node@12 / @14 (npm@6) and forgot to switch back. 馃う

@dougwilson
Copy link
Contributor

Gorcha. Yea, that is the only thing that gives me pause regarding rhe peer dep thing, that this module will end up in a pickle when like express changes to mime-types 2 but supertest is still mime-types 1. Not even sure how to go about resolving that bc express itself has a dev dep on supertest, so i would presume we'd have to get supertest to first upgrade the mime-types to 2 before express could even do so so that express could have a compatible mime-db loaded at the top level for both 馃

@broofa
Copy link
Member Author

broofa commented Dec 11, 2023

@dougwilson Can we reopen and reset this to come from the broofa:peers branch?

'Tried to move this off my master branch to peers and github decided to close it. :-/

[Edit: If that's too much trouble let me know and I'll just open another PR, but I'd prefer to keep the conversation here.]

@dougwilson
Copy link
Contributor

dougwilson commented Dec 11, 2023

Screenshot_20231211_164211_Chrome

Looks like i cannot reopen.

Edit: also cannot change the source branch, but I think only the author can do that anyway, or cannot when it is closed, not sure.

@ljharb
Copy link

ljharb commented Dec 11, 2023

@broofa for future reference you can't ever move a PR off of the branch it started on.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants