refactor(koa): use @eggjs/cookies instead of cookies #5744
Conversation
WalkthroughThe pull request migrates the Koa package dependency from the standard Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 undertakes a significant refactoring within the Koa package, transitioning its cookie handling mechanism from an external Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. 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: |
b70ba8a
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://6fd46078.egg-cci.pages.dev |
| Branch Preview URL: | https://use-egg-cookies.egg-cci.pages.dev |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## next #5744 +/- ##
==========================================
- Coverage 87.61% 87.60% -0.01%
==========================================
Files 563 563
Lines 10930 10930
Branches 1238 1239 +1
==========================================
- Hits 9576 9575 -1
- Misses 1270 1271 +1
Partials 84 84 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the Koa package to use @eggjs/cookies instead of the cookies package. The changes are mostly straightforward dependency and API usage updates. I've identified a couple of areas for improvement: one regarding a FIXME comment that should be removed for better code clarity, and another in a test case to make its intent clearer and more robust after the dependency change. Overall, the refactoring is well-executed.
Deploying egg-v3 with
|
| Latest commit: |
b70ba8a
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://cacae6bb.egg-v3.pages.dev |
| Branch Preview URL: | https://use-egg-cookies.egg-v3.pages.dev |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 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 (4)
packages/koa/package.json(1 hunks)packages/koa/src/context.ts(2 hunks)packages/koa/test/context/cookies.test.ts(2 hunks)pnpm-workspace.yaml(1 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
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
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/koa/src/context.tspackages/koa/test/context/cookies.test.ts
**/*.{ts,tsx,js,mjs}
📄 CodeRabbit inference engine (AGENTS.md)
Use
.tssources over CommonJS and prefer ESM for exports
Files:
packages/koa/src/context.tspackages/koa/test/context/cookies.test.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/koa/src/context.tspackages/koa/test/context/cookies.test.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Name classes in PascalCase
Name functions and variables in camelCase
**/*.{ts,tsx}: Use explicit return types for all exported functions, methods, and getters to support TypeScript's--isolatedDeclarationsflag
Avoid computed property names with symbols in class declarations; use override methods instead for compatibility with--isolatedDeclarations
Add explicit type annotations for class properties, particularly for exported symbols which should useunique symboltype
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 classes and interfaces
Avoidanytype; useunknownwhen type is truly unknown
Files:
packages/koa/src/context.tspackages/koa/test/context/cookies.test.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/koa/test/context/cookies.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
Files:
packages/koa/test/context/cookies.test.ts
🧠 Learnings (15)
📚 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/koa/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/plugin/*/package.json : New plugins should define `eggPlugin` in `package.json` with dependencies
Applied to files:
packages/koa/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/**/package.json : Egg.js version must be >= 4.1.0
Applied to files:
packages/koa/package.jsonpnpm-workspace.yaml
📚 Learning: 2025-12-21T03:04:53.982Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-21T03:04:53.982Z
Learning: Applies to package.json : Use `workspace:*` for internal package dependencies and `catalog:` for external dependencies defined in pnpm-workspace.yaml
Applied to files:
packages/koa/package.jsonpnpm-workspace.yaml
📚 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/koa/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/koa/package.json
📚 Learning: 2025-12-21T03:04:53.982Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-21T03:04:53.982Z
Learning: All packages must include TypeScript type checking with `tsc --noEmit` in the `typecheck` script
Applied to files:
packages/koa/package.json
📚 Learning: 2025-12-21T03:04:53.982Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-21T03:04:53.982Z
Learning: All packages must define a `src/types.ts` file that extends the Egg module declarations using `declare module 'egg'` pattern
Applied to files:
packages/koa/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/**/pnpm-workspace.yaml : Use pnpm workspaces with `catalog:` protocol for shared external dependencies
Applied to files:
packages/koa/package.jsonpnpm-workspace.yaml
📚 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:
packages/koa/package.jsonpnpm-workspace.yaml
📚 Learning: 2025-12-21T03:04:53.982Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-21T03:04:53.982Z
Learning: All new packages must use Vitest for testing; Exception: egg-bin and cookies packages use Mocha
Applied to files:
packages/koa/package.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/**/test/**/*.test.ts : Use Node.js built-in 'assert' module for assertions in tests
Applied to files:
packages/koa/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/**/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
📚 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:
pnpm-workspace.yaml
📚 Learning: 2025-12-21T03:04:53.982Z
Learnt from: CR
Repo: eggjs/egg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-21T03:04:53.982Z
Learning: Applies to plugins/*/package.json : All plugins must include egg in peerDependencies and define `build`, `clean`, and `prepublishOnly` scripts in package.json
Applied to files:
pnpm-workspace.yaml
⏰ 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). (18)
- GitHub Check: Test (macos-latest, 24, 2/3)
- GitHub Check: Test (windows-latest, 24, 1/3)
- GitHub Check: Test (macos-latest, 20, 3/3)
- GitHub Check: Test (windows-latest, 22, 1/3)
- GitHub Check: Test (windows-latest, 20, 3/3)
- GitHub Check: Test (ubuntu-latest, 24, 3/3)
- GitHub Check: Test (ubuntu-latest, 24, 2/3)
- GitHub Check: Test (ubuntu-latest, 22, 2/3)
- GitHub Check: Test (macos-latest, 24, 3/3)
- GitHub Check: Test bin (ubuntu-latest, 24, 0/3)
- GitHub Check: Test bin (windows-latest, 24, 2/3)
- GitHub Check: Test bin (ubuntu-latest, 24, 1/3)
- GitHub Check: Test bin (ubuntu-latest, 24, 2/3)
- GitHub Check: Test bin (windows-latest, 24, 0/3)
- GitHub Check: typecheck
- GitHub Check: cnpmcore E2E test
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Cloudflare Pages: egg-v3
🔇 Additional comments (5)
packages/koa/src/context.ts (1)
5-5: LGTM: Import updated to named export.The change from default import to named import correctly aligns with the @eggjs/cookies API.
packages/koa/test/context/cookies.test.ts (2)
11-13: LGTM: Tests updated for new Koa constructor API.The test correctly demonstrates the new public API where Koa accepts an options object with a
keysproperty. This is a useful ergonomic improvement over settingapp.keysseparately.
45-45: LGTM: Error message reflects @eggjs/cookies validation.The updated error message "keys must be provided and should be an array" accurately reflects the validation from the @eggjs/cookies library. This test confirms that the error is thrown when attempting to set signed cookies without keys, not during Cookies initialization.
packages/koa/package.json (1)
43-43: LGTM: Dependency correctly uses workspace protocol.The addition of
"@eggjs/cookies": "workspace:*"correctly follows the monorepo convention for internal workspace dependencies. The removal of the externalcookiespackage and@types/cookies(as noted in the AI summary) properly completes the migration to the internal @eggjs/cookies implementation.Based on learnings about using
workspace:*for internal monorepo dependencies.pnpm-workspace.yaml (1)
14-14: Remove or correct the@eggjs/cookiesversion specification.The version
^3.1.0appears not to exist as a stable release. Latest releases show@eggjs/cookiesat4.0.0-beta.x. Verify and pin to an actual stable version.
| // FIXME: keys is required for encrypt/sign cookies | ||
| this._cookies = new Cookies(this, this.app.keys ?? [], { | ||
| secure: this.request.secure, | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, let's look at the actual file to understand the context
echo "=== Viewing packages/koa/src/context.ts around lines 226-229 ==="
sed -n '220,235p' packages/koa/src/context.ts
# Check for @eggjs/cookies type definitions or documentation in the codebase
echo -e "\n=== Looking for @eggjs/cookies references ==="
rg -n "@eggjs/cookies" --type ts packages/
# Search for Cookies constructor usage across the codebase
echo -e "\n=== All Cookies constructor usage ==="
rg -n "new Cookies\(" --type ts -C 3
# Check the package.json to see the @eggjs/cookies version
echo -e "\n=== Checking @eggjs/cookies dependency ==="
rg "@eggjs/cookies" packages/koa/package.json -A 2 -B 2Repository: eggjs/egg
Length of output: 2438
🌐 Web query:
@eggjs/cookies constructor API documentation
💡 Result:
Brief summary — constructor and its API
-
Constructor signature: new Cookies(ctx, keys[, defaultCookieOptions]) — or call Cookies(ctx, keys, defaultCookieOptions). [1]
-
Parameters:
- ctx: Koa/Egg context (request/response wrapper). [1][2]
- keys: signing/encryption keys (string(s) / Keygrip). First key used for signing/encryption; additional keys used to verify/decrypt. [1][2]
- defaultCookieOptions (optional): defaults applied to set/get if not overridden. Common options include:
- maxAge (Number, ms), expires (Date), path (String), domain (String). [2]
- httpOnly (Boolean), signed (Boolean), encrypt (Boolean), secure (Boolean), sameSite (String). [2]
- (Egg docs/releases also note newer cookie attrs such as partitioned, priority, removeUnpartitioned for recent versions). [2][3]
-
Notes:
- encrypt behaves like signed but encrypts the value (requires keys). Use same option on get and set. [1][2]
- When cookie value length > ~4093 bytes an event is emitted (cookieLimitExceed). [1]
Example:
- import { Cookies } from '@eggjs/cookies';
const cookies = new Cookies(ctx, keys, { httpOnly: true, encrypt: true });
cookies.set('foo', 'bar', { maxAge: 24 * 3600 * 1000 }); [1]
Sources:
- @eggjs/cookies README (npm). [1]
- Egg.js documentation — Cookie & Session (core docs). [2]
- Egg.js release notes mentioning added cookie options (partitioned / priority / removeUnpartitioned). [3]
Address the empty keys fallback behavior.
The FIXME comment indicates keys are required for signed/encrypted cookies, yet the code provides an empty array fallback (this.app.keys ?? []). While this prevents initialization errors, it could lead to runtime failures when developers attempt to use signed or encrypted cookies without configuring keys.
Consider either:
- Throwing an error during initialization if keys are required for your use case, or
- Documenting that keys can be set via
app.keysbefore using signed/encrypted cookies
🤖 Prompt for AI Agents
In packages/koa/src/context.ts around lines 226 to 229, the code falls back to
an empty array for this.app.keys which masks the fact that keys are required for
signed/encrypted cookies; change the initialization to fail fast by throwing a
clear Error when this.app.keys is null/undefined (instead of using []), with a
message instructing developers to set app.keys (e.g. "app.keys is required for
signed/encrypted cookies; set app.keys before creating the app"), so
initialization surfaces the misconfiguration and prevents later runtime
failures.
There was a problem hiding this comment.
Pull request overview
This PR refactors the @eggjs/koa package to use the internal @eggjs/cookies package instead of the external cookies package, closing issue #5735. The change modernizes the cookie handling by adopting the in-house implementation that extends the original pillarjs/cookies with additional features like encryption.
- Replaced external
cookiespackage with workspace package@eggjs/cookies - Updated Cookies API usage to match the new constructor signature
- Updated test expectations to match new error messages
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| pnpm-workspace.yaml | Added @eggjs/cookies to the catalog for version documentation |
| pnpm-lock.yaml | Removed external cookies and @types/cookies dependencies, added workspace link to internal @eggjs/cookies package |
| packages/koa/package.json | Replaced cookies dependency with workspace reference to @eggjs/cookies, removed @types/cookies from devDependencies |
| packages/koa/src/context.ts | Changed import from default to named export, updated Cookies constructor to use new API signature (ctx, keys, options) |
| packages/koa/test/context/cookies.test.ts | Added keys initialization to unsigned cookie test, updated error message assertion to match new Keygrip validation message |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
close #5735
Summary by CodeRabbit
New Features
new Koa({ keys: [...] })keysproperty for accessing signing key configurationChanges
✏️ Tip: You can customize this high-level summary in your review settings.