Skip to content

chore: use rolldown for packaging#922

Merged
SamTV12345 merged 1 commit intomainfrom
feature/rolldown
Apr 13, 2026
Merged

chore: use rolldown for packaging#922
SamTV12345 merged 1 commit intomainfrom
feature/rolldown

Conversation

@SamTV12345
Copy link
Copy Markdown
Member

No description provided.

@qodo-free-for-open-source-projects
Copy link
Copy Markdown

Review Summary by Qodo

Migrate from Rollup to Rolldown for package bundling

✨ Enhancement

Grey Divider

Walkthroughs

Description
• Replace Rollup with Rolldown for faster bundling
• Remove Rollup plugin dependencies and configuration
• Add separate TypeScript declaration generation step
• Add GitHub Actions CI workflow for automated testing
Diagram
flowchart LR
  A["Rollup + Plugins"] -->|"Remove"| B["Old Build System"]
  C["Rolldown"] -->|"Add"| D["New Build System"]
  E["TypeScript"] -->|"Add"| F["Declaration Generation"]
  D -->|"Output"| G["dist/"]
  F -->|"Output"| G
  H["GitHub Actions"] -->|"Add"| I["CI Pipeline"]
Loading

Grey Divider

File Changes

1. rollup.config.cjs.js ⚙️ Configuration changes +0/-34

Remove Rollup configuration file

• Deleted entire Rollup configuration file
• Removed TypeScript plugin setup with include/exclude patterns
• Removed Node resolve, CommonJS, JSON, and Terser plugins

rollup.config.cjs.js


2. rolldown.config.mjs ⚙️ Configuration changes +15/-0

Add Rolldown configuration file

• Created new Rolldown configuration file
• Configured external module handling for Node library builds
• Set output format to CommonJS with named exports
• Simplified configuration compared to previous Rollup setup

rolldown.config.mjs


3. package.json Dependencies +4/-7

Update dependencies and build scripts

• Removed Rollup and all Rollup plugin dependencies
• Removed rollup-plugin-typescript2 dependency
• Added Rolldown v1.0.0-rc.15 as new bundler
• Updated build script to use Rolldown and separate TypeScript compilation
• Split build into two steps: JavaScript bundling and type declaration generation

package.json


View more (3)
4. pnpm-lock.yaml Dependencies +227/-323

Update lock file with Rolldown dependencies

• Removed Rollup and all associated plugin dependencies
• Removed rollup-plugin-typescript2 and its dependencies
• Added Rolldown and platform-specific bindings for multiple architectures
• Added new dependencies for Rolldown ecosystem (@emnapi, @napi-rs, @oxc-project)
• Cleaned up transitive dependencies no longer needed

pnpm-lock.yaml


5. tsconfig.json ⚙️ Configuration changes +1/-1

Update module resolution strategy

• Changed moduleResolution from "node" to "bundler"
• Aligns with modern bundler expectations for module resolution

tsconfig.json


6. .github/workflows/ci.yml ⚙️ Configuration changes +37/-0

Add GitHub Actions CI workflow

• Created new GitHub Actions CI workflow
• Runs on push to main/master and on pull requests
• Installs pnpm v10.33.0 and Node.js v22.22.0
• Executes lint, type check, and build steps
• Uses frozen lockfile for reproducible builds

.github/workflows/ci.yml


Grey Divider

Qodo Logo

@qodo-free-for-open-source-projects
Copy link
Copy Markdown

qodo-free-for-open-source-projects Bot commented Apr 13, 2026

Code Review by Qodo

🐞 Bugs (2)   📘 Rule violations (0)   📎 Requirement gaps (0)
🐞\ ≡ Correctness (1) ⚙ Maintainability (1)

Grey Divider


Action required

1. Missing runtime dependencies 🐞
Description
rolldown.config.mjs externalizes every non-relative import, so the built dist/index.js will keep
require('mysql2'), require('pg'), etc. Because package.json declares these runtime driver
packages only in devDependencies (and has no dependencies/optionalDependencies), consumers can get
MODULE_NOT_FOUND just by importing ueberdb2.
Code

rolldown.config.mjs[R5-13]

+  input: ['./index.ts'],
+  // Keep third-party modules external for a Node library build.
+  external: (id) => !id.startsWith('.') && !path.isAbsolute(id),
+  output: {
+    preserveModules: false,
+    dir: './dist',
+    format: 'cjs',
+    exports: 'named',
+  },
Evidence
The entrypoint eagerly imports all database implementations, and at least one of those
implementations imports a third-party driver at module top-level. With the new rolldown external
function, those driver imports are not bundled and therefore must be installed by consumers, but
package.json does not list them in dependencies/optionalDependencies.

rolldown.config.mjs[4-13]
index.ts[20-40]
databases/mysql_db.ts[17-21]
package.json[2-56]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`rolldown.config.mjs` externalizes all third-party imports, which makes the output bundle rely on runtime-installed DB driver packages (e.g., `mysql2`, `pg`, `mssql`, etc.). The package does not declare these as `dependencies`/`optionalDependencies`, so consumers may hit `MODULE_NOT_FOUND` on import.

