-
Notifications
You must be signed in to change notification settings - Fork 29.9k
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
[@types/mime] Remove mime types in favour of mime@4.0.0
#69213
Conversation
@codershiba Thank you for submitting this PR! This is a live comment which I will keep updated. This PR touches some part of DefinitelyTyped infrastructure, so a DT maintainer will need to review it. This is rare — did you mean to do this? 1 package in this PR (and infra files)
Code ReviewsBecause this is a widely-used package, a DT maintainer will need to review it before it can be merged. You can test the changes of this PR in the Playground. Status
All of the items on the list are green. To merge, you need to post a comment including the string "Ready to merge" to bring in your changes. Diagnostic Information: What the bot saw about this PR{
"type": "info",
"now": "-",
"pr_number": 69213,
"author": "codershiba",
"headCommitOid": "c4b3053db4d474bcc1e305436da96ec18da4eff1",
"mergeBaseOid": "e59d0a7f1f9bc6dcdaa7cc1abe95cd3d9c6b81c7",
"lastPushDate": "2024-03-30T15:52:33.000Z",
"lastActivityDate": "2024-03-31T09:26:24.000Z",
"mergeOfferDate": "2024-03-30T22:10:17.000Z",
"mergeRequestDate": "2024-03-31T09:26:24.000Z",
"mergeRequestUser": "codershiba",
"hasMergeConflict": false,
"isFirstContribution": false,
"tooManyFiles": false,
"hugeChange": false,
"popularityLevel": "Critical",
"pkgInfo": [
{
"name": null,
"kind": "edit",
"files": [
{
"path": "notNeededPackages.json",
"kind": "infrastructure"
}
],
"owners": [],
"addedOwners": [],
"deletedOwners": [],
"popularityLevel": "Critical",
"isSafeInfrastructureEdit": false
},
{
"name": "mime",
"kind": "delete",
"files": [
{
"path": "types/mime/.npmignore",
"kind": "package-meta-ok"
},
{
"path": "types/mime/Mime.d.ts",
"kind": "definition"
},
{
"path": "types/mime/index.d.ts",
"kind": "definition"
},
{
"path": "types/mime/lite.d.ts",
"kind": "definition"
},
{
"path": "types/mime/mime-tests.ts",
"kind": "test"
},
{
"path": "types/mime/package.json",
"kind": "package-meta-ok"
},
{
"path": "types/mime/tsconfig.json",
"kind": "package-meta-ok"
},
{
"path": "types/mime/v1/.npmignore",
"kind": "package-meta-ok"
},
{
"path": "types/mime/v1/Mime.d.ts",
"kind": "definition"
},
{
"path": "types/mime/v1/index.d.ts",
"kind": "definition"
},
{
"path": "types/mime/v1/lite.d.ts",
"kind": "definition"
},
{
"path": "types/mime/v1/mime-tests.ts",
"kind": "test"
},
{
"path": "types/mime/v1/package.json",
"kind": "package-meta-ok"
},
{
"path": "types/mime/v1/tsconfig.json",
"kind": "package-meta-ok"
}
],
"owners": [
"jedigo",
"dhritzkiv"
],
"addedOwners": [],
"deletedOwners": [],
"popularityLevel": "Critical"
}
],
"reviews": [
{
"type": "approved",
"reviewer": "peterblazejewicz",
"date": "2024-03-30T22:09:34.000Z",
"isMaintainer": true
},
{
"type": "approved",
"reviewer": "dhritzkiv",
"date": "2024-03-30T15:58:11.000Z",
"isMaintainer": false
}
],
"mainBotCommentID": 2028163564,
"ciResult": "pass"
} |
🔔 @jedigo @dhritzkiv — please review this PR in the next few days. Be sure to explicitly select |
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.
@codershiba thanks!
Ready to merge |
@codershiba this breaks See #69231 |
Yeah I have tried to fix this in PR #69231 by pinning the version to a lower version. |
We should revert this; this PR should not have modified notNeededPaackages as the mime types are far too popular (22M weekly!!!) for a shim package to work. |
…pes in favour of `mime@4.0.0` by @codershiba" This reverts commit 8de1d4d.
Er, it's too late, |
Sorry for this, I could have undervalued the v3, it was listed as updated (npm) 2 years ago. |
No worries, I think the shim is actually okay in this instance given it's a clear major version difference. But it's also a failure of the tooling that we did not detect that the other package would break. |
Please fill in this template.
pnpm test <package to test>
.If removing a declaration:
notNeededPackages.json
.