chore: replace tsup bundle with plain tsc emit - #50
Merged
Conversation
Now that the package is ESM-only, the reasons tsup was adopted (dual CJS+ESM output, bundled .d.ts) no longer apply, so drop the bundler: - build with tsc -p tsconfig.build.json; dist/ mirrors src/ with per-module .js/.d.ts/.js.map instead of a single bundle - the emitting compiler is the same one that typechecks, so build output can never drift from what tsc verified - public entry points are unchanged (dist/index.js, dist/index.d.ts); the exports map still blocks deep imports, so consumers see no difference - esm-package.spec.ts builds via tsc and still verifies the built package under real Node ESM - remove tsup from devDependencies and drop tsup.config.ts Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017zXcPSv6FFTPgD59T4KudM
There was a problem hiding this comment.
Pull request overview
This PR removes the tsup bundling step and switches the package build to a plain TypeScript compiler emit (tsc -p tsconfig.build.json), producing a dist/ tree that mirrors src/ for the ESM-only package.
Changes:
- Replace the
tsupbuild withtscusing a dedicatedtsconfig.build.json. - Remove
tsupconfiguration and dependency. - Update the ESM package integration test to build with
tscdirectly and validate emitted modules.
Reviewed changes
Copilot reviewed 5 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tsup.config.ts | Removes the tsup bundler configuration. |
| tsconfig.json | Stops including the removed tsup.config.ts in the main TS project. |
| tsconfig.build.json | Adds a build-specific TS config to emit dist/ from src/. |
| test/integration/esm-package.spec.ts | Builds via tsc and validates emitted ESM output under real Node. |
| package.json | Updates build script to run tsc (but currently has an eval bug). |
| package-lock.json | Removes tsup and its dependency tree from the lockfile. |
| .github/copilot-instructions.md | Updates contributor instructions to reflect the new build. |
The predefined-builtins behavior of node --eval is documented and the previous form worked, but the explicit require reads unambiguously. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017zXcPSv6FFTPgD59T4KudM
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #24. Now that the package is ESM-only, the reasons tsup was adopted in Phase 3 (dual CJS+ESM output, bundled
.d.tsrollup) no longer apply — so this removes the bundler instead of maintaining it.Changes
npm run buildnow runstsc -p tsconfig.build.json(new config extendingtsconfig.jsonwithnoEmit: false,outDir: dist,rootDir: src).dist/mirrorssrc/with per-module.js/.d.ts/.js.mapinstead of a single bundle.tsupdevDependency andtsup.config.ts.dist/index.js/dist/index.d.ts, somain,types, and theexportsmap are untouched, and theexportsmap continues to block deep imports into internals. Docker (node dist/index.js) unchanged.esm-package.spec.tsbuilds viatsc(shell-free, cross-platform) and still verifies the built package loads under real Node ESM through the exports map with no empty side-effect imports — now checking every emitted module rather than one bundle.Why
nodenext+verbatimModuleSyntax), so the published output can never drift from whattscverified — and the declaration rollup step (historically the fragile part of bundler-based library builds) is gone.@dopry/pecans/backends, …) a one-lineexportsaddition.Verified: lint, prettier, typecheck, full vitest suite (771 tests),
node dist/index.jsserver start, and a real-Nodeimport('@dopry/pecans')smoke test.🤖 Generated with Claude Code
https://claude.ai/code/session_017zXcPSv6FFTPgD59T4KudM
Generated by Claude Code