Skip to content

feat!: move mime-db to peerDependencies#117

Open
broofa wants to merge 1 commit intojshttp:masterfrom
broofa:peers
Open

feat!: move mime-db to peerDependencies#117
broofa wants to merge 1 commit intojshttp:masterfrom
broofa:peers

Conversation

@broofa
Copy link
Member

@broofa broofa commented Dec 11, 2023

See prior conversation at #114

@dougwilson
Copy link
Contributor

I think the main part the convo left off is how to work through dealing with multiple mime-types majors being in the dep tree at the same time and them wanting different mime-db majors.

So like we'd need to have mime-types support at minimum one major mime-db version prior each time or have a delicate dance of getting everything updated all in order. I would guess that express would always need form-data, then superagent, and finally supertest to all update their mime-types major before express itself could if multuple mime-db majors are not supported by mime-types.

@ljharb
Copy link

ljharb commented Dec 11, 2023

A challenge is that if mime-types depends on mime-db with "v1 or v2", eg, then npm's auto-installing logic will install v2 by default, so v1 users would be forced to be explicit with their dependency - which could arguably be considered a breaking change to v1 users.

@broofa
Copy link
Member Author

broofa commented Dec 11, 2023

Yeah, that's kind of where my head went to. mime-types@next would have to be mime-db@1.x aware (declare something like "mime-db": ">=1.0.0 <3.0.0 in its peerDependencies), to allow downstream modules to update without being blocked by upstream dependencies.

@wesleytodd
Copy link
Member

I don't have a strong opinion either way here, but as we just published a new version of mime-db this came to my attention while I was updating it here. I would like to remove the pin (add ^) in the least so that we don't need to manually update here again, assuming no one is opposed. Removing the pin means we can at least reduce the impact of duplicates unless mime-db itself issues a major which seems likely to be really uncommon.

@wesleytodd
Copy link
Member

Opened a PR with the above change: #126

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.

4 participants