Skip to content

refactor: clean up API surface, fix metadata, and improve internals#770

Merged
tada5hi merged 2 commits intomasterfrom
api-surface
Apr 9, 2026
Merged

refactor: clean up API surface, fix metadata, and improve internals#770
tada5hi merged 2 commits intomasterfrom
api-surface

Conversation

@tada5hi
Copy link
Copy Markdown
Contributor

@tada5hi tada5hi commented Apr 9, 2026

Summary by CodeRabbit

  • Breaking Changes

    • Removed subpath exports (./body, ./cookie, ./query) from @routup/basic package
    • Removed handler and parse/stringify re-exports from @routup/cookie and @routup/query
    • Updated @routup/i18n ilingo peer dependency to ^5.0.0
  • Bug Fixes

    • Fixed package descriptions in Prometheus and rate-limit-redis packages
    • Corrected import path in Swagger documentation
  • Documentation

    • Updated rate-limit examples with improved IP parameter handling
    • Cleaned up i18n README formatting
  • Tests

    • Added test scripts to rate-limit-redis package

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 9, 2026

📝 Walkthrough

Walkthrough

This PR consolidates internal symbol constants across the body package, simplifies export maps in the basic package, refactors handler delegation patterns, removes re-exports from cookie and query packages, and updates documentation and metadata across multiple packages.

Changes

Cohort / File(s) Summary
Symbol Centralization
packages/body/src/constants.ts, packages/body/src/helpers/...
Created centralized constants.ts exporting OptionsSymbol, BodySymbol, and RawBodySymbol, then updated options.ts, read-raw.ts, and read.ts to import these symbols instead of defining them locally.
Export Map Simplification
packages/basic/package.json, packages/basic/src/body.ts, packages/basic/src/cookie.ts, packages/basic/src/query.ts, packages/basic/src/index.ts, packages/basic/tsdown.config.ts
Removed subpath exports (./body, ./cookie, ./query) from package exports; removed corresponding re-exports from source files; reduced tsdown build entry from 4 modules to single src/index.ts.
Handler Delegation Refactoring
packages/assets/src/handler.ts, packages/swagger/src/ui/handler.ts
Simplified handler delegation by passing handler function directly to defineCoreHandler instead of wrapping in inline arrow function callback.
Re-export Removals
packages/cookie/src/index.ts, packages/query/src/index.ts
Removed export * from './handler' from both packages; query also removed named exports for parse and stringify from qs package.
Query Test Adjustment
packages/query/test/unit/module.spec.ts
Updated test to import stringify from qs package directly instead of relying on re-export from local module.
Rate Limit Internal Refactor
packages/rate-limit/src/request.ts
Renamed internal symbol constant from generic symbol to descriptive RateLimitSymbol for clarity; updated all read/write sites accordingly.
Documentation & Metadata Updates
packages/assets/README.md, packages/assets/package.json, packages/i18n/README.md, packages/i18n/package.json, packages/prometheus/package.json, packages/rate-limit/README.md, packages/rate-limit/package.json, packages/rate-limit-redis/package.json, packages/swagger/README.md
Updated npm badge URLs, repository directory references, package descriptions, peer dependencies (ilingo ^3.2.0^5.0.0), added npm test scripts, corrected documentation imports and README examples.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Poem

