build: switch from tsup to tsdown#1515
Conversation
🦋 Changeset detectedLatest commit: 437a90d The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
PR SummaryMedium Risk Overview Published layout is meant to stay the same via Reviewed by Cursor Bugbot for commit 437a90d. Bugbot is set up for automated code reviews on this repo. Configure here. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f9a01cb728
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| "json2md": "^2.0.1", | ||
| "knip": "^5.43.6", | ||
| "tsup": "^8.4.0", | ||
| "tsdown": "^0.22.3", |
There was a problem hiding this comment.
The repo is still pinned to Node 20.19.5 in .tool-versions/the codegen Dockerfile, but the new lockfile entry for tsdown@0.22.3 declares engines.node: ^22.18.0 || >=24.11.0. Any local, CI, or publish path that uses the pinned Node 20 toolchain and runs this new pnpm build/prepublishOnly script is now invoking an unsupported build tool, which will fail with engine-strict installs or when tsdown uses Node 22-only APIs; either bump the build Node version everywhere or choose a tsdown version that supports Node 20.
Useful? React with 👍 / 👎.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| "json2md": "^2.0.1", | ||
| "knip": "^5.43.6", | ||
| "tsup": "^8.4.0", | ||
| "tsdown": "^0.22.3", |
There was a problem hiding this comment.
🔴 tsdown@0.22.3 (and several transitive deps like rolldown-plugin-dts@0.26.0, @babel/parser@8.0.0, ast-kit@3.0.0) declare engines.node: ^22.18.0 || >=24.11.0, but .tool-versions pins nodejs 20.19.5 and .npmrc sets engine-strict=true. Every CI workflow (lint, typecheck, cli_tests, js_sdk_tests, pkg_artifacts, publish_packages, release, etc.) runs pnpm install --frozen-lockfile on Node 20 via TOOL_VERSION_NODEJS, so install will hard-fail with EBADENGINE and block the entire pipeline. Fix by bumping .tool-versions (and the engines.node fields in both packages/cli/package.json and packages/js-sdk/package.json) to >=22.18.0, or pin to a tsdown version that still supports Node 20.
Extended reasoning...
What the bug is
This PR switches the build tooling from tsup to tsdown@^0.22.3. The resolved version tsdown@0.22.3 declares a Node engine constraint of ^22.18.0 || >=24.11.0 (visible in pnpm-lock.yaml). Several of its transitive dependencies pulled in by this PR carry the same constraint: rolldown-plugin-dts@0.26.0, ast-kit@3.0.0, @babel/generator@8.0.0, @babel/parser@8.0.0, @babel/types@8.0.0, @babel/helper-string-parser@8.0.0, and @babel/helper-validator-identifier@8.0.2.
Why this breaks CI
Three project-level facts combine to make this fatal rather than a warning:
.tool-versionspinsnodejs 20.19.5..npmrccontainsengine-strict=true, which makes pnpm refuse to install packages whose engines do not match the running Node, instead of merely warning.- Every Node-using CI workflow (
lint.yml,typecheck.yml,cli_tests.yml,js_sdk_tests.yml,pkg_artifacts.yml,publish_packages.yml,publish_candidates.yml,release.yml,generated_files.yml, and.github/actions/build-cli/action.yml) reads the Node version from.tool-versionsviawistia/parse-tool-versionsand setsnode-version: '${{ env.TOOL_VERSION_NODEJS }}'— i.e. they all install on Node 20.19.5.
With engine-strict=true + Node 20 + a lockfile that includes a package requiring Node ^22.18.0 || >=24.11.0, pnpm install --frozen-lockfile exits non-zero with ERR_PNPM_BAD_ENGINE / EBADENGINE. This happens before any build/test/publish step runs.
Why existing code doesn't prevent it
The engines.node fields in packages/cli/package.json (>=20) and packages/js-sdk/package.json (>=20.18.1) only describe what these published packages support — they do not relax the engine of a dev-dependency being installed. pnpm checks each installed package's own engines field against the current Node, regardless of what the host package declares.
The PR author's local verification ("typecheck and lint pass, CLI tests pass") almost certainly ran on Node 22+; nothing in the PR itself changes the pinned Node version.
Step-by-step proof
- CI job (e.g.
cli_tests.yml) starts onubuntu-latest. wistia/parse-tool-versions@v2.1.1reads.tool-versionsand exportsTOOL_VERSION_NODEJS=20.19.5.actions/setup-nodeinstalls Node 20.19.5.- The job runs
pnpm install --frozen-lockfile. - pnpm reads
.npmrc→engine-strict=true. - pnpm resolves the lockfile and encounters
tsdown@0.22.3withengines: { node: '^22.18.0 || >=24.11.0' }. semver.satisfies('20.19.5', '^22.18.0 || >=24.11.0')→false.- pnpm exits with
ERR_PNPM_BAD_ENGINE; the workflow fails beforepnpm buildis ever invoked.
This reproduces for every workflow that installs dependencies — lint, typecheck, tests, build, publish.
Impact
Merging this PR will red-CI the repo on the first push: lint, typecheck, CLI tests, JS SDK tests, package artifacts, and (most damagingly) the publish/release workflows will all fail at the install step. The patch changeset cannot be released.
How to fix
Pick one:
- Recommended: bump
.tool-versionsto a Node release matching^22.18.0 || >=24.11.0(e.g.nodejs 22.18.0), and bump theengines.nodefields in bothpackages/cli/package.jsonandpackages/js-sdk/package.jsonaccordingly. This is a coordinated change with downstream-consumer implications, so it should be called out in the changeset. - Alternatively, pin
tsdownto a pre-0.22release that still supports Node 20 (and verify its transitive deps also do). - Or, less ideally, set
engine-strict=falsein.npmrc— but this defeats the project's existing safety net for all engine constraints, not just tsdown's, so it's a regression in its own right.
|
This one is blocked on #1512 |
mishushakov
force-pushed
the
mishushakov/switch-tsup-to-tsdown
branch
from
f9a01cb to
44523e2
Compare
Replace tsup with tsdown (rolldown-based) for building the js-sdk and cli packages. Published artifact layout is preserved: js-sdk ships dist/index.js (CJS), dist/index.mjs (ESM) and .d.ts/.d.mts; the cli ships an executable dist/index.js plus dist/templates. The cli build targets Node 22. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
mishushakov
force-pushed
the
mishushakov/switch-tsup-to-tsdown
branch
from
44523e2 to
437a90d
Compare
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 437a90d. Configure here.
| export default defineConfig({ | ||
| entry: ['src/index.ts'], | ||
| shims: true, // Needed for "open" package, as it uses import.meta.url and we are building for cjs | ||
| target: 'node22', |
There was a problem hiding this comment.
Node 22 target, engines unchanged
Medium Severity
The CLI build now uses target: 'node22', but package.json still declares "engines": { "node": ">=20" }. Installers and users on Node 20/21 get no signal that the published bundle may require Node 22, which conflicts with the stated intent to align runtime support with the new compile target.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 437a90d. Configure here.
There was a problem hiding this comment.
jajaja that's another pr my lil bro
1 participant
Switches the build tooling for
packages/js-sdkandpackages/clifromtsup(esbuild) totsdown(rolldown), replacing eachtsup.config.jswith atsdown.config.tsand updating thebuild/devscripts and devDependencies accordingly. The published artifact layout is intentionally unchanged — the SDK still shipsdist/index.js(CJS),dist/index.mjs(ESM) anddist/index.d.ts/.d.mts, and the CLI still ships an executabledist/index.jsplusdist/templates— kept identical viafixedExtension: false. CLI dependency bundling is preserved by mapping the oldnoExternalto tsdown'sdeps.alwaysBundle(still excluding the ESM-only, dynamically-importedinquirer), with template copying moved from anonSuccessshell step to tsdown'scopyoption. The CLI build now targetsnode22; the js-sdk stays at its conservative, browser-compatiblees2017target. This is an internal tooling change with no public API or runtime behavior changes.Verification
typecheckandlintpass for both packages; CLI tests pass (94, excluding the pre-existingE2B_API_KEY-gated integration test).require) and ESM (import), exposing the defaultSandboxexport and all named exports; the built CLI runs (--version,--help).