Skip to content

feat(cookies): migrate cookies package into monorepo #5543

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.

Sign up for GitHub

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

Merged
fengmk2 merged 1 commit into from
Sep 26, 2025
Merged

feat(cookies): migrate cookies package into monorepo #5543

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 52 additions & 8 deletions CLAUDE.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@ This is the **Eggjs** framework - a progressive Node.js framework for building e
- **`packages/cluster/`** - Cluster management (merged from @eggjs/cluster)
- `src/` - Cluster TypeScript source code
- `test/` - Cluster test suite
- **`packages/cookies/`** - Cookie handling utilities (merged from @eggjs/cookies)
- `src/` - Cookies TypeScript source code
- `test/` - Cookies test suite with Mocha
- **`packages/koa/`** - Koa web framework (merged from @eggjs/koa)
Comment on lines +30 to 33
Copy link
Contributor

@coderabbitai coderabbitai bot Sep 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Fix test runner mention for cookies (uses Vitest, not Mocha).

Docs say cookies tests use Mocha, but the new tests import from vitest. Update to Vitest to avoid confusion.

Apply this diff:

-  - `test/` - Cookies test suite with Mocha
+  - `test/` - Cookies test suite with Vitest
📝 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.

Suggested change
- **`packages/cookies/`** - Cookie handling utilities (merged from @eggjs/cookies)
- `src/` - Cookies TypeScript source code
- `test/` - Cookies test suite with Mocha
- **`packages/koa/`** - Koa web framework (merged from @eggjs/koa)
- **`packages/cookies/`** - Cookie handling utilities (merged from @eggjs/cookies)
- `src/` - Cookies TypeScript source code
- `test/` - Cookies test suite with Vitest
- **`packages/koa/`** - Koa web framework (merged from @eggjs/koa)
🤖 Prompt for AI Agents 
In CLAUDE.md around lines 30 to 33, the docs incorrectly state the cookies test
suite uses Mocha; update the text to say Vitest instead. Replace the mention
"Cookies test suite with Mocha" with "Cookies test suite with Vitest" and ensure
any related references in this section (e.g., import examples or tooling notes)
consistently reference Vitest.

- `src/` - Koa TypeScript source code
- `test/` - Koa test suite
Expand Down Expand Up @@ -105,7 +108,9 @@ The framework follows a specific loading order:

- `pnpm -r run build` - Build all packages
- `pnpm -r run clean` - Clean dist directories in all packages
- `pnpm lint` - Run ESLint in all packages
- `pnpm -r run typecheck` - Run TypeScript type checking with `tsc --noEmit`
- `pnpm lint` - Run oxlint with type-aware checking in all packages
- `pnpm lint:fix` - Auto-fix linting issues with oxlint

### Examples

Expand Down Expand Up @@ -256,6 +261,10 @@ Plugins should configure their package.json following this pattern:
"scripts": {
"build": "tsdown",
"clean": "rimraf dist",
"typecheck": "tsc --noEmit",
"lint": "oxlint --type-aware",
"lint:fix": "npm run lint -- --fix",
"test": "npm run lint:fix && vitest",
"prepublishOnly": "npm run build"
}
}
Expand All @@ -281,13 +290,24 @@ Tool packages (like egg-bin) should be placed in the `tools/` directory:
### Testing Strategy

- **IMPORTANT: All new packages MUST use Vitest for testing** - this is the standard test runner for the monorepo
- **Exception: egg-bin uses Mocha** - the CLI tool package uses Mocha for consistency with CLI testing patterns
- **Exception: egg-bin and cookies use Mocha** - these packages use Mocha for consistency with their testing patterns
- Use `pnpm --filter=egg run test` for framework tests
- Test fixtures are in `packages/egg/test/fixtures/apps/`
- Create apps in fixtures to test specific scenarios
- Use `pnpm test` to run tests across all packages
- Follow existing test patterns for consistency

Comment on lines 292 to 299
Copy link
Contributor

@coderabbitai coderabbitai bot Sep 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Align “Testing Strategy” section: only egg-bin uses Mocha.

This block lists cookies as a Mocha exception, but the codebase uses Vitest for cookies tests. Update to avoid drift.

Apply this diff:

-- **Exception: egg-bin and cookies use Mocha** - these packages use Mocha for consistency with their testing patterns
+- **Exception: egg-bin uses Mocha** - this package uses Mocha for CLI testing; all other packages (including cookies) use Vitest
📝 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.

Suggested change
- **IMPORTANT: All new packages MUST use Vitest for testing** - this is the standard test runner for the monorepo
- **Exception: egg-bin uses Mocha** - the CLI tool package uses Mocha for consistency with CLI testing patterns
- **Exception: egg-bin and cookies use Mocha** - these packages use Mocha for consistency with their testing patterns
- Use `pnpm --filter=egg run test` for framework tests
- Test fixtures are in `packages/egg/test/fixtures/apps/`
- Create apps in fixtures to test specific scenarios
- Use `pnpm test` to run tests across all packages
- Follow existing test patterns for consistency
- **IMPORTANT: All new packages MUST use Vitest for testing** - this is the standard test runner for the monorepo
- **Exception: egg-bin uses Mocha** - this package uses Mocha for CLI testing; all other packages (including cookies) use Vitest
- Use `pnpm --filter=egg run test` for framework tests
- Test fixtures are in `packages/egg/test/fixtures/apps/`
- Create apps in fixtures to test specific scenarios
- Use `pnpm test` to run tests across all packages
- Follow existing test patterns for consistency
🤖 Prompt for AI Agents 
In CLAUDE.md around lines 292 to 299, the Testing Strategy currently states both
egg-bin and cookies use Mocha; update this so only egg-bin is listed as the
Mocha exception and remove cookies from the exception list. Modify the text to
read that all new packages must use Vitest, with the single exception of egg-bin
using Mocha, and keep the remaining testing instructions unchanged.

