You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Closes#134. The published cdr-cli printed a stale version for --version (the 0.2.2 binary reported 0.1.2), because apps/cli/src/index.ts hardcoded .version("0.1.2") into commander. The release workflow bumps only the four package.json manifests, so the npm-level version was correct but the binary never knew its own version.
The version is now derived from package.json at runtime, so the stamp cannot drift from the published version again. This is issue #134's Option 1, kept compatible with dual (ESM + CJS) publishing via a tshy dialect polyfill rather than dropping the CJS build or generating a file at build time.
Approach
src/version.ts (ESM): createRequire(import.meta.url)("@piplabs/cdr-cli/package.json").version. Self-referencing through the package's own exports map resolves correctly from both src/ (tsx dev) and dist/esm/ without a depth-fragile relative path.
src/version-cjs.cts: the CommonJS counterpart using native require. tshy's -cjs.cts dialect-polyfill convention swaps this in for the CJS build only, where import.meta won't compile. A //@ts-ignore on the ESM file's import.meta line is required because tshy still type-checks it during the CJS pass — this is the documented convention.
The release workflow's bump step runs npm pkg set version on apps/cli (it's in the PACKAGES list) and nothing else, then rebuilds before publish via prepublishOnly. Both build dialects read the version at runtime from that freshly-bumped manifest, so a bump-only change flows to the binary automatically — verified by simulation below.
Test plan
pnpm lint (tsc --noEmit) and pnpm build (tshy) clean
--version reports 0.2.2 from all run modes: built ESM (dist/esm/index.js, the bin), built CJS (dist/commonjs/index.js), and dev (tsx src/index.ts)
Future-bump simulation: npm pkg set version=9.9.9 on package.json only (no source edits), rebuild → ESM/CJS/dev all report 9.9.9; restored to 0.2.2 after
Packaged-artifact check: pnpm pack → installed the tarball into a clean project → ./node_modules/.bin/cdr-cli --version reports the correct version (reproduces the environment where 0.2.2 failed)
New integration case in apps/cli/__integration__/cli.test.ts asserts the binary's --version equals package.json (network-independent; guards against reintroducing a hardcoded literal)
Notes
The --version regression is caught by the integration suite (source tree) and by post-release verification (published tarball, which is what caught it last time). Neither gates the publish itself — this PR makes the drift structurally impossible rather than adding a pre-publish gate. A release-time assertion can be added separately if we want.
The new integration assertion exercises the ESM build, which is the only dialect bin.cdr-cli points at — i.e. the only version surface a CLI user can reach. The CJS --version path is verified manually but is not consumer-reachable given the current package shape (no require-able version API; bin is a literal ESM path, not an exports-conditional entry).
The fix is structurally sound and the approach is well-reasoned. Two issues worth verifying before merge:
exports map dependency (both version.ts and version-cjs.cts): require("@piplabs/cdr-cli/package.json") is a self-referencing sub-path import. In Node.js 12+ this throws ERR_PACKAGE_PATH_NOT_EXPORTED unless "./package.json": "./package.json" appears in the package's exports map. The diff doesn't show apps/cli/package.json, so this constraint can't be confirmed from the review. The pack+install test in the test plan would catch a missing entry, but CI should guard this too.
Integration test placement / misleading "no network, no wallet" comment: The new describe("binary metadata") block sits in the same file as the chain-mutation suite and shares its module-level beforeAll, which deploys a smart contract and requires CDR_TEST_PRIVATE_KEY, CDR_API_URL, and CDR_RPC_URL. The test will not run in a network-less environment despite the comment claiming otherwise, and the 30 s timeout reflects the blockchain wait rather than the actual work of the test.
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
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.
Summary
Closes #134. The published
cdr-cliprinted a stale version for--version(the 0.2.2 binary reported0.1.2), becauseapps/cli/src/index.tshardcoded.version("0.1.2")into commander. The release workflow bumps only the fourpackage.jsonmanifests, so the npm-level version was correct but the binary never knew its own version.The version is now derived from
package.jsonat runtime, so the stamp cannot drift from the published version again. This is issue #134's Option 1, kept compatible with dual (ESM + CJS) publishing via a tshy dialect polyfill rather than dropping the CJS build or generating a file at build time.Approach
src/version.ts(ESM):createRequire(import.meta.url)("@piplabs/cdr-cli/package.json").version. Self-referencing through the package's ownexportsmap resolves correctly from bothsrc/(tsx dev) anddist/esm/without a depth-fragile relative path.src/version-cjs.cts: the CommonJS counterpart using nativerequire. tshy's-cjs.ctsdialect-polyfill convention swaps this in for the CJS build only, whereimport.metawon't compile. A//@ts-ignoreon the ESM file'simport.metaline is required because tshy still type-checks it during the CJS pass — this is the documented convention.src/index.ts:.version("0.1.2")→.version(version).Why this survives the next release
The release workflow's bump step runs
npm pkg set versiononapps/cli(it's in thePACKAGESlist) and nothing else, then rebuilds before publish viaprepublishOnly. Both build dialects read the version at runtime from that freshly-bumped manifest, so a bump-only change flows to the binary automatically — verified by simulation below.Test plan
pnpm lint(tsc --noEmit) andpnpm build(tshy) clean--versionreports0.2.2from all run modes: built ESM (dist/esm/index.js, the bin), built CJS (dist/commonjs/index.js), and dev (tsx src/index.ts)npm pkg set version=9.9.9onpackage.jsononly (no source edits), rebuild → ESM/CJS/dev all report9.9.9; restored to0.2.2afterpnpm pack→ installed the tarball into a clean project →./node_modules/.bin/cdr-cli --versionreports the correct version (reproduces the environment where 0.2.2 failed)apps/cli/__integration__/cli.test.tsasserts the binary's--versionequalspackage.json(network-independent; guards against reintroducing a hardcoded literal)Notes
--versionregression is caught by the integration suite (source tree) and by post-release verification (published tarball, which is what caught it last time). Neither gates the publish itself — this PR makes the drift structurally impossible rather than adding a pre-publish gate. A release-time assertion can be added separately if we want.bin.cdr-clipoints at — i.e. the only version surface a CLI user can reach. The CJS--versionpath is verified manually but is not consumer-reachable given the current package shape (norequire-able version API;binis a literal ESM path, not anexports-conditional entry).