-
Notifications
You must be signed in to change notification settings - Fork 129
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’ll 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
Conversation
There was a problem hiding this 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" |
There was a problem hiding this comment.
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
.
There was a problem hiding this comment.
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. 😛
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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"?
I'm also not sure why "different versioning schemes" makes anything tricky. |
'Just referring to the fact there's no direct relationship between |
Ah, then yes, peer deps should always have been the mechanism used here :-) |
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 |
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. |
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. |
It's worth noting that
Then Basically |
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. |
@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. |
Ahh, I was wondering about that. When I was playing with this yesterday I could have sworn |
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 🤔 |
@dougwilson Can we reopen and reset this to come from the 'Tried to move this off my [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.] |
@broofa for future reference you can't ever move a PR off of the branch it started on. |
Currently, it's difficult for downstream libs to pin
mime-types
to a specific version ofmime-db
. To do so, they have to (indirectly) do that by pinning to a specific version ofmime-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, whilemime-types@latest
is v2.1.35.Switching
mime-db
to a peer dependency has the following benefits:mime-db
version, and does so in an easy, intuitive manner (see below)mime-db
versionmime-types
with everymime-db
release.To Do:
mime-types
by runningnpm 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 thegeojson
type (for whatever reason), the logical way of doing this would benpm install mime-db@<desired version>
...Actual behavior
Why?
Making this change decouples
mime-types
frommime-db
. Users can installmime-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
andmime-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 ofmime-db
andmime-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 addingextensions
to thetext/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:
I very much prefer the latter perspective, as that's what downstream libs like
mime-types
andmime
are most concerned with . As the maintainer ofmime
, 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, ormime
's API. Thus, such changes are trivial wheremime
is concerned and I'd like to see that conveyed inmime-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 ofmime-db
data - i.e. the structures documented in @types/mime-db. And I think it appropriate to expectmime-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:MimeSource
typecompressible
field, or changing it's type.MimeSource
typeMimeEntry
type.extensions
for theapplication/javascript
entryextensions
to thetext/javascript
entryBasically changes to structure = "at least minor", while changes to data = "at most minor"
How should
mime-types
incorporatemime-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 thedb.json
data, whilemime-types
is (or at least, should) uses it to describe the API.Hence, this PR. Allowing users to directly specify both the
mime-db
andmime-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. :-)