#### Linting and Type Checking Strategy

- **All packages must include TypeScript type checking** - Use `tsc --noEmit` in `typecheck` script
- **All packages use oxlint for linting** - No ESLint configurations should be present
- Use `oxlint --type-aware` for enhanced TypeScript checking
- oxlint automatically respects `.gitignore` patterns for file exclusion
- Package-specific scripts:
- `"typecheck": "tsc --noEmit"` - Pure TypeScript type checking
- `"lint": "oxlint --type-aware"` - Linting with type awareness
- Remove any `.eslintrc` or `.eslintrc.js` files when migrating packages

Comment on lines +300 to +310
Copy link
Contributor

@coderabbitai coderabbitai bot Sep 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Good: oxlint + tsc guidance. Ensure no ESLint leftovers.

Recommend verifying no residual ESLint configs remain in migrated packages.

Run:

🏁 Script executed:

#!/bin/bash
# Find ESLint configs and ESLint dependencies
fd -H '(.eslintrc|.eslintrc.*|eslint.config.*)' .
rg -nP '"eslint[^"]*"\s*:' package.json
rg -nP '\beslint\b' pnpm-workspace.yaml

Length of output: 219

Remove leftover ESLint configs and dependencies
– Delete the following .eslintrc files:
• packages/core/test/fixtures/helloworld-ts/.eslintrc
• tools/create-egg/src/templates/egg3-simple-js/.eslintrc
• tools/create-egg/src/templates/egg3-simple-ts/.eslintrc
– Remove eslint and eslint-config-egg entries from package.json

🤖 Prompt for AI Agents 
In CLAUDE.md around lines 300 to 310, the review asks to remove leftover ESLint
configs and deps: delete the three .eslintrc files at
packages/core/test/fixtures/helloworld-ts/.eslintrc,
tools/create-egg/src/templates/egg3-simple-js/.eslintrc, and
tools/create-egg/src/templates/egg3-simple-ts/.eslintrc, and remove eslint and
eslint-config-egg entries from any package.json files where they appear
(dependencies/devDependencies), updating package.json accordingly and running
any lockfile update (npm/yarn pnpm) to persist the removal; ensure no other
.eslintrc files remain in the repo.

#### Vitest Configuration

- Each package should include a `vitest.config.ts` file for test configuration
Expand Down Expand Up @@ -345,11 +365,17 @@ Tool packages (like egg-bin) should be placed in the `tools/` directory:

### Linting and Formatting

- **ESLint** - Used for code linting across all packages
- **TypeScript Compiler (tsc)** - Type checking with `tsc --noEmit`
- Ensures type safety without generating output files
- Run `pnpm -r run typecheck` for all packages
- **oxlint** - Fast, type-aware linter used across all packages
- Provides additional linting rules beyond TypeScript checking
- Significantly faster than ESLint with comparable rules
- Uses `--type-aware` flag for enhanced TypeScript analysis
- **Prettier** - Code formatting (primarily for documentation)
- Run `pnpm lint` to check code quality
- Run `pnpm lint` to check code quality with oxlint
- Run `pnpm lint:fix` to auto-fix linting issues
- Each package has its own `.eslintrc.js` extending from root configuration
- Each package uses oxlint which automatically respects `.gitignore` patterns

### TypeScript Best Practices

Expand Down Expand Up @@ -477,11 +503,14 @@ pnpm install
# Clean all build artifacts
pnpm -r run clean

# Check TypeScript types across all packages
pnpm -r run typecheck

# Check TypeScript configuration for specific package
pnpm --filter=<package> run typecheck

# Rebuild TypeScript references
pnpm run build:ts

# Check TypeScript configuration
pnpm --filter=<package> run tsc --noEmit
```

#### Test Failures
Expand Down Expand Up @@ -524,6 +553,21 @@ NODE_OPTIONS='--inspect-brk' pnpm --filter=egg run test

## Migration Guide

### Migrating from ESLint to oxlint

1. Remove ESLint dependencies from package.json:
- Remove `eslint`, `eslint-config-egg`, and any ESLint plugins
- Add `"oxlint": "catalog:"` to devDependencies
2. Delete `.eslintrc`, `.eslintrc.js`, or `.eslintrc.json` files
3. Update scripts in package.json:
- Add `"typecheck": "tsc --noEmit"` for TypeScript type checking
- Change `"lint": "eslint ..."` to `"lint": "oxlint --type-aware"`
- Add `"lint:fix": "npm run lint -- --fix"`
4. Ensure both type checking and linting are run:
- Use `tsc --noEmit` for pure TypeScript type checking
- Use `oxlint --type-aware` for additional linting rules
5. Run `pnpm install` to update dependencies

### Migrating from Egg v2 to v3

1. Update Node.js to v14+ (v18+ recommended)
Expand Down
7 changes: 7 additions & 0 deletions packages/cookies/.gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
node_modules
coverage
test/ts/report
package-lock.json
.tshy*
.eslintcache
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Sep 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

This .gitignore entry for .eslintcache seems to be a leftover from the previous setup. Since this pull request migrates the package from ESLint to oxlint, this entry is likely no longer necessary and can be removed to avoid confusion.

dist
Loading
Loading