feat(co-busboy): port co-busboy from JavaScript to TypeScript #5757
Conversation
Port the co-busboy library as @eggjs/co-busboy with full TypeScript support: - Replace `chan` library with native Promise-based queue implementation - Add comprehensive type definitions for Parts, FileStream, FieldTuple - Support autoFields, checkField, and checkFile options - Handle gzip/deflate compression via inflation - Enforce limits with proper 413 error codes - Port all tests from Mocha to Vitest (22 tests passing) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughAdds a new TypeScript package Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant Parser as parse()
participant Busboy
participant Queue as PromiseQueue
participant Validator as checkField/checkFile
participant Stream as FileStream / BlackHole
Client->>Parser: parse(request, options)
Parser->>Busboy: attach listeners (field, file, error, finish)
Busboy->>Queue: emit field event (name, val, info)
Queue->>Validator: checkField(name, val)
alt field valid
Queue-->>Client: yield FieldTuple
else field invalid
Queue-->>Client: emit error / stop
end
Busboy->>Queue: emit file event (stream, info)
Queue->>Validator: checkFile(fieldname, filename, encoding, mime)
alt file valid
Queue-->>Client: yield FileStream (readable with metadata)
else file invalid
Stream->>BlackHole: pipe to discard
Queue-->>Client: emit error / skip
end
Busboy->>Queue: emit finish/error
Queue-->>Client: yield null (end)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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 |
Summary of ChangesHello @fengmk2, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a significant upgrade to the Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
Deploying egg with
|
| Latest commit: |
fa792f3
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://cbbcaf6a.egg-cci.pages.dev |
| Branch Preview URL: | https://port-co-busboy.egg-cci.pages.dev |
Deploying egg-v3 with
|
| Latest commit: |
fa792f3
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://9eb44069.egg-v3.pages.dev |
| Branch Preview URL: | https://port-co-busboy.egg-v3.pages.dev |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## next #5757 +/- ##
==========================================
+ Coverage 87.56% 87.67% +0.10%
==========================================
Files 563 564 +1
Lines 10940 11043 +103
Branches 1242 1248 +6
==========================================
+ Hits 9580 9682 +102
- Misses 1276 1277 +1
Partials 84 84 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![gemini-code-assist[bot]](https://cdn.statically.io/img/avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This is a solid pull request that successfully ports co-busboy to TypeScript, introducing modern async/await patterns and comprehensive type definitions. The replacement of the chan library with a native Promise-based queue is a great improvement. The code is well-structured and the tests are quite thorough.
I've identified a potential bug in the error handling logic within onEnd that could swallow certain errors from busboy. I've also suggested an improvement to a test case to make it more robust by explicitly asserting that an error is thrown under specific conditions. Overall, these are minor issues in an otherwise excellent contribution.
| // Ignore 'Unexpected end of form' if we already have an error | ||
| if (!lastError && err && err.message !== 'Unexpected end of form') { | ||
| lastError = err; | ||
| debug('set lastError'); | ||
| } |
There was a problem hiding this comment.
The current logic for handling errors in onEnd seems to incorrectly swallow the 'Unexpected end of form' error, even when it's the only error that has occurred. This contradicts the behavior expected by the test should handle error: Unexpected end of form.
The condition err.message !== 'Unexpected end of form' prevents lastError from being set for this specific error, causing promiseQueue.push(null) to be called instead of propagating the error.
To align with the test expectations and provide more robust error handling, this condition should be removed to ensure all errors from busboy are captured and propagated if no other error has been recorded.
| // Ignore 'Unexpected end of form' if we already have an error | |
| if (!lastError && err && err.message !== 'Unexpected end of form') { | |
| lastError = err; | |
| debug('set lastError'); | |
| } | |
| // If an error is emitted by busboy and we haven't already captured an error, | |
| // let's store it. | |
| if (!lastError && err) { | |
| lastError = err; | |
| debug('set lastError: %s', err); | |
| } |
| it('will get nothing if set wrong encoding on gziped content', async () => { | ||
| const stream = gziped(); | ||
| delete (stream.headers as { 'content-encoding'?: string })['content-encoding']; | ||
|
|
||
| const parts = parse(stream as unknown as { headers: Record<string, string> }, { | ||
| autoFields: true, | ||
| }); | ||
| let promise; | ||
| let part; | ||
| let fields = 0; | ||
| let streams = 0; | ||
|
|
||
| while (((promise = parts()), (part = await promise))) { | ||
| assert.ok(promise instanceof Promise); | ||
| if (Array.isArray(part)) { | ||
| fields++; | ||
| } else { | ||
| streams++; | ||
| part.resume(); | ||
| } | ||
| } | ||
|
|
||
| assert.strictEqual(fields, 0); | ||
| assert.strictEqual(streams, 0); | ||
| assert.strictEqual(parts.fields.length, 0); | ||
| assert.strictEqual(Object.keys(parts.field).length, 0); | ||
| }); |
There was a problem hiding this comment.
This test correctly checks that no parts are parsed when the content-encoding header is missing from a gzipped request. However, it doesn't assert that an error is thrown, which is the expected behavior from busboy when it receives malformed data. The test currently passes if an error is thrown (because the while loop breaks), but it doesn't verify that this is what actually happened.
It would be more robust to explicitly assert that the promise rejects with an appropriate error, such as 'Unexpected end of form'. This ensures the error handling path is working as expected.
it('should throw an error for wrong encoding on gzipped content', async () => {
const stream = gziped();
delete (stream.headers as { 'content-encoding'?: string })['content-encoding'];
const parts = parse(stream as unknown as { headers: Record<string, string> }, {
autoFields: true,
});
await assert.rejects(async () => {
// eslint-disable-next-line no-empty
while (await parts()) {}
}, (err: Error) => {
assert.ok(
/Malformed part header|Unexpected end of form/.test(err.message),
`Expected a parsing error, but got: "${err.message}"`
);
return true;
});
assert.strictEqual(parts.fields.length, 0);
assert.strictEqual(Object.keys(parts.field).length, 0);
});![coderabbitai[bot]](https://cdn.statically.io/img/avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (4)
packages/co-busboy/package.json (2)
32-38: Consider adding explicit types export in publishConfigThe
publishConfig.exportsmaps"."to"./dist/index.js"but doesn't include an explicittypesfield. While TypeScript can often resolve types automatically, explicitly specifying types improves compatibility with different TypeScript moduleResolution settings.🔎 Suggested enhancement
"publishConfig": { "access": "public", "exports": { - ".": "./dist/index.js", + ".": { + "types": "./dist/index.d.ts", + "import": "./dist/index.js", + "default": "./dist/index.js" + }, "./package.json": "./package.json" } },
39-41: Consider adding standard scripts for consistencyThe scripts section only includes
typecheck. Other packages in the monorepo typically includebuildandtestscripts. If these are handled at the root level, this is fine, but consider adding them for standalone package development.packages/co-busboy/test/index.test.ts (2)
390-399: Test fixture cleanup is properly handled.The
beforeAll/afterAllhooks correctly manage the 10MB test file lifecycle. Consider wrappingfs.unlinkSyncin a try-catch to prevent test failures if the file was already removed.🔎 Defensive cleanup
afterAll(() => { - fs.unlinkSync(logfile); + try { + fs.unlinkSync(logfile); + } catch { + // File may already be removed + } });
653-672: Clarify test intent with a comment.This test verifies that
checkFileerrors are overridden by busboy's "Unexpected end of form" error when the form is malformed. The expected behavior might not be immediately obvious to future maintainers.🔎 Add clarifying comment
- it('should handle error: Unexpected end of form with checkFile', async () => { + // Verifies that busboy's parsing error takes precedence over checkFile validation errors + it('should handle error: Unexpected end of form with checkFile', async () => {
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (11)
packages/co-busboy/.gitignorepackages/co-busboy/CHANGELOG.mdpackages/co-busboy/LICENSEpackages/co-busboy/README.mdpackages/co-busboy/package.jsonpackages/co-busboy/src/index.tspackages/co-busboy/test/index.test.tspackages/co-busboy/tsconfig.jsonpackages/co-busboy/vitest.config.tspnpm-workspace.yamltsconfig.json
🧰 Additional context used
📓 Path-based instructions (12)
packages/*/tsconfig.json
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
All sub-project tsconfig.json files MUST extend from root tsconfig.json using 'extends': '../../tsconfig.json'
Files:
packages/co-busboy/tsconfig.json
**/tsconfig.json
📄 CodeRabbit inference engine (CLAUDE.md)
**/tsconfig.json: Sub-project TypeScript configurations must follow the minimal pattern with only 'extends: ../../tsconfig.json' and no additional options - all settings are managed at workspace root
Each package in the monorepo must include a 'tsconfig.json' file that extends from the root workspace tsconfig.json with only the 'extends' field
Files:
packages/co-busboy/tsconfig.jsontsconfig.json
**/package.json
📄 CodeRabbit inference engine (CLAUDE.md)
**/package.json: Use 'workspace:*' for internal package dependencies in package.json to ensure workspace linking
Use 'catalog:' prefix for external dependencies defined in pnpm-workspace.yaml catalog in package.json files
Add 'type: module' to package.json to enable ES Module support and convert from CommonJS to ESM
Files:
packages/co-busboy/package.json
packages/**/*.ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
packages/**/*.ts: Use TypeScript throughout all packages in the Eggjs monorepo
Use strict TypeScript mode in all packages
Files:
packages/co-busboy/vitest.config.tspackages/co-busboy/test/index.test.tspackages/co-busboy/src/index.ts
**/*.{ts,tsx,js,mjs}
📄 CodeRabbit inference engine (AGENTS.md)
Use
.tssources over CommonJS and prefer ESM for exportsUse ESM (ES Modules) syntax with 'import' and 'export' statements - add 'type: module' to package.json for CommonJS to ESM migration
Files:
packages/co-busboy/vitest.config.tspackages/co-busboy/test/index.test.tspackages/co-busboy/src/index.ts
{packages,plugins}/**/*.{ts,tsx,js,mjs}
📄 CodeRabbit inference engine (AGENTS.md)
Name files in lowercase with hyphens (e.g.
loader-context.ts)
Files:
packages/co-busboy/vitest.config.tspackages/co-busboy/test/index.test.tspackages/co-busboy/src/index.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Name classes in PascalCase
Name functions and variables in camelCase
Files:
packages/co-busboy/vitest.config.tspackages/co-busboy/test/index.test.tspackages/co-busboy/src/index.ts
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.ts: All exported functions and methods must have explicit return type annotations to support TypeScript's isolatedDeclarations flag
Avoid using computed property names with symbols in class declarations - use override methods instead for custom property definitions
Use explicit type annotations for class properties and add 'unique symbol' type for exported symbols to support isolatedDeclarations
Enable strict mode in all TypeScript packages and use explicit return types for public APIs
Prefer interfaces over type aliases for object shapes in TypeScript
Use readonly modifiers where appropriate in TypeScript class properties and interface definitions
Avoid using 'any' type in TypeScript - use 'unknown' when type is truly unknown
Use BaseContextClass as the base class for context-aware components like services and subscriptions in Egg applications
Monitor memory usage and implement proper cleanup in lifecycle hooks to avoid memory leaks in long-running processes
Avoid global state in worker processes - use application context or request context for state management
Files:
packages/co-busboy/vitest.config.tspackages/co-busboy/test/index.test.tspackages/co-busboy/src/index.ts
**/{tsdown,vitest}.config.ts
📄 CodeRabbit inference engine (CLAUDE.md)
All config file exports (tsdown.config.ts, vitest.config.ts) must use typed intermediate variables with explicit type annotations
Files:
packages/co-busboy/vitest.config.ts
packages/**/test/**/*.test.ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
packages/**/test/**/*.test.ts: Use Vitest as the standard test runner for all packages; test files follow pattern 'test/**/*.test.ts'
Use 'import { describe, it } from "vitest"' for test functions in Vitest test files
Use Node.js built-in 'assert' module for assertions in tests
Files:
packages/co-busboy/test/index.test.ts
**/test/**/*.test.ts
📄 CodeRabbit inference engine (AGENTS.md)
Configure Vitest to discover
**/test/**/*.test.tswithin each package and mirror this pattern when adding test suites
**/test/**/*.test.ts: Test files in Vitest-based packages must follow the naming pattern 'test/**/*.test.ts' and import test functions from vitest
Configure test files to use Node.js built-in 'assert' module for assertions in Vitest tests
Import test functions (describe, it, expect, beforeEach, afterEach) from 'vitest' package in test files
Files:
packages/co-busboy/test/index.test.ts
pnpm-workspace.yaml
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Define dependencies in pnpm-workspace.yaml catalog section and reference catalog entries with 'package-name': 'catalog:' or internal workspace dependencies with 'package-name': 'workspace:*'
Files:
pnpm-workspace.yaml
🧠 Learnings (46)
📓 Common learnings
Learnt from: CR
Repo: eggjs/egg PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-27T14:37:59.711Z
Learning: Applies to packages/**/*.ts : Use TypeScript throughout all packages in the Eggjs monorepo
📚 Learning: 2025-12-20T09:24:59.960Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-20T09:24:59.960Z
Learning: Run `pnpm run test` for Vitest suite or `pnpm run test:cov` for coverage before submitting
Applied to files:
packages/co-busboy/.gitignorepackages/co-busboy/test/index.test.ts
📚 Learning: 2025-12-20T09:24:59.960Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-20T09:24:59.960Z
Learning: Maintain Egg as a pnpm monorepo with core runtime code in `packages/egg`, supporting modules in `packages/core`, `packages/cluster`, and `packages/utils`, optional integrations under `plugins/`, examples in `examples/`, documentation site in `site/`, and unit tests beside packages under `test/` directories with fixtures in `test/fixtures/`
Applied to files:
packages/co-busboy/.gitignorepackages/co-busboy/package.jsontsconfig.json
📚 Learning: 2025-12-21T03:05:13.313Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: tegg/CLAUDE.md:0-0
Timestamp: 2025-12-21T03:05:13.313Z
Learning: Applies to tegg/core/*/tsconfig.json : Each new core package should include `tsconfig.json` extending `eggjs/tsconfig`
Applied to files:
packages/co-busboy/tsconfig.jsonpackages/co-busboy/package.jsontsconfig.json
📚 Learning: 2025-11-27T14:37:59.711Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-27T14:37:59.711Z
Learning: Applies to packages/*/tsconfig.json : All sub-project tsconfig.json files MUST extend from root tsconfig.json using 'extends': '../../tsconfig.json'
Applied to files:
packages/co-busboy/tsconfig.jsontsconfig.json
📚 Learning: 2025-12-21T14:11:29.307Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-21T14:11:29.307Z
Learning: Applies to **/tsconfig.json : Sub-project TypeScript configurations must follow the minimal pattern with only 'extends: ../../tsconfig.json' and no additional options - all settings are managed at workspace root
Applied to files:
packages/co-busboy/tsconfig.jsonpackages/co-busboy/vitest.config.tstsconfig.json
📚 Learning: 2025-12-21T14:11:29.307Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-21T14:11:29.307Z
Learning: Applies to **/tsconfig.json : Each package in the monorepo must include a 'tsconfig.json' file that extends from the root workspace tsconfig.json with only the 'extends' field
Applied to files:
packages/co-busboy/tsconfig.jsontsconfig.json
📚 Learning: 2025-11-27T14:37:59.711Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-27T14:37:59.711Z
Learning: Applies to packages/*/tsdown.config.ts : Use tsdown for unbundled ESM builds that preserves file structure; configure via tsdown.config.ts in each package
Applied to files:
packages/co-busboy/tsconfig.json
📚 Learning: 2025-11-27T14:37:59.711Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-27T14:37:59.711Z
Learning: Applies to packages/**/*.ts : Use TypeScript throughout all packages in the Eggjs monorepo
Applied to files:
packages/co-busboy/tsconfig.jsonpackages/co-busboy/package.json
📚 Learning: 2025-12-21T14:11:29.307Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-21T14:11:29.307Z
Learning: Applies to /tsconfig.json : Root tsconfig.json must extend from 'eggjs/tsconfig' and use ${configDir} variable for dynamic path resolution in sub-projects
Applied to files:
packages/co-busboy/tsconfig.json
📚 Learning: 2025-12-21T14:11:29.307Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-21T14:11:29.307Z
Learning: All packages must include TypeScript type checking using 'tsc --noEmit' in the typecheck script
Applied to files:
packages/co-busboy/tsconfig.jsontsconfig.json
📚 Learning: 2025-11-27T14:37:59.711Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-27T14:37:59.711Z
Learning: Applies to packages/**/*.ts : Use strict TypeScript mode in all packages
Applied to files:
packages/co-busboy/tsconfig.json
📚 Learning: 2025-12-21T14:11:29.307Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-21T14:11:29.307Z
Learning: Applies to **/*.ts : Enable strict mode in all TypeScript packages and use explicit return types for public APIs
Applied to files:
packages/co-busboy/tsconfig.jsonpackages/co-busboy/src/index.ts
📚 Learning: 2025-12-20T09:24:59.960Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-20T09:24:59.960Z
Learning: Applies to **/*.{ts,tsx,js,mjs} : Use `.ts` sources over CommonJS and prefer ESM for exports
Applied to files:
packages/co-busboy/tsconfig.json
📚 Learning: 2025-12-21T14:11:29.307Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-21T14:11:29.307Z
Learning: Applies to **/*.{ts,tsx,js,mjs} : Use ESM (ES Modules) syntax with 'import' and 'export' statements - add 'type: module' to package.json for CommonJS to ESM migration
Applied to files:
packages/co-busboy/tsconfig.jsonpackages/co-busboy/package.jsonpackages/co-busboy/src/index.ts
📚 Learning: 2025-12-21T14:11:29.307Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-21T14:11:29.307Z
Learning: Applies to **vitest.config.ts : All plugin Vitest configurations should use a vitest.config.ts file for test setup and configuration
Applied to files:
packages/co-busboy/tsconfig.jsonpackages/co-busboy/vitest.config.ts
📚 Learning: 2025-12-21T14:11:29.307Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-21T14:11:29.307Z
Learning: Applies to /tsconfig.json : Root tsconfig.json must use project references for all sub-packages and include them in the references array for proper incremental builds
Applied to files:
packages/co-busboy/tsconfig.jsontsconfig.json
📚 Learning: 2025-12-21T14:11:29.307Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-21T14:11:29.307Z
Learning: Applies to plugins/*/tsdown.config.ts : Most plugins do NOT need a tsdown.config.ts file - they inherit all settings from workspace root, only create one if you need custom options like copying assets
Applied to files:
packages/co-busboy/tsconfig.json
📚 Learning: 2025-12-21T14:11:29.307Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-21T14:11:29.307Z
Learning: Applies to plugins/*/package.json : Plugin package.json exports must mirror between development (./src/*.ts) and published (./dist/*.js) configurations - use publishConfig.exports to override for npm
Applied to files:
packages/co-busboy/package.json
📚 Learning: 2025-12-21T14:11:29.307Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-21T14:11:29.307Z
Learning: Applies to **/package.json : Add 'type: module' to package.json to enable ES Module support and convert from CommonJS to ESM
Applied to files:
packages/co-busboy/package.json
📚 Learning: 2025-12-21T03:05:13.313Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: tegg/CLAUDE.md:0-0
Timestamp: 2025-12-21T03:05:13.313Z
Learning: Applies to tegg/core/*/package.json : New core packages should include standard scripts in `package.json` with `typecheck` script using tsgo
Applied to files:
packages/co-busboy/package.json
📚 Learning: 2025-12-21T03:05:13.313Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: tegg/CLAUDE.md:0-0
Timestamp: 2025-12-21T03:05:13.313Z
Learning: Applies to tegg/**/package.json : Node.js version must be >= 22.18.0
Applied to files:
packages/co-busboy/package.json
📚 Learning: 2025-12-21T03:05:13.313Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: tegg/CLAUDE.md:0-0
Timestamp: 2025-12-21T03:05:13.313Z
Learning: Applies to tegg/**/package.json : Use `workspace:*` protocol for internal monorepo dependencies (both tegg and egg packages)
Applied to files:
packages/co-busboy/package.jsonpnpm-workspace.yaml
📚 Learning: 2025-12-21T03:05:13.313Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: tegg/CLAUDE.md:0-0
Timestamp: 2025-12-21T03:05:13.313Z
Learning: Applies to tegg/core/*/src/index.ts : Core packages should export public API through `src/index.ts`
Applied to files:
packages/co-busboy/package.json
📚 Learning: 2025-12-21T03:05:13.313Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: tegg/CLAUDE.md:0-0
Timestamp: 2025-12-21T03:05:13.313Z
Learning: Applies to tegg/**/package.json : Egg.js version must be >= 4.1.0
Applied to files:
packages/co-busboy/package.json
📚 Learning: 2025-12-21T14:11:29.307Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-21T14:11:29.307Z
Learning: Applies to plugins/*/package.json : All plugin package.json files must include egg in peerDependencies to ensure compatibility with the framework
Applied to files:
packages/co-busboy/package.json
📚 Learning: 2025-12-21T03:05:13.313Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: tegg/CLAUDE.md:0-0
Timestamp: 2025-12-21T03:05:13.313Z
Learning: Applies to tegg/**/vitest.config.ts : Configure test discovery through monorepo root `vitest.config.ts` which includes all tegg packages
Applied to files:
packages/co-busboy/vitest.config.ts
📚 Learning: 2025-11-27T14:37:59.711Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-27T14:37:59.711Z
Learning: Applies to packages/**/test/**/*.test.ts : Use Vitest as the standard test runner for all packages; test files follow pattern 'test/**/*.test.ts'
Applied to files:
packages/co-busboy/vitest.config.ts
📚 Learning: 2025-12-21T14:11:29.307Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-21T14:11:29.307Z
Learning: Applies to **/test/**/*.test.ts : Import test functions (describe, it, expect, beforeEach, afterEach) from 'vitest' package in test files
Applied to files:
packages/co-busboy/vitest.config.tspackages/co-busboy/test/index.test.ts
📚 Learning: 2025-12-20T09:24:59.960Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-20T09:24:59.960Z
Learning: Applies to **/test/**/*.test.ts : Configure Vitest to discover `**/test/**/*.test.ts` within each package and mirror this pattern when adding test suites
Applied to files:
packages/co-busboy/vitest.config.ts
📚 Learning: 2025-12-21T14:11:29.307Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-21T14:11:29.307Z
Learning: Applies to **/test/**/*.test.ts : Configure test files to use Node.js built-in 'assert' module for assertions in Vitest tests
Applied to files:
packages/co-busboy/vitest.config.tspackages/co-busboy/test/index.test.ts
📚 Learning: 2025-12-21T14:11:29.307Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-21T14:11:29.307Z
Learning: Applies to **/test/**/*.test.ts : Test files in Vitest-based packages must follow the naming pattern 'test/**/*.test.ts' and import test functions from vitest
Applied to files:
packages/co-busboy/vitest.config.ts
📚 Learning: 2025-11-27T14:37:59.711Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-27T14:37:59.711Z
Learning: Applies to packages/**/test/**/*.test.ts : Use 'import { describe, it } from "vitest"' for test functions in Vitest test files
Applied to files:
packages/co-busboy/vitest.config.ts
📚 Learning: 2025-12-21T03:05:13.313Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: tegg/CLAUDE.md:0-0
Timestamp: 2025-12-21T03:05:13.313Z
Learning: Applies to tegg/**/__tests__/**/*.test.{ts,tsx,js} : Use Vitest for testing all tegg packages
Applied to files:
packages/co-busboy/vitest.config.ts
📚 Learning: 2025-12-21T14:11:29.307Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-21T14:11:29.307Z
Learning: Applies to **/{tsdown,vitest}.config.ts : All config file exports (tsdown.config.ts, vitest.config.ts) must use typed intermediate variables with explicit type annotations
Applied to files:
packages/co-busboy/vitest.config.ts
📚 Learning: 2025-12-20T09:24:59.960Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-20T09:24:59.960Z
Learning: Add regression cases that exercise both CommonJS and TypeScript example apps when features affect HTTP or process orchestration
Applied to files:
packages/co-busboy/test/index.test.ts
📚 Learning: 2025-11-27T14:37:59.711Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-27T14:37:59.711Z
Learning: Applies to packages/**/test/**/*.test.ts : Use Node.js built-in 'assert' module for assertions in tests
Applied to files:
packages/co-busboy/test/index.test.ts
📚 Learning: 2025-11-27T14:37:59.711Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-27T14:37:59.711Z
Learning: Run 'pnpm run test' to validate changes (set timeout to 180+ seconds for all tests, 120+ seconds for coverage) and never cancel the command
Applied to files:
packages/co-busboy/test/index.test.ts
📚 Learning: 2025-12-21T14:11:29.307Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-21T14:11:29.307Z
Learning: Applies to **/*.ts : Use explicit type annotations for class properties and add 'unique symbol' type for exported symbols to support isolatedDeclarations
Applied to files:
packages/co-busboy/src/index.ts
📚 Learning: 2025-11-27T14:37:59.711Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-27T14:37:59.711Z
Learning: Applies to pnpm-workspace.yaml : Define dependencies in pnpm-workspace.yaml catalog section and reference catalog entries with 'package-name': 'catalog:' or internal workspace dependencies with 'package-name': 'workspace:*'
Applied to files:
pnpm-workspace.yaml
📚 Learning: 2025-12-21T14:11:29.307Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-21T14:11:29.307Z
Learning: Use pnpm workspace with catalog mode for centralized dependency management - define dependencies once in pnpm-workspace.yaml catalog and reference them using 'catalog:' in individual packages
Applied to files:
pnpm-workspace.yaml
📚 Learning: 2025-12-21T14:11:29.307Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-21T14:11:29.307Z
Learning: Applies to **/package.json : Use 'catalog:' prefix for external dependencies defined in pnpm-workspace.yaml catalog in package.json files
Applied to files:
pnpm-workspace.yaml
📚 Learning: 2025-12-21T03:05:13.313Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: tegg/CLAUDE.md:0-0
Timestamp: 2025-12-21T03:05:13.313Z
Learning: Applies to tegg/**/pnpm-workspace.yaml : Use pnpm workspaces with `catalog:` protocol for shared external dependencies
Applied to files:
pnpm-workspace.yaml
📚 Learning: 2025-12-21T14:11:29.307Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-21T14:11:29.307Z
Learning: Applies to **/package.json : Use 'workspace:*' for internal package dependencies in package.json to ensure workspace linking
Applied to files:
pnpm-workspace.yaml
📚 Learning: 2025-12-20T09:24:59.960Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-20T09:24:59.960Z
Learning: Run `pnpm install` to hydrate the workspace (Node.js ≥ 20.19.0 required)
Applied to files:
pnpm-workspace.yaml
📚 Learning: 2025-12-21T03:05:13.313Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: tegg/CLAUDE.md:0-0
Timestamp: 2025-12-21T03:05:13.313Z
Learning: Applies to tegg/**/pnpm-workspace.yaml : Centralize all shared dependency versions in the root `pnpm-workspace.yaml` rather than in individual tegg package files
Applied to files:
pnpm-workspace.yaml
🧬 Code graph analysis (1)
packages/co-busboy/test/index.test.ts (1)
packages/co-busboy/src/index.ts (1)
parse(193-370)
🪛 Biome (2.1.2)
packages/co-busboy/src/index.ts
[error] 266-266: The function should not return a value because its return type is void.
The function is here:
'void' signals the absence of value. The returned value is likely to be ignored by the caller.
(lint/correctness/noVoidTypeReturn)
[error] 318-318: The function should not return a value because its return type is void.
The function is here:
'void' signals the absence of value. The returned value is likely to be ignored by the caller.
(lint/correctness/noVoidTypeReturn)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (22)
- GitHub Check: Socket Security: Pull Request Alerts
- GitHub Check: Test (macos-latest, 22, 1/3)
- GitHub Check: Test (macos-latest, 22, 3/3)
- GitHub Check: Test (ubuntu-latest, 22, 1/3)
- GitHub Check: Test (ubuntu-latest, 24, 3/3)
- GitHub Check: Test (macos-latest, 20, 2/3)
- GitHub Check: Test (macos-latest, 20, 1/3)
- GitHub Check: Test (macos-latest, 24, 1/3)
- GitHub Check: Test (ubuntu-latest, 24, 1/3)
- GitHub Check: Test (ubuntu-latest, 22, 2/3)
- GitHub Check: Test (windows-latest, 20, 3/3)
- GitHub Check: Test (windows-latest, 22, 3/3)
- GitHub Check: Test (ubuntu-latest, 20, 3/3)
- GitHub Check: Test bin (ubuntu-latest, 24, 1/3)
- GitHub Check: Test bin (ubuntu-latest, 24, 0/3)
- GitHub Check: Test bin (windows-latest, 24, 2/3)
- GitHub Check: Test bin (windows-latest, 24, 0/3)
- GitHub Check: Test bin (ubuntu-latest, 24, 2/3)
- GitHub Check: Test bin (windows-latest, 24, 1/3)
- GitHub Check: typecheck
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Cloudflare Pages: egg-v3
🔇 Additional comments (18)
packages/co-busboy/LICENSE (1)
1-21: LGTM!Standard MIT license text is correctly applied to the new package.
packages/co-busboy/.gitignore (1)
1-4: LGTM!Standard ignore patterns for a TypeScript package with build outputs and test coverage.
packages/co-busboy/tsconfig.json (1)
1-3: LGTM!The TypeScript configuration correctly follows the monorepo pattern by extending from the root tsconfig.json with no additional options.
As per coding guidelines, this is the required minimal pattern for sub-project configurations.
tsconfig.json (1)
35-37: LGTM!The project reference for the new co-busboy package is correctly added to enable TypeScript's incremental builds and project references.
packages/co-busboy/README.md (1)
1-142: LGTM!The documentation is comprehensive and well-structured, covering:
- Installation and basic usage
- Complete API reference with TypeScript interfaces
- Error handling patterns
- Compression support
The examples clearly demonstrate both basic and advanced usage patterns.
packages/co-busboy/CHANGELOG.md (1)
13-13: The Node.js >= 22.18.0 requirement in the changelog is correct and intentional. This minimum version is documented in CLAUDE.md as being enforced across all packages in the monorepo to support modern JavaScript features and optimal performance. Node.js 22.18.0 introduced native TypeScript file execution support, which aligns with the framework's ESM-first approach.Likely an incorrect or invalid review comment.
pnpm-workspace.yaml (1)
27-27: The dependency versions are current and stable. Verification confirms:
busboy@^1.6.0— Latest version (1.6.0)inflation@^2.1.0— Latest version (2.1.0)@types/busboy@^1.5.4— Latest version (1.5.4, published May 2024)black-hole-stream@0.0.1— Only published version (pre-1.0, last published ~12 years ago; no newer version available)packages/co-busboy/package.json (1)
1-56: Package configuration follows project conventions.The package correctly uses ESM (
"type": "module"),workspace:*for internal dependencies, andcatalog:for external dependencies. The engine requirement and exports configuration align with the monorepo standards.packages/co-busboy/test/index.test.ts (3)
1-11: Imports follow project conventions.The test file correctly imports from
vitestfor test functions and uses Node.js built-inassertmodule for assertions, as required by the coding guidelines.
20-81: Well-structured test helpers with proper multipart fixtures.The mock request helpers correctly construct multipart form data with appropriate boundaries and content types. The type casting approach (
as unknown as MockRequest) is verbose but acceptable for test fixtures.
133-706: Comprehensive test suite with good coverage.The test suite thoroughly exercises the
parsefunction across multiple scenarios including autoFields mode, gzip compression, field/file limits, validation hooks, and error handling. The promise-based iteration tests in the "with promise" describe block validate the async API contract.packages/co-busboy/src/index.ts (7)
1-14: Clean imports and utility setup.The
@ts-expect-errorcomments for untyped packages (black-hole-stream,inflation) are appropriate. Usingdebuglogfromnode:utilis the correct approach for optional debug output.
16-108: Well-designed type definitions.The types are explicit and support
isolatedDeclarations. ThePartsinterface correctly models the callable function with attached properties.CoBusboyOptionsappropriately omitsheadersfromBusboyConfigsince headers come from the request.
114-160: Solid PromiseQueue implementation.The queue correctly handles the producer-consumer pattern with proper error propagation. One consideration: if
finishedis set totrueand there are items inpendingResolvers, they would remain unresolved. However, this scenario shouldn't occur in normal usage sinceonEndalways pushesnullor an error before finishing.
244-256: Good defensive initialization of field/fields.Initializing
parts.fieldandparts.fieldseven whenautoFieldsis false ensures type safety and prevents runtime errors if consumers accidentally access these properties.
361-370: Proper cleanup prevents memory leaks.The cleanup function correctly removes event listeners from both the inflated request stream and busboy. Keeping finish/error listeners ensures all data is processed before completion.
193-256: Well-structured parse function with comprehensive feature support.The implementation correctly handles:
- Koa context and raw Node.js requests
- Compressed request bodies (gzip/deflate)
- Field and file validation hooks
- Request limits with proper 413 status codes
- Auto-field collection mode
The docblock example is helpful for API consumers.
337-359: Well-handled end/error lifecycle with proper cleanup.The
setImmediatefor removing the error listener is a smart technique to catch late errors. The logic to ignore "Unexpected end of form" whenlastErrorexists ensures user-defined errors (fromcheckField/checkFile) take precedence.Note: When there's no prior error, "Unexpected end of form" will still be thrown since the condition
!lastError && err.message !== 'Unexpected end of form'prevents it from being set aslastError, but then line 354-355 won't execute either. Actually, reviewing again - iferr.message === 'Unexpected end of form'and!lastError, thenlastErrorstays undefined andnullis pushed. But the test expects the error to be thrown. Let me check the test again...The test at line 636-650 shows the error IS thrown. This works because busboy emits the error, and since there's no
lastError, the condition on line 348 evaluates tofalse(becauseerr.message === 'Unexpected end of form'), solastErrorremains undefined, andnullis pushed. Wait, that would mean the loop ends normally without error.Looking at the test more carefully - the error must come from somewhere else. The
invalidRequest()creates a form that will trigger an error in busboy during parsing, which might be a different error path.#!/bin/bash # Check how busboy handles invalid boundaries and what errors it emits rg -n "Unexpected end of form" --type ts -C 3
| function onField(name: string, val: string, info: FieldInfo): void { | ||
| const fieldnameTruncated = info.nameTruncated; | ||
| const valTruncated = info.valueTruncated; | ||
|
|
||
| if (checkField) { | ||
| const err = checkField(name, val, fieldnameTruncated, valTruncated); | ||
| if (err) { | ||
| debug('onField error: %s', err); | ||
| return onError(err); | ||
| } | ||
| } |
There was a problem hiding this comment.
Fix void return type violation.
The static analysis tool correctly identifies that return onError(err) returns a value from a function with implicit void return type. The return is used for early exit, but the value is meaningless.
🔎 Proposed fix
if (checkField) {
const err = checkField(name, val, fieldnameTruncated, valTruncated);
if (err) {
debug('onField error: %s', err);
- return onError(err);
+ onError(err);
+ return;
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function onField(name: string, val: string, info: FieldInfo): void { | |
| const fieldnameTruncated = info.nameTruncated; | |
| const valTruncated = info.valueTruncated; | |
| if (checkField) { | |
| const err = checkField(name, val, fieldnameTruncated, valTruncated); | |
| if (err) { | |
| debug('onField error: %s', err); | |
| return onError(err); | |
| } | |
| } | |
| function onField(name: string, val: string, info: FieldInfo): void { | |
| const fieldnameTruncated = info.nameTruncated; | |
| const valTruncated = info.valueTruncated; | |
| if (checkField) { | |
| const err = checkField(name, val, fieldnameTruncated, valTruncated); | |
| if (err) { | |
| debug('onField error: %s', err); | |
| onError(err); | |
| return; | |
| } | |
| } |
🧰 Tools
🪛 Biome (2.1.2)
[error] 266-266: The function should not return a value because its return type is void.
The function is here:
'void' signals the absence of value. The returned value is likely to be ignored by the caller.
(lint/correctness/noVoidTypeReturn)
🤖 Prompt for AI Agents
packages/co-busboy/src/index.ts around lines 258 to 268: the handler uses
"return onError(err)" which violates the function's void return type by
returning a value; call onError(err) without returning its value and explicitly
exit the function by using a bare "return;" (i.e. replace "return onError(err)"
with "onError(err); return;") so the early-exit is preserved but no value is
returned.
| if (checkFile) { | ||
| const err = checkFile(fieldname, file, filename, encoding, mimeType); | ||
| if (err) { | ||
| // Make sure request stream's data has been read | ||
| const blackHoleStream = new BlackHoleStream(); | ||
| file.pipe(blackHoleStream); | ||
| return onError(err); | ||
| } | ||
| } |
There was a problem hiding this comment.
Fix void return type violation.
Same issue as in onField - the return onError(err) statement returns a value from a void function.
🔎 Proposed fix
if (checkFile) {
const err = checkFile(fieldname, file, filename, encoding, mimeType);
if (err) {
// Make sure request stream's data has been read
const blackHoleStream = new BlackHoleStream();
file.pipe(blackHoleStream);
- return onError(err);
+ onError(err);
+ return;
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (checkFile) { | |
| const err = checkFile(fieldname, file, filename, encoding, mimeType); | |
| if (err) { | |
| // Make sure request stream's data has been read | |
| const blackHoleStream = new BlackHoleStream(); | |
| file.pipe(blackHoleStream); | |
| return onError(err); | |
| } | |
| } | |
| if (checkFile) { | |
| const err = checkFile(fieldname, file, filename, encoding, mimeType); | |
| if (err) { | |
| // Make sure request stream's data has been read | |
| const blackHoleStream = new BlackHoleStream(); | |
| file.pipe(blackHoleStream); | |
| onError(err); | |
| return; | |
| } | |
| } |
🧰 Tools
🪛 Biome (2.1.2)
[error] 318-318: The function should not return a value because its return type is void.
The function is here:
'void' signals the absence of value. The returned value is likely to be ignored by the caller.
(lint/correctness/noVoidTypeReturn)
🤖 Prompt for AI Agents
In packages/co-busboy/src/index.ts around lines 312 to 320, the handler is using
"return onError(err)" inside a void function which causes a void return type
violation; instead call onError(err) without returning its value and then return
void to stop execution. Concretely, after piping the file to the
BlackHoleStream, invoke onError(err); and then use a bare return (or otherwise
exit the function) so no value is returned from the void function.
| import { defineProject } from 'vitest/config'; | ||
|
|
||
| export default defineProject({ | ||
| test: { | ||
| testTimeout: 15000, | ||
| include: ['test/**/*.test.ts'], | ||
| exclude: ['**/test/fixtures/**', '**/node_modules/**', '**/dist/**'], | ||
| }, | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add explicit type annotation for the config export.
According to coding guidelines, config file exports must use typed intermediate variables with explicit type annotations to support better type checking and IDE support.
🔎 Proposed fix
-import { defineProject } from 'vitest/config';
+import { defineProject, type UserProjectConfigExport } from 'vitest/config';
-export default defineProject({
+const config: UserProjectConfigExport = defineProject({
test: {
testTimeout: 15000,
include: ['test/**/*.test.ts'],
exclude: ['**/test/fixtures/**', '**/node_modules/**', '**/dist/**'],
},
});
+
+export default config;Based on coding guidelines for config file exports.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import { defineProject } from 'vitest/config'; | |
| export default defineProject({ | |
| test: { | |
| testTimeout: 15000, | |
| include: ['test/**/*.test.ts'], | |
| exclude: ['**/test/fixtures/**', '**/node_modules/**', '**/dist/**'], | |
| }, | |
| }); | |
| import { defineProject, type UserProject } from 'vitest/config'; | |
| const config: UserProject = defineProject({ | |
| test: { | |
| testTimeout: 15000, | |
| include: ['test/**/*.test.ts'], | |
| exclude: ['**/test/fixtures/**', '**/node_modules/**', '**/dist/**'], | |
| }, | |
| }); | |
| export default config; |
🤖 Prompt for AI Agents
In packages/co-busboy/vitest.config.ts lines 1-9, the default export lacks an
explicit typed intermediate variable; create a const (e.g., const config:
UserConfig) with an explicit type annotation imported as a type from Vitest
(import type { UserConfig } from 'vitest' or 'vitest/config'), assign the
existing object to that const, and then pass that typed const into defineProject
and export it as the default export to satisfy the typing guideline and improve
IDE/type-checking support.
There was a problem hiding this comment.
Pull request overview
This PR ports the co-busboy library from JavaScript to TypeScript as a new monorepo package @eggjs/co-busboy. The implementation replaces the deprecated chan library with a native Promise-based queue, adds comprehensive TypeScript type definitions, and migrates all tests from Mocha to Vitest.
Key changes include:
- New TypeScript-based Promise queue implementation replacing the
chandependency - Comprehensive type definitions for Parts, FileStream, FieldTuple, and configuration options
- Full test suite migration to Vitest with 22 passing tests covering all functionality
Reviewed changes
Copilot reviewed 12 out of 14 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tsconfig.json | Adds co-busboy package reference to project configuration |
| pnpm-workspace.yaml | Adds required dependencies (@types/busboy, busboy, inflation, black-hole-stream) to catalog |
| pnpm-lock.yaml | Lock file updates for new dependencies |
| packages/co-busboy/package.json | Package configuration with ESM exports and Node.js >=22.18.0 requirement |
| packages/co-busboy/tsconfig.json | TypeScript configuration extending root config |
| packages/co-busboy/vitest.config.ts | Vitest test configuration |
| packages/co-busboy/src/index.ts | Main implementation with PromiseQueue and parse function |
| packages/co-busboy/test/index.test.ts | Comprehensive test suite (22 tests) covering parsing, compression, validation, and error handling |
| packages/co-busboy/README.md | API documentation and usage examples |
| packages/co-busboy/CHANGELOG.md | Initial release changelog |
| packages/co-busboy/LICENSE | MIT license |
| packages/co-busboy/.gitignore | Standard ignores for dist and coverage |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
| if (this.finished) return; | ||
|
|
||
| // If item is null, mark as finished | ||
| if (item === null) { | ||
| this.finished = true; | ||
| } | ||
|
|
||
| const resolver = this.pendingResolvers.shift(); | ||
| if (resolver) { | ||
| if (item instanceof Error) { | ||
| resolver.reject(item); | ||
| } else { | ||
| resolver.resolve(item); | ||
| } |
There was a problem hiding this comment.
The PromiseQueue implementation has a potential memory leak. When an error is pushed to the queue after the queue is marked as finished, pending resolvers are never cleared. If push(null) is called to mark the queue as finished, followed by push(error), the error will be ignored (due to the early return on line 123), but any pending resolvers waiting for data will never be resolved or rejected, causing a memory leak.
This can occur if multiple errors are encountered during stream processing or if errors arrive after the stream finishes. Consider removing the early return or handling errors even when finished is true to ensure all pending resolvers are properly cleaned up.
| if (this.finished) return; | |
| // If item is null, mark as finished | |
| if (item === null) { | |
| this.finished = true; | |
| } | |
| const resolver = this.pendingResolvers.shift(); | |
| if (resolver) { | |
| if (item instanceof Error) { | |
| resolver.reject(item); | |
| } else { | |
| resolver.resolve(item); | |
| } | |
| // Handle terminal signals even if we're already marked as finished | |
| if (item === null) { | |
| // Normal completion: mark finished and resolve all pending pulls with null | |
| this.finished = true; | |
| while (this.pendingResolvers.length > 0) { | |
| const { resolve } = this.pendingResolvers.shift()!; | |
| resolve(null); | |
| } | |
| return; | |
| } | |
| if (item instanceof Error) { | |
| // Error completion: mark finished and reject all pending pulls | |
| this.finished = true; | |
| while (this.pendingResolvers.length > 0) { | |
| const { reject } = this.pendingResolvers.shift()!; | |
| reject(item); | |
| } | |
| return; | |
| } | |
| // Non-terminal item; ignore if we've already finished | |
| if (this.finished) return; | |
| const resolver = this.pendingResolvers.shift(); | |
| if (resolver) { | |
| resolver.resolve(item); |
| // It's a file stream with additional properties | ||
| console.log('File:', part.filename, part.mimeType); | ||
| // Consume the stream | ||
| part.pipe(fs.createWriteStream(`./uploads/${part.filename}`)); |
There was a problem hiding this comment.
In the usage example, the code writes uploaded files directly to ./uploads/${part.filename}, which uses the attacker-controlled filename value to construct a filesystem path. An attacker can supply values like ../../../../etc/passwd as filename to perform path traversal and overwrite or create arbitrary files outside the intended upload directory. To avoid this, normalize and strictly validate or replace the original filename before using it in a filesystem path, and join it to a fixed upload directory in a way that prevents escaping that directory.
- Update multipart plugin to use the new @eggjs/co-busboy package - Import Part type for proper type narrowing - Add truncated property to FileStream interface - Fix type assertions for AsyncIterable return 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
fengmk2
force-pushed
the
port-co-busboy
branch
from
392c61c to
fa792f3
Compare
Port the co-busboy library as @eggjs/co-busboy with full TypeScript support:
chanlibrary with native Promise-based queue implementationclose #5756
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Tests
Chores
Style
✏️ Tip: You can customize this high-level summary in your review settings.