Repository navigation
chore: update accepts to v2 - #2016
shuaiyafei wants to merge 1 commit into
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideUpdates the project to File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Priority: ⬇️ Low Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation |
|---|---|---|
| Docstring Coverage |  |
No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0… |
| Linked Issues check | |
Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | |
Check skipped because no linked issues were found for this pull request. |
| Description Check | |
Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | |
The title clearly and concisely describes the main change: upgrading the accepts dependency to version 2. |
Full details: Docstring Coverage
-
Explanation
No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0 files. (1 skipped: 1 unsupported.)
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #2016 +/- ##
=======================================
Coverage 99.90% 99.90%
=======================================
Files 9 9
Lines 2144 2144
=======================================
Hits 2142 2142
Misses 2 2 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="package.json" line_range="39" />
<code_context>
"license": "MIT",
"dependencies": {
- "accepts": "^1.3.8",
+ "accepts": "^2.0.0",
"content-disposition": "~1.0.1",
"content-type": "^1.0.5",
</code_context>
<issue_to_address>
**Accept instance checks fail**
When a consumer installs `accepts@1` separately and uses its constructor to check Koa's `ctx.accept`, a consumer with its own `accepts@1` import gets `false` from `ctx.accept instanceof Accept`, because Koa now creates `ctx.accept` with the `accepts@2` constructor; constructor-based integrations reject the exposed object.
Avoid making constructor identity part of the exposed contract, or document that consumers must use the same `accepts` major version as Koa.
</issue_to_address>Sourcery assessment
Approval pending. 1 finding to address first.
Blocking findings: package.json:39
|
@shuaiyafei this is the right approach, but we should hold this off for v4, which will be started very soon, hopefully. Thank you so much! |
Koa directly uses
mime-typesv3 but still pulls a separate v2 tree throughacceptsv1. Upgradeacceptsto v2 and refresh the lockfile to share the existing MIME dependency and usenegotiatorv1.References #1903.
Validation:
npm ci --ignore-scripts,npm run build,npm run lint, andnpm test(466 passed). A focused compatibility check also confirmed that constructor identity is not stable across two separate copies ofaccepts@1, while the sampled public negotiation results match between v1 and v2.