Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThis PR migrates the project's build tooling from Rollup to tsdown. The rollup.config.mjs file is removed and replaced with a new tsdown.config.mjs configuration that defines dual-entry builds (index and extract) with both ESM and CJS output formats. The package.json exports map is restructured to use nested import/require objects with separate types and default entries. Dependencies are updated to remove Rollup-related plugins and add tsdown. Additionally, a utility function fix corrects the exponent operator in src/utils/round.ts, the src/types.ts module adds a re-exported OutputBundle type alias from Rollup, and the test file is updated to import this type from the local types module. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 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 |
vio
force-pushed
the
tsdown
branch
3 times, most recently
from
010440e to
51e6295
Compare
![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
🧹 Nitpick comments (2)
tsdown.config.mjs (1)
12-12: Consider disabling minification for library code.Minifying library code can make debugging more difficult for consumers, as it obfuscates stack traces and makes source inspection harder even with source maps. Libraries typically ship unminified code and let consuming applications decide whether to minify.
Apply this diff to disable minification:
format: 'esm', hash: false, - minify: true, + minify: false, sourcemap: true, }, { outDir: 'dist/cjs', entry: { index: './src/index.ts', extract: './src/extract.ts', }, format: 'cjs', hash: false, - minify: true, + minify: false, sourcemap: true, },Also applies to: 23-23
package.json (1)
77-77: Confirm tsdown ^0.15.12 dependency management strategy.The latest stable release is v0.15.12 (released Oct 29, 2025), and the project is under active development with frequent releases and breaking changes; users should expect API/config migrations and occasional regressions. The original concern is valid—the community recommends testing upgrades in CI or a branch before adopting in production.
Consider implementing one of the suggested approaches:
- Pin to a specific version (e.g.,
"tsdown": "0.15.12") to avoid unexpected breaking changes during active development, or- Establish a monitoring and testing process in your CI/CD pipeline before upgrading to new tsdown releases.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (4)
package.json(3 hunks)rollup.config.mjs(0 hunks)tsconfig.json(0 hunks)tsdown.config.mjs(1 hunks)
💤 Files with no reviewable changes (2)
- rollup.config.mjs
- tsconfig.json
⏰ 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). (1)
- GitHub Check: Socket Security: Pull Request Alerts
🔇 Additional comments (2)
package.json (1)
4-4: Beta version is appropriate for this migration.The version bump to 1.5.3-beta.0 correctly signals that this is a pre-release version with significant build tooling changes, allowing early testing before a stable release.
tsdown.config.mjs (1)
1-26: Now let me verify tsdown's behavior regarding automatic type declaration generation:No action required—tsdown automatically generates type declarations.
Tsdown automatically enables declaration file generation when package.json contains a types or typings field. The package.json already specifies both
"typings": "dist/esm/index.d.ts"and"types"entries in exports, so declaration file generation will be enabled by default. With TypeScript (5.9.3) installed as a devDependency, the configuration is complete and requires no changes. The absence of an explicitdtsoption or tsconfig.json is not a problem.Likely an incorrect or invalid review comment.
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
vio
force-pushed
the
tsdown
branch
2 times, most recently
from
2672215 to
c15f463
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (4)
package.jsonrollup.config.mjstsconfig.jsontsdown.config.mjs
💤 Files with no reviewable changes (1)
- rollup.config.mjs
⏰ 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). (1)
- GitHub Check: tests
🔇 Additional comments (4)
tsdown.config.mjs (1)
1-14: LGTM! Clean tsdown configuration for library builds.The configuration is well-structured with appropriate settings for a library package:
- Dual ESM/CJS output matches modern package distribution best practices
unbundle: trueensures all dependencies remain externalsourcemap: trueaids debugging whileminify: falsekeeps output readable- Entry points align with the package.json exports structure
tsconfig.json (1)
3-3: Good alignment with modern Node.js module resolution.The
rootDir: "."andmodule: "nodenext"settings properly support the new tsdown-based build workflow and dual ESM/CJS output strategy.Also applies to: 13-13
package.json (2)
27-29: Appropriate top-level entry points for dual-package support.The
main,module, andtypesfields correctly establish fallback entry points for older tooling. The exports map provides more granular control for modern resolvers.
59-59: Well-structured build and testing scripts.The migration to tsdown is clean, and the additional test:package scripts ensure compatibility across multiple bundlers (rolldown, rollup, vite). The bump/release scripts support the release workflow.
Also applies to: 63-68
There was a problem hiding this comment.
Actionable comments posted: 2
Fix all issues with AI Agents 🤖
In @src/utils/round.ts:
- Around line 1-4: The round function uses the bitwise XOR operator (10 ^
precision) instead of exponentiation, producing wrong multipliers; change the
multiplier calculation in round to use exponentiation (e.g., Math.pow(10,
precision) or 10 ** precision) and ensure precision is treated as an integer
(cast/validate if needed) so Math.round(value * multiplier) / multiplier returns
correct results.
In @test/unit/extract.test.ts:
- Around line 9-25: The tests currently use "as any" on rollupStats.stats which
triggers ESLint; instead give the fixture and tests proper types: declare and
export rollupStats.stats with a concrete type (e.g., export const stats:
RollupStats = ...) in the fixture (create or import a RollupStats/Stats
interface from your src types) and then update test calls from
extract(deepFreeze(rollupStats.stats as any)) to
extract(deepFreeze(rollupStats.stats)) (or if unavoidable, use rollupStats.stats
as unknown as RollupStats). Update any imports so extract, deepFreeze and
rollupStats.stats all operate on the correct typed value.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (7)
package.jsonrollup.config.mjssrc/index.tssrc/utils/round.tstest/unit/extract.test.tstsconfig.jsontsdown.config.mjs
💤 Files with no reviewable changes (2)
- src/index.ts
- rollup.config.mjs
🧰 Additional context used
🪛 GitHub Actions: ci
test/unit/extract.test.ts
[error] 9-9: ESLint: Unexpected any. Specify a different type (@typescript-eslint/no-explicit-any)
🪛 GitHub Check: lint
test/unit/extract.test.ts
[failure] 25-25:
Unexpected any. Specify a different type
[failure] 21-21:
Unexpected any. Specify a different type
[failure] 17-17:
Unexpected any. Specify a different type
[failure] 13-13:
Unexpected any. Specify a different type
[failure] 9-9:
Unexpected any. Specify a different type
🔇 Additional comments (5)
tsdown.config.mjs (1)
1-14: LGTM!The tsdown configuration is well-structured with appropriate settings for a library: dual format output (ESM/CJS), type declarations enabled, and sourcemaps for debugging. The entry points correctly match the package.json exports.
package.json (3)
33-53: Exports structure looks correct.The nested exports configuration properly handles both ESM and CJS consumers with their respective type declarations. The structure follows the recommended Node.js conditional exports pattern.
59-67: Good build workflow pattern.Running
type-checkbeforetsdownensures type errors are caught early, preventing builds with type issues. This is a solid improvement over the previous Rollup-based workflow.
88-88: tsdown version 0.18.4 is available on npm.Version 0.18.4 exists and is a valid, published release.
tsconfig.json (1)
23-27: Declaration settings are correctly configured for the tsdown workflow.The
declaration,isolatedDeclarations, anddeclarationMapoptions alongsidenoEmit: truework as intended. TypeScript validates types and declaration requirements during thetype-checkscript (which runs before the build), while tsdown withdts: truegenerates the actual.d.ts,.d.mts, and.d.ctsfiles todist/.
There was a problem hiding this comment.
Actionable comments posted: 1
Fix all issues with AI Agents 🤖
In @test/unit/extract.test.ts:
- Line 1: Replace the failing direct type assertions in
test/unit/extract.test.ts by changing each "as OutputBundle" cast on the fixture
objects to the double-cast pattern "as unknown as OutputBundle" (apply this to
all five test cases where fixtures are asserted to OutputBundle) so TypeScript
accepts the intentional coercion.
🧹 Nitpick comments (1)
package.json (1)
1-109: Consider adding"type": "module"field for clarity.A previous review suggested adding a
"type": "module"field to ensure consistent module resolution. While your explicit.mjs/.cjsextensions in the exports should work, adding this field provides clarity about the package's primary module format and can help avoid edge cases with tools that rely on this field.🔎 Suggested addition
Add near the top of package.json (e.g., after "license"):
"license": "MIT", + "type": "module", "private": false,
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (6)
package.jsonrollup.config.mjssrc/utils/round.tstest/unit/extract.test.tstsconfig.jsontsdown.config.mjs
💤 Files with no reviewable changes (1)
- rollup.config.mjs
🚧 Files skipped from review as they are similar to previous changes (2)
- tsconfig.json
- tsdown.config.mjs
🧰 Additional context used
🪛 GitHub Actions: ci
test/unit/extract.test.ts
[error] 10-10: TS2352: Conversion of type '{ 'assets/logo.abcd1234.svg': { fileName: string; type: string; source: string; }; 'assets/main.abcd1234.js': { fileName: string; type: string; code: string; modules: { '/home/user/project/src/main.js': { ...; }; '/home/user/project/src/utils.js': { ...; }; }; map: { ...; }; }; 'assets/vendors.abcd1234.js': { ...; }...' to type 'OutputBundle' may be a mistake because neither type sufficiently overlaps with the other. If this was intentional, convert the expression to 'unknown' first.
🪛 GitHub Check: build
test/unit/extract.test.ts
[failure] 26-26:
Conversion of type '{ 'assets/logo.abcd1234.svg': { fileName: string; type: string; source: string; }; 'assets/main.abcd1234.js': { fileName: string; type: string; code: string; modules: { '/home/user/project/src/main.js': { ...; }; '/home/user/project/src/utils.js': { ...; }; }; map: { ...; }; }; 'assets/vendors.abcd1234.js': { ...; }...' to type 'OutputBundle' may be a mistake because neither type sufficiently overlaps with the other. If this was intentional, convert the expression to 'unknown' first.
[failure] 22-22:
Conversion of type '{ 'assets/logo.abcd1234.svg': { fileName: string; type: string; source: string; }; 'assets/main.abcd1234.js': { fileName: string; type: string; code: string; modules: { '/home/user/project/src/main.js': { ...; }; '/home/user/project/src/utils.js': { ...; }; }; map: { ...; }; }; 'assets/vendors.abcd1234.js': { ...; }...' to type 'OutputBundle' may be a mistake because neither type sufficiently overlaps with the other. If this was intentional, convert the expression to 'unknown' first.
[failure] 18-18:
Conversion of type '{ 'assets/logo.abcd1234.svg': { fileName: string; type: string; source: string; }; 'assets/main.abcd1234.js': { fileName: string; type: string; code: string; modules: { '/home/user/project/src/main.js': { ...; }; '/home/user/project/src/utils.js': { ...; }; }; map: { ...; }; }; 'assets/vendors.abcd1234.js': { ...; }...' to type 'OutputBundle' may be a mistake because neither type sufficiently overlaps with the other. If this was intentional, convert the expression to 'unknown' first.
[failure] 14-14:
Conversion of type '{ 'assets/logo.abcd1234.svg': { fileName: string; type: string; source: string; }; 'assets/main.abcd1234.js': { fileName: string; type: string; code: string; modules: { '/home/user/project/src/main.js': { ...; }; '/home/user/project/src/utils.js': { ...; }; }; map: { ...; }; }; 'assets/vendors.abcd1234.js': { ...; }...' to type 'OutputBundle' may be a mistake because neither type sufficiently overlaps with the other. If this was intentional, convert the expression to 'unknown' first.
[failure] 10-10:
Conversion of type '{ 'assets/logo.abcd1234.svg': { fileName: string; type: string; source: string; }; 'assets/main.abcd1234.js': { fileName: string; type: string; code: string; modules: { '/home/user/project/src/main.js': { ...; }; '/home/user/project/src/utils.js': { ...; }; }; map: { ...; }; }; 'assets/vendors.abcd1234.js': { ...; }...' to type 'OutputBundle' may be a mistake because neither type sufficiently overlaps with the other. If this was intentional, convert the expression to 'unknown' first.
🔇 Additional comments (4)
src/utils/round.ts (1)
1-2: LGTM! Excellent fixes for type safety and correctness.The explicit return type annotation improves type safety, and the exponentiation operator fix (
**instead of^) corrects the critical calculation bug.package.json (3)
29-29: Good: Modernized types field with correct extension.The change from "typings" to "types" follows current best practices, and the
.d.ctsextension correctly indicates CommonJS declaration files for dual-module packages.
35-52: LGTM! Exports properly structured for dual-module package.The conditional exports correctly separate ESM (
.mjs/.d.mts) and CJS (.cjs/.d.cts) entry points with appropriate type declarations for each condition.
59-59: Good addition: type-checking before build.Adding a separate type-check step before tsdown ensures type errors are caught early in the build process.
Also applies to: 67-67
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI Agents
In @package.json:
- Line 88: The package.json entry for the "tsdown" dependency is pointing to a
non-existent version (0.18.4); update the version string to the existing npm
release 0.14.2 by replacing "tsdown": "0.18.4" with "tsdown": "0.14.2" in
package.json so npm install succeeds.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (1)
package.json
🔇 Additional comments (5)
package.json (5)
4-4: LGTM: Version bump to beta is appropriate.The minor version bump with beta tag is suitable for the build tooling migration, allowing for testing before stable release.
29-29: LGTM: CJS-first types configuration is correct.Using the standard
typesfield pointing to.d.ctsis a valid approach for dual packages. ESM consumers will correctly resolve.d.mtsthrough the exports map.
33-53: LGTM: Exports structure follows modern dual-package best practices.The nested conditional exports with separate type declarations for ESM (
.d.mts) and CJS (.d.cts) correctly implements the dual-package pattern. The previous issue with the "./extract" export has been resolved.
59-59: LGTM: Build script migration to tsdown is clean.The new build workflow with type-checking before compilation is a solid approach. Running
tscfirst ensures type errors are caught before tsdown generates output.Also applies to: 67-67
79-79: LGTM: Type definitions added for existing dependency.Adding
@types/deep-freeze-strictprovides TypeScript support for thedeep-freeze-strictpackage already in use.
package.json
Outdated
| "prettier": "3.7.4", | ||
| "release-it": "19.2.2", | ||
| "rollup": "4.54.0", | ||
| "tsdown": "0.18.4", |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
tsdown npm package latest version
💡 Result:
Latest npm release: tsdown v0.14.2 (published 13 days ago). (npmjs.com)
Citations:
Update tsdown version from 0.18.4 to 0.14.2.
tsdown version 0.18.4 does not exist on npm; the latest available version is 0.14.2. This will cause npm install to fail.
🤖 Prompt for AI Agents
In @package.json at line 88, The package.json entry for the "tsdown" dependency
is pointing to a non-existent version (0.18.4); update the version string to the
existing npm release 0.14.2 by replacing "tsdown": "0.18.4" with "tsdown":
"0.14.2" in package.json so npm install succeeds.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
package.json (2)
28-30:⚠️ Potential issue | 🔴 CriticalLegacy fields (
main,module,typings) reference stale paths.The
exportsmap points to./dist/index.cjs,./dist/index.mjs,./dist/index.d.mts, etc., but these legacy fields still reference./dist/cjs/index.js,./dist/esm/index.js, and./dist/types/index.d.ts. Consumers on older Node.js versions (or tools that fall back tomain/module) will get module-not-found errors.Proposed fix
- "main": "./dist/cjs/index.js", - "module": "./dist/esm/index.js", - "typings": "./dist/types/index.d.ts", + "main": "./dist/index.cjs", + "module": "./dist/index.mjs", + "typings": "./dist/index.d.cts",
1-6:⚠️ Potential issue | 🟡 MinorAdd
"type": "module"field to package.jsonThe
"type": "module"field is missing from the root of package.json. Since the package defines ESM exports with.mjsfiles, this field is needed for Node.js to properly recognize the module type. Add it after the"private"field:"private": false, "type": "module",
🤖 Fix all issues with AI agents
In `@package.json`:
- Line 92: The package.json contains an invalid dependency entry "tsdown":
"0.20.3"; update the package.json to use a published version (e.g., "tsdown":
"latest" or the exact version returned by running npm view tsdown versions
--json such as 0.14.2 or a valid 0.16/0.17.x release) so the dependency resolves
correctly and reinstall/update lockfile.
| "release-it": "19.2.4", | ||
| "rimraf": "6.0.1", | ||
| "rollup": "4.57.1", | ||
| "tsdown": "0.20.3", |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
tsdown npm package version 0.20.3
💡 Result:
tsdown@0.20.3 does not appear to be a version you can reliably install from the npm registry.
- The npm package page for
tsdownshows the package at 0.14.2 (at the time it was crawled). [2] - A third-party npm-version index (Socket) shows recent versions around 0.16.x–0.17.0, with no indication of
0.20.x. [3] - While jsDelivr’s landing page displays “Version 0.20.3”, that conflicts with the above registry-derived version listings. [1]
What to do:
- Install the latest published version:
npm i -D tsdown@latest - Or verify what your npm sees:
npm view tsdown versionandnpm view tsdown versions --json(then check whether0.20.3is present)
Sources: jsDelivr package page [1], npm package page [2], Socket version listing snippet [3].
Remove or replace tsdown@0.20.3—this version does not exist on npm. The npm registry shows tsdown at 0.14.2; third-party version indexes list only 0.16.x–0.17.0. Use tsdown@latest or verify the correct published version with npm view tsdown versions --json.
🤖 Prompt for AI Agents
In `@package.json` at line 92, The package.json contains an invalid dependency
entry "tsdown": "0.20.3"; update the package.json to use a published version
(e.g., "tsdown": "latest" or the exact version returned by running npm view
tsdown versions --json such as 0.14.2 or a valid 0.16/0.17.x release) so the
dependency resolves correctly and reinstall/update lockfile.
1 participant
Summary by CodeRabbit
Bug Fixes
Chores
rel: relative-ci/roadmap#70