🐰 Symbols unite in their constants home,
Exports grow lean, less to roam,
Handlers now flow without extra wrap,
Clean and direct—no callback gap!
A tidy refactor, symbols all known,
The codebase's garden, beautifully grown.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main changes: removing unused API exports, updating metadata fields, and refactoring internal implementations across multiple packages.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch api-surface

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/i18n/package.json`:
- Line 50: Add a CHANGELOG entry documenting the dependency bump of "ilingo" to
^5.0.0: note this is a major, breaking change (ESM-only requirement and the API
rename Store → IStore), state that `@routup/i18n` itself is compatible but
downstream consumers who depend on ilingo v3 or use transitive v3 should be
warned, and include migration guidance (update imports to ESM and rename Store
to IStore) and the package.json change ("ilingo": "^5.0.0") as the trigger for
the entry.

In `@packages/rate-limit-redis/package.json`:
- Around line 21-22: The package.json scripts "test" and "test:coverage" invoke
the vitest CLI but vitest is not declared in devDependencies; add "vitest"
(choose a current compatible semver, e.g. "^1.0.0" or your repo's standard
version) to the devDependencies object in packages/rate-limit-redis/package.json
so the scripts can run, then run npm/yarn/pnpm install to update lockfiles;
ensure the version aligns with other workspace packages to avoid mismatches.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: d756c18f-87bd-43ad-a1aa-d0517f6d6028

📥 Commits

Reviewing files that changed from the base of the PR and between c23f388 and e20b017.

📒 Files selected for processing (25)
  • packages/assets/README.md
  • packages/assets/package.json
  • packages/assets/src/handler.ts
  • packages/basic/package.json
  • packages/basic/src/body.ts
  • packages/basic/src/cookie.ts
  • packages/basic/src/index.ts
  • packages/basic/src/query.ts
  • packages/basic/tsdown.config.ts
  • packages/body/src/constants.ts
  • packages/body/src/helpers/options.ts
  • packages/body/src/helpers/read-raw.ts
  • packages/body/src/helpers/read.ts
  • packages/cookie/src/index.ts
  • packages/i18n/README.md
  • packages/i18n/package.json
  • packages/prometheus/package.json
  • packages/query/src/index.ts
  • packages/query/test/unit/module.spec.ts
  • packages/rate-limit-redis/package.json
  • packages/rate-limit/README.md
  • packages/rate-limit/package.json
  • packages/rate-limit/src/request.ts
  • packages/swagger/README.md
  • packages/swagger/src/ui/handler.ts
💤 Files with no reviewable changes (8)
  • packages/i18n/README.md
  • packages/basic/src/cookie.ts
  • packages/basic/src/body.ts
  • packages/basic/src/index.ts
  • packages/basic/src/query.ts
  • packages/query/src/index.ts
  • packages/basic/package.json
  • packages/cookie/src/index.ts

"homepage": "https://github.com/routup/plugins#readme",
"peerDependencies": {
"ilingo": "^3.2.0",
"ilingo": "^5.0.0",
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🌐 Web query:

ilingo npm package version 5.0.0 breaking changes migration guide

💡 Result:

ilingo v5.0.0 (released 2026-01-14) — breaking changes + migration

Breaking changes

  1. ESM-only: ilingo no longer supports CommonJS (require(...)). Your project/build must consume it as ESM. [1]
  2. Type/interface rename: the Store interface was renamed to IStore. Update any imports/annotations/implements accordingly. [1]

Migration checklist (from 4.4.x → 5.0.0)

  • Update imports to ESM

    • ✅ ESM:
      import { Ilingo } from "ilingo";
    • ❌ CommonJS (won’t work in v5):
      const { Ilingo } = require("ilingo");
    • If your app is still CommonJS, you’ll need to migrate the app to ESM (or use an ESM-capable bundler/runtime strategy) because the package is ESM-only. [1]
  • Rename StoreIStore everywhere

    • Example:
      // before
      import type { Store } from "ilingo";
      
      // after
      import type { IStore } from "ilingo";
    • If you had custom stores implementing Store, change them to implement IStore. [1]

Notes:

  • The same release set also moved @ilingo/vue to ESM-only and renamed StoreIStore there as well. [1]

Source(s)
[1] GitHub Releases (tada5hi/ilingo) – entries for ilingo: v5.0.0 and related packages, including the “⚠ BREAKING CHANGES” section. (github.com)

Citations:


🏁 Script executed:

#!/bin/bash
# Description: Identify ilingo API usage patterns in the i18n package to assess compatibility risk

# Search for ilingo imports and usage patterns
echo "=== Ilingo imports ==="
rg -n "from ['\"]ilingo['\"]" packages/i18n/src/

echo -e "\n=== Ilingo API usage patterns ==="
# Look for common ilingo API methods that may have changed
rg -n -C3 "\b(translate|t|locale|language|setLocale|getLocale|addTranslation|loadTranslation)\(" packages/i18n/src/

echo -e "\n=== Check for any ilingo type imports ==="
rg -n "import.*ilingo.*type" packages/i18n/src/

Repository: routup/plugins

Length of output: 1397


🏁 Script executed:

#!/bin/bash
# Description: Check if migration documentation exists for this breaking change

# Check CHANGELOG for ilingo v5 migration notes
echo "=== Checking CHANGELOG ==="
fd -t f "CHANGELOG" packages/i18n/ --exec cat {}

# Check README for migration notes
echo -e "\n=== Checking README ==="
fd -t f "README" packages/i18n/ --exec cat {}

# Check for any migration guide files
echo -e "\n=== Looking for migration guides ==="
fd -t f -e md . packages/i18n/ --exec basename {}

Repository: routup/plugins

Length of output: 4165


🏁 Script executed:

# Read types.ts to see what's imported from ilingo
cat -n packages/i18n/src/types.ts

Repository: routup/plugins

Length of output: 526


🏁 Script executed:

# Search for Store type usage in the entire packages/i18n/src directory
rg -n "Store" packages/i18n/src/

Repository: routup/plugins

Length of output: 40


🏁 Script executed:

# Check the module.ts file more thoroughly to understand store usage
cat -n packages/i18n/src/module.ts | head -100

Repository: routup/plugins

Length of output: 2654


🏁 Script executed:

# List all TypeScript source files in packages/i18n/src
find packages/i18n/src -type f -name "*.ts" -o -name "*.tsx" | sort

Repository: routup/plugins

Length of output: 206


🏁 Script executed:

# Check for any imports of Store type across the entire packages/i18n directory
rg -n "import.*Store" packages/i18n/

Repository: routup/plugins

Length of output: 201


Document the ilingo v5.0.0 upgrade in the CHANGELOG.

The bump from ^3.2.0 to ^5.0.0 is a major version jump that introduces breaking changes in ilingo (ESM-only requirement and Store→IStore rename). While the @routup/i18n package source code is compatible with ilingo v5, downstream consumers should be notified about this dependency upgrade. Add a CHANGELOG entry documenting this breaking change for consumers who use ilingo v3 directly or have transitive dependencies on it.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/i18n/package.json` at line 50, Add a CHANGELOG entry documenting the
dependency bump of "ilingo" to ^5.0.0: note this is a major, breaking change
(ESM-only requirement and the API rename Store → IStore), state that
`@routup/i18n` itself is compatible but downstream consumers who depend on ilingo
v3 or use transitive v3 should be warned, and include migration guidance (update
imports to ESM and rename Store to IStore) and the package.json change
("ilingo": "^5.0.0") as the trigger for the entry.

Comment on lines +21 to +22
"test": "vitest run --config ./test/vitest.config.ts",
"test:coverage": "vitest run --config ./test/vitest.config.ts --coverage",
Copy link
Copy Markdown

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

🏁 Script executed:

#!/bin/bash
# Description: Verify the vitest config file exists and check vitest version in package files

# Check if the vitest config file exists
fd -t f "vitest.config.ts" packages/rate-limit-redis/test/

# Check vitest version in package.json files
rg -g "package*.json" '"vitest":\s*"[^"]+"' packages/rate-limit-redis/

Repository: routup/plugins

Length of output: 105


🏁 Script executed:

cat -n packages/rate-limit-redis/package.json | head -40

Repository: routup/plugins

Length of output: 1480


🏁 Script executed:

cat -n packages/rate-limit-redis/package.json | tail -30

Repository: routup/plugins

Length of output: 1081


Add vitest to devDependencies.

The test scripts reference vitest as a command, but the package is not declared in devDependencies. Add vitest with an appropriate version to the devDependencies section so the test scripts can execute.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/rate-limit-redis/package.json` around lines 21 - 22, The
package.json scripts "test" and "test:coverage" invoke the vitest CLI but vitest
is not declared in devDependencies; add "vitest" (choose a current compatible
semver, e.g. "^1.0.0" or your repo's standard version) to the devDependencies
object in packages/rate-limit-redis/package.json so the scripts can run, then
run npm/yarn/pnpm install to update lockfiles; ensure the version aligns with
other workspace packages to avoid mismatches.

@tada5hi tada5hi merged commit 285d6ce into master Apr 9, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant