Repository navigation
Conversation
🦋 Changeset detectedLatest commit: 12454c9 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
Contributor
Coverage Report
📁 File Coverage (20 files)
|
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
lookpath, progress-stream and @inquirer/type (already pulled in by the @InQuirer prompts) are never imported. @types/inquirer, @types/node-fetch, @types/prompts and @types/progress-stream type packages that are not used, and tslib is only needed when tsc emits code with importHelpers. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XGgBftPRVbzZUyL3gjvJaj
build.ts bundles every package into dist/index.js (and into the compiled binaries), so the published CLI only imports node builtins at runtime. Listing them as dependencies made every npm install of @cartesi/cli download packages that are never loaded. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XGgBftPRVbzZUyL3gjvJaj
The package is a CLI exposed through bin, not a library, and the build never emits dist/index.d.ts. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XGgBftPRVbzZUyL3gjvJaj
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XGgBftPRVbzZUyL3gjvJaj
Bun 1.4.1 and later (1.4.3 included) break the tmp package when bundling to ESM: the bundler inlines its CommonJS module but keeps a reference to an exports object it never defines, so dist/index.js throws "ReferenceError: exports_tmp is not defined" at startup. The compiled binaries are not affected since bytecode output is CommonJS. tmp was only used for one temporary path, the buildx --metadata-file. That file now lives in an mkdtemp directory that is removed once the metadata is read; before, tmpNameSync only produced a name, so setGracefulCleanup never removed it. Tests use mkdtempSync and rmSync like the other tests already do. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XGgBftPRVbzZUyL3gjvJaj
tuler
force-pushed
the
claude/cli-dependency-cleanup
branch
from
October 14, 2026 02:49
c82817f to
12454c9
Compare
tuler
marked this pull request as ready for review
October 14, 2026 02:53
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.
Cleans up
apps/cli/package.json, pins Bun 1.4.3, and replacestmpso the npm bundle works with that version. There is one commit per change.1. Remove unused packages
These are never imported in
src/ortests/:lookpath,progress-stream,@inquirer/type(already pulled in by@inquirer/core,input,selectandconfirm)@types/inquirer,@types/node-fetch,@types/prompts,@types/progress-stream,tslib(only needed whentscemits withimportHelpers; heretscruns withnoEmit)commanderstays because@commander-js/extra-typingsrequires it as a peer dependency.2. Move bundled runtime packages to
devDependenciesbuild.tscallsBun.buildwithoutexternalorpackages: "external", so every package is bundled intodist/index.jsand into the compiled binaries. I checked the built bundle: its only imports are Node built-ins, and nothing readsnode_modulesat runtime. Listing these packages asdependenciesmade everynpm i @cartesi/clidownload viem, execa, listr2 and the rest, even though they are never loaded. Homebrew users get the compiled binaries, so they are not affected.Trade-offs:
npm auditrun by people who install the CLI no longer reports vulnerabilities in these packages. Fixes reach users only when a new version is released (true in practice already, since the code is bundled).build.ts, it has to go back intodependencies.3. Drop
exportsandtypesThe package is a CLI exposed through
bin, not a library.dist/index.d.tsis never generated, and nothing in the monorepo imports@cartesi/cli.4. Pin Bun 1.4.3
packageManagergoes tobun@1.4.3, which the CI workflows read throughsetup-bun.@types/bunandCLAUDE.mdare updated to match.5. Replace
tmpwithnode:fsBun 1.4.1 and later, including 1.4.3, break the CommonJS
tmppackage when bundling to ESM. The bundler inlines the module but keeps a reference to anexports_tmpobject it never defines, sodist/index.jsthrowsReferenceError: exports_tmp is not definedat startup (minified:Kj is not defined/sq is not defined). This affects only the ESM bundle, which is the npm package. The compiled binaries usebytecode: true, so their output is CommonJS and works. The integration tests runnode dist/index.js, so they would have failed with the pin alone.In production code,
tmpwas only used for one path, the buildx--metadata-file:mkdtempdirectory, which is removed in afinallyonce the metadata has been read.tmpNameSync()only produced a name, andsetGracefulCleanup()does not delete files thattmpdid not create. So the metadata file was never deleted, and that call is now gone.mkdtempSyncandrmSyncinstead, liketests/unit/commands/refund.test.tsand others already do.A changeset (
@cartesi/clipatch) is included.Testing
Everything below used Bun 1.4.3:
bun install --frozen-lockfile,bun run lint,tscand the unit tests (335 pass, 0 fail) all pass.build.ts:node dist/index.js --version,bun dist/index.js --versionandbin/cartesi-linux-x64 --versionall print2.0.0-alpha.38. The bundle imports only Node built-ins.buildImage: no Docker daemon was available, so I ranbuild()against a fakedockeronPATH:cartesi-buildx-*directorydocker image inspect🤖 Generated with Claude Code
https://claude.ai/code/session_01XGgBftPRVbzZUyL3gjvJaj