### Issue Context
- `index.ts` imports all DB implementations.
- DB implementation modules import their driver packages at module scope.
- `rolldown` is configured to keep all non-relative imports external.

### Fix Focus Areas
- rolldown.config.mjs[4-13]
- index.ts[20-40]
- databases/mysql_db.ts[17-21]
- package.json[25-56]

### What to change
Choose one consistent strategy:
1) **Bundle runtime deps (closest to prior rollup-bundle behavior):** remove/relax the `external` function so DB drivers are bundled into `dist/index.js`.
  - If you still want externals, externalize **only Node built-ins** (e.g., `node:*`, `fs`, `path`, etc.), not npm packages.

2) **Keep externals but make installs correct:**
  - Move runtime-needed drivers from `devDependencies` to `dependencies` (or `optionalDependencies`/`peerDependencies`), and
  - Refactor to lazy-load DB drivers/implementations (so users don’t need every driver installed just to import the package).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

2. Unpublished d.ts map files 🐞
Description
build:types runs tsc --emitDeclarationOnly while tsconfig.json has declarationMap: true, but
package.json files does not include top-level dist/*.d.ts.map. This can publish
dist/index.d.ts with a sourceMappingURL to a missing index.d.ts.map.
Code

package.json[R78-80]

+    "build": "pnpm run build:js && pnpm run build:types",
+    "build:js": "pnpm exec rolldown -c rolldown.config.mjs",
+    "build:types": "pnpm exec tsc --emitDeclarationOnly",
Evidence
The PR adds a dedicated build:types step that uses tsc, and the tsconfig enables declaration
maps. The npm publish whitelist includes dist/*.d.ts but not dist/*.d.ts.map, so the top-level
declaration map is likely excluded from the published package.

package.json[66-84]
tsconfig.json[48-51]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The build now emits declaration files via `tsc --emitDeclarationOnly`, and `declarationMap` is enabled. However, the package `files` allowlist does not include top-level `*.d.ts.map` files, which can leave dangling sourceMappingURL references in published `.d.ts` files.

### Issue Context
- `tsconfig.json` has `"declarationMap": true`.
- `build:types` runs `tsc --emitDeclarationOnly`.
- `package.json#files` includes `dist/*.d.ts` but not `dist/*.d.ts.map`.

### Fix Focus Areas
- package.json[66-71]
- package.json[78-80]
- tsconfig.json[48-51]

### What to change
Pick one:
1) Add `dist/*.d.ts.map` (and optionally `dist/**/*.d.ts.map`) to `package.json.files`, **or**
2) Disable `declarationMap` for published builds (e.g., set it to false in `tsconfig.json` or use a separate `tsconfig.build-types.json`).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

ⓘ The new review experience is currently in Beta. Learn more

Grey Divider

Qodo Logo

@SamTV12345 SamTV12345 merged commit d1d24d5 into main Apr 13, 2026
12 checks passed
@SamTV12345 SamTV12345 deleted the feature/rolldown branch April 13, 2026 18:05
Comment thread rolldown.config.mjs
Comment on lines +5 to +13
input: ['./index.ts'],
// Keep third-party modules external for a Node library build.
external: (id) => !id.startsWith('.') && !path.isAbsolute(id),
output: {
preserveModules: false,
dir: './dist',
format: 'cjs',
exports: 'named',
},
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Action required

1. Missing runtime dependencies 🐞 Bug ≡ Correctness

rolldown.config.mjs externalizes every non-relative import, so the built dist/index.js will keep
require('mysql2'), require('pg'), etc. Because package.json declares these runtime driver
packages only in devDependencies (and has no dependencies/optionalDependencies), consumers can get
MODULE_NOT_FOUND just by importing ueberdb2.
Agent Prompt
### Issue description
`rolldown.config.mjs` externalizes all third-party imports, which makes the output bundle rely on runtime-installed DB driver packages (e.g., `mysql2`, `pg`, `mssql`, etc.). The package does not declare these as `dependencies`/`optionalDependencies`, so consumers may hit `MODULE_NOT_FOUND` on import.

### Issue Context
- `index.ts` imports all DB implementations.
- DB implementation modules import their driver packages at module scope.
- `rolldown` is configured to keep all non-relative imports external.

### Fix Focus Areas
- rolldown.config.mjs[4-13]
- index.ts[20-40]
- databases/mysql_db.ts[17-21]
- package.json[25-56]

### What to change
Choose one consistent strategy:
1) **Bundle runtime deps (closest to prior rollup-bundle behavior):** remove/relax the `external` function so DB drivers are bundled into `dist/index.js`.
   - If you still want externals, externalize **only Node built-ins** (e.g., `node:*`, `fs`, `path`, etc.), not npm packages.

2) **Keep externals but make installs correct:**
   - Move runtime-needed drivers from `devDependencies` to `dependencies` (or `optionalDependencies`/`peerDependencies`), and
   - Refactor to lazy-load DB drivers/implementations (so users don’t need every driver installed just to import the package).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

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