feat: make library tree-shakeable with per-language entry points#91
feat: make library tree-shakeable with per-language entry points#91
Conversation
- Add per-language entry points in tsup config - Add new exports for lang/* subpaths in package.json - Each language is now a separate bundle (~1-4KB vs 10.4KB for all) Benchmark improvement: - Main API: 1.56x faster than number-to-words (was 1.09x slower) - Direct import: 3.1x faster than number-to-words Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAdds per-language build/export support and project metadata, updates tsup to emit language-specific bundles (no splitting, bundled outputs), introduces a benchmark package and Bun-focused docs, and makes small lint-related no-op edits to individual language modules and workspace manifests. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
|
LGTM! The changes look good:
The Vercel deployment failure appears to be unrelated to this PR (likely a preview environment issue). |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
library/package.json (1)
23-36:⚠️ Potential issue | 🔴 CriticalCommonJS paths incorrectly point to
.jsinstead of.cjsfiles.With
"type": "module", Node.js treats.jsfiles as ES modules. Since tsup is configured withformat: ['esm', 'cjs'], it generates.cjsfiles for CommonJS output. Therequirepaths must reference.cjs:
- Line 25:
"./dist/index.js"→"./dist/index.cjs"- Line 35:
"./dist/lang/*.js"→"./dist/lang/*.cjs"CommonJS consumers (e.g.,
const { numInWords } = require('i18n-num-in-words')) will fail without this fix, as they would attempt to load ES modules as CommonJS.🐛 Proposed fix
"require": { "types": "./dist/index.d.cts", - "default": "./dist/index.js" + "default": "./dist/index.cjs" } }, "./lang/*": { "import": { "types": "./dist/lang/*.d.ts", "default": "./dist/lang/*.js" }, "require": { "types": "./dist/lang/*.d.cts", - "default": "./dist/lang/*.js" + "default": "./dist/lang/*.cjs" }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@library/package.json` around lines 23 - 36, Update the CommonJS "require" defaults in package.json so they point to the generated .cjs outputs: change the "require" entry under the package root from "default": "./dist/index.js" to "default": "./dist/index.cjs" and change the "require" entry inside the "./lang/*" export from "default": "./dist/lang/*.js" to "default": "./dist/lang/*.cjs"; keep the "types" fields as-is and ensure these are the only edits so CommonJS consumers load the .cjs bundles produced by tsup.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@library/package.json`:
- Around line 23-36: Update the CommonJS "require" defaults in package.json so
they point to the generated .cjs outputs: change the "require" entry under the
package root from "default": "./dist/index.js" to "default": "./dist/index.cjs"
and change the "require" entry inside the "./lang/*" export from "default":
"./dist/lang/*.js" to "default": "./dist/lang/*.cjs"; keep the "types" fields
as-is and ensure these are the only edits so CommonJS consumers load the .cjs
bundles produced by tsup.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
library/src/lang/ur.ts (1)
3-5: Consider using_numprefix for consistency across language files.This file uses
void num;to silence the unused parameter warning, whilebn.tsandfr.tsuse the_numprefix convention. Both approaches work, but the underscore prefix is more idiomatic in TypeScript and makes the intent clearer at the function signature level.💡 Optional: Use underscore prefix for unused parameter
-const ur = (num: number): string => { - void num; return 'Not implemented yet!'; +const ur = (_num: number): string => { + return 'Not implemented yet!'; };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@library/src/lang/ur.ts` around lines 3 - 5, Rename the unused parameter in the ur function from num to _num and remove the void num; statement so the signature itself documents the unused parameter; update the function declaration for ur(num: number): string to use _num: number and return the same placeholder string (leave behavior unchanged).benchmark/index.ts (1)
4-7: Guard globalconsole.debugrestoration withtry/finally.If any error occurs before Line 77,
console.debugremains overridden for the process.Proposed fix
const originalDebug = console.debug; -console.debug = () => {}; +console.debug = () => {}; -const ITERATIONS = 100000; +try { + const ITERATIONS = 100000; -// Test data - various number ranges -const testCases = [ + // Test data - various number ranges + const testCases = [ 0, 1, 5, 10, 15, 21, 99, 100, 101, 999, 1000, 1001, 9999, 10000, 100000, 1000000, 10000000, 100000000, 1000000000, 1000000000000, -]; + ]; -// ... existing benchmark logic ... + // ... existing benchmark logic ... -// Restore console.debug -console.debug = originalDebug; +} finally { + // Restore console.debug + console.debug = originalDebug; +}Also applies to: 76-77
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@benchmark/index.ts` around lines 4 - 7, The global console.debug override (originalDebug and console.debug = () => {}) must be guarded so it always gets restored: wrap the benchmark execution block that runs after saving originalDebug in a try/finally, running the existing restore (setting console.debug = originalDebug) in the finally clause so any thrown error before the current restore point (around the existing restore at lines referencing originalDebug/console.debug) will still reset console.debug; locate the override by the symbols originalDebug and console.debug and move the restore into a finally block that encloses the code that may throw.package.json (1)
28-30: Pinsyncpack@alphaandsherif@latestto specific versions for reproducible workspace checks.Floating version channels (
@alphaand@latest) can drift over time, causingformat:wsandlint:wsbehavior to change unexpectedly. Both commands run during postinstall, affecting entire workspace reproducibility. Use pinned versions (e.g.,syncpack@0.14.0andsherif@2.1.0) instead.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@package.json` around lines 28 - 30, The package.json scripts "format:ws" and "lint:ws" use floating channels (`syncpack@alpha` and `sherif@latest`) which can change over time; update those script entries to pin to specific versions (e.g., replace `syncpack@alpha` with a chosen release like `syncpack@0.14.0` and `sherif@latest` with `sherif@2.1.0`) so workspace formatting and linting are reproducible during postinstall; ensure the chosen versions are compatible with the workspace and update any lockfile or install steps accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@benchmark/index.ts`:
- Around line 55-65: The "Other Languages" benchmark is only exercising error
handling because numInWords (library/src/num-in-words.ts) implements only 'en'
and 'id' and all other entries fall into handleError; update the benchmark in
benchmark/index.ts so it measures real conversion work: either replace the
languages array with only implemented languages (e.g., ['en','id']) or add real
implementations for 'pt','ru','zh','ar','es' in numInWords (or another
conversion function) before including them; locate the languages declaration and
the loop that calls numInWords and ensure unsupported languages are removed or
supported implementations are added so the benchmark measures conversion
performance rather than suppressed errors.
In `@package.json`:
- Line 31: The postinstall npm script currently uses a semicolon which allows
later commands to run even if earlier ones fail; update the "postinstall" script
to use && so it becomes "postinstall": "bun run format:ws && bun run lint:ws"
(ensuring the format:ws command failure stops execution before lint:ws runs).
Locate the "postinstall" entry in package.json and replace the separator between
format:ws and lint:ws from ";" to "&&".
---
Nitpick comments:
In `@benchmark/index.ts`:
- Around line 4-7: The global console.debug override (originalDebug and
console.debug = () => {}) must be guarded so it always gets restored: wrap the
benchmark execution block that runs after saving originalDebug in a try/finally,
running the existing restore (setting console.debug = originalDebug) in the
finally clause so any thrown error before the current restore point (around the
existing restore at lines referencing originalDebug/console.debug) will still
reset console.debug; locate the override by the symbols originalDebug and
console.debug and move the restore into a finally block that encloses the code
that may throw.
In `@library/src/lang/ur.ts`:
- Around line 3-5: Rename the unused parameter in the ur function from num to
_num and remove the void num; statement so the signature itself documents the
unused parameter; update the function declaration for ur(num: number): string to
use _num: number and return the same placeholder string (leave behavior
unchanged).
In `@package.json`:
- Around line 28-30: The package.json scripts "format:ws" and "lint:ws" use
floating channels (`syncpack@alpha` and `sherif@latest`) which can change over
time; update those script entries to pin to specific versions (e.g., replace
`syncpack@alpha` with a chosen release like `syncpack@0.14.0` and
`sherif@latest` with `sherif@2.1.0`) so workspace formatting and linting are
reproducible during postinstall; ensure the chosen versions are compatible with
the workspace and update any lockfile or install steps accordingly.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
benchmark/bun.lockis excluded by!**/*.lockbun.lockis excluded by!**/*.lockbun.lockbis excluded by!**/bun.lockb
📒 Files selected for processing (18)
.gitignorebenchmark/.gitignorebenchmark/CLAUDE.mdbenchmark/README.mdbenchmark/index.tsbenchmark/package.jsonbenchmark/tsconfig.jsonlibrary/src/lang/bn.tslibrary/src/lang/de.tslibrary/src/lang/es.tslibrary/src/lang/fr.tslibrary/src/lang/hi.tslibrary/src/lang/ja.tslibrary/src/lang/mr.tslibrary/src/lang/sw.tslibrary/src/lang/ur.tspackage.jsonwebsite/package.json
✅ Files skipped from review due to trivial changes (5)
- benchmark/README.md
- benchmark/package.json
- benchmark/.gitignore
- benchmark/CLAUDE.md
- .gitignore
- Fix require paths to use .cjs instead of .js - Fix ur.ts unused parameter warning Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
- Changed languages array to only include 'en' and 'id' which are stable and supported by numInWords() - Removed unnecessary try-catch since both languages work Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
1 participant
Summary
This PR makes the library tree-shakeable by adding per-language entry points.
Changes
lang/*subpathsResults
Bundle Size
Users can now import only the languages they need:
Performance Benchmark
Test Plan
Related Issues
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Chores