chore: fix publish package version #481
Conversation
WalkthroughThis PR adds publish-time tooling and release script updates: new Bun-based scripts to rewrite workspace and catalog dependency versions before publishing and restore them afterward, moves commitlint config into package.json, updates the Husky commit-msg hook to use bunx, adds a .changeset entry, and expands .gitignore. Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant RS as release (npm script)
participant PP as prepare-publish.ts
participant CS as changeset publish
participant RP as restore-packages.ts
participant BK as .package-backups
Dev->>RS: run release / release:dry-run
RS->>PP: bun scripts/prepare-publish.ts
PP->>BK: backup package.json files
PP->>PP: resolve workspace:/catalog: versions and rewrite
RS->>CS: changeset publish (--dry-run)
RS->>RP: bun scripts/restore-packages.ts --cleanup
RP->>BK: restore originals, then remove backups
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🔭 Outside diff range comments (1)
package.json (1)
51-63: Move @changesets/cli to devDependencies (tooling only).
@changesets/cliis a release-time tool and shouldn't be a runtime dependency of the root package. Keeping it in devDependencies avoids unnecessary installs for consumers and better reflects intent.Apply this diff to add it to devDependencies:
"devDependencies": { @@ - "typedoc-plugin-missing-exports": "^3.1.0" + "typedoc-plugin-missing-exports": "^3.1.0", + "@changesets/cli": "^2.29.5" },And remove it from dependencies (see next comment).
🧹 Nitpick comments (2)
scripts/prepare-publish.ts (2)
30-36: Consider error handling for file I/O operations.The
readJSONandwriteJSONfunctions don't handle potential file system errors like permission issues or malformed JSON files.function readJSON<T = any>(filePath: string): T { - return JSON.parse(fs.readFileSync(filePath, 'utf8')); + try { + return JSON.parse(fs.readFileSync(filePath, 'utf8')); + } catch (error) { + throw new Error(`Failed to read JSON from ${filePath}: ${error instanceof Error ? error.message : error}`); + } } function writeJSON(filePath: string, data: any): void { - fs.writeFileSync(filePath, JSON.stringify(data, null, 4) + '\n'); + try { + fs.writeFileSync(filePath, JSON.stringify(data, null, 4) + '\n'); + } catch (error) { + throw new Error(`Failed to write JSON to ${filePath}: ${error instanceof Error ? error.message : error}`); + } }
38-57: Consider making package discovery more flexible.The function hardcodes the
packagesdirectory and assumes a flat structure. This might break if workspace packages are organized differently.#!/bin/bash # Check if there are workspace packages outside the packages/ directory rg -A 5 '"workspaces"' package.json
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.changeset/tall-taxes-change.md(1 hunks).gitignore(1 hunks).husky/commit-msg(1 hunks)commitlint.config.js(0 hunks)package.json(3 hunks)scripts/prepare-publish.ts(1 hunks)scripts/restore-packages.ts(1 hunks)
💤 Files with no reviewable changes (1)
- commitlint.config.js
🔇 Additional comments (13)
.husky/commit-msg (1)
1-1: LGTM! Good alignment with the Bun ecosystem.The change from
npxtobunxis consistent with the repository's migration to Bun-based tooling and the move of commitlint configuration to package.json..gitignore (1)
15-15: LGTM! Appropriate backup directory exclusion.Adding
.package-backupsto gitignore is correct since this directory contains temporary backup files created by the publish tooling and should not be tracked in version control..changeset/tall-taxes-change.md (1)
1-18: LGTM! Comprehensive changeset for publish tooling.The changeset properly documents patch changes across all workspace packages affected by the new publish tooling. The description accurately reflects the purpose of fixing workspace package version handling during publishing.
scripts/prepare-publish.ts (5)
6-7: LGTM! Proper path resolution using import.meta.dir.The use of
import.meta.diris the correct Bun-specific way to get the current script directory, and the backup directory path is appropriately constructed.
103-111: LGTM! Proper workspace version resolution with fallback.The logic correctly handles
workspace:*dependencies by resolving them to caret ranges using actual workspace package versions, with appropriate warning for unresolved dependencies.
112-121: LGTM! Catalog version resolution implemented correctly.The catalog dependency resolution correctly maps
catalog:references to their defined versions from the root package.json workspaces.catalog configuration.
161-166: LGTM! Proper error handling and process exit.The top-level error handler correctly catches exceptions, logs meaningful error messages, and exits with a non-zero status code for CI/CD pipeline failure detection.
1-1: Bun installation is already configured in CI workflowsAll relevant GitHub Actions workflows (e.g. .github/workflows/ci.yaml, docs.yaml, release.yaml) invoke
uses: oven-sh/setup-bun@v1before running anybuncommands, ensuringbunis on the PATH and the shebang will resolve correctly. No changes needed.scripts/restore-packages.ts (4)
1-1: LGTM! Consistent with Bun ecosystem adoption.The script correctly uses the Bun shebang, maintaining consistency with the
prepare-publish.tsscript and the repository's move to Bun-based tooling.
19-38: LGTM! Robust recursive restoration with proper path handling.The nested
restoreFilesfunction correctly handles recursive directory traversal, proper relative path calculation, and safe restoration by checking for original file existence before copying.
45-50: LGTM! Safe cleanup with force flag.The cleanup function uses
rmSyncwithrecursive: trueandforce: true, which is appropriate for removing the backup directory and handles cases where the directory might not exist.
64-67: LGTM! Consistent error handling pattern.The error handling matches the pattern established in
prepare-publish.tswith proper error message extraction and non-zero exit codes for CI/CD integration.package.json (1)
68-71: Inline commitlint config in package.json: LGTM.Consolidating commitlint config here is clean and reduces config sprawl. Works with Husky’s commit-msg hook and commitlint CLI.
| @@ -1,5 +1,6 @@ | |||
| { | |||
| "name": "dojo.js", | |||
| "type": "module", | |||
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Adding "type":"module" at the root can break CJS-only scripts—verify repo-wide usage.
Setting ESM at the root affects script execution (e.g., require, module.exports, __dirname/__filename usage). Since scripts now run under ESM semantics in many contexts, ensure no CJS-only constructs remain in repo-level scripts and configs.
Run this to locate potential incompatibilities:
🏁 Script executed:
#!/bin/bash
# Find common CJS-only patterns that may break under ESM at the repo root.
rg -n --hidden --glob '!**/node_modules/**' --glob '!**/dist/**' --glob '!**/build/**' --glob '!**/.package-backups/**' \
-e '\brequire\(' \
-e '\bmodule\.exports\b' \
-e '\bexports\.' \
-e '\b__dirname\b' \
-e '\b__filename\b' \
-S
echo
echo "If matches show up in root-level scripts/configs executed by Node/Bun, consider migrating to ESM (import/export) and using fileURLToPath for __dirname/__filename."Length of output: 2798
CJS Patterns Detected—Ensure ESM Compatibility
Our rg scan shows multiple CommonJS‐only constructs that will break once "type": "module" makes all .js files ESM by default:
• packages/utils/jest.config.js & packages/state/jest.config.js
– module.exports = { … }
• packages/torii-wasm/node.mjs
– const dojo_c = require("./pkg/node/dojo_c.js");
• packages/create-dojo/tsup.config.ts & generator code
– __dirname/__filename for path resolution
• packages/grpc/src/benchmarks/report.ts & scripts/fetch-raw-test-data.ts
– require("node:fs"), path.join(__dirname, …)
• Example projects (*.cjs, tailwind.config.ts, vite.config.ts, log.js)
– mixed use of require(), module.exports, __dirname
Actions Required:
- Rename any CJS‐only
.jsfiles to.cjsso Node still treats them as CommonJS. - Where possible, convert to ESM:
- Replace
module.exports→export default/export { … } - Swap
require()calls forimportstatements.
- Replace
- For filesystem paths, replace
__dirname/__filenamewith:import { fileURLToPath } from 'url'; const __filename = fileURLToPath(import.meta.url); const __dirname = path.dirname(__filename);
- If you must dynamically load CJS from an ESM file, use:
import { createRequire } from 'module'; const require = createRequire(import.meta.url);
Please update these files before shipping root-level "type": "module".
🤖 Prompt for AI Agents
In package.json around lines 3 to 3, adding "type": "module" will make all .js
files be treated as ESM and break existing CommonJS patterns; update the listed
files by renaming any CJS-only .js files to .cjs (so Node treats them as
CommonJS), convert files that can be ESM by replacing module.exports with ES
exports and require() calls with import statements, and for code that relies on
__dirname/__filename replace that logic with the import.meta.url -> file URL to
path approach, while any ESM file that must load CJS at runtime should create a
require function from the module loader and use it for dynamic imports; apply
these changes to packages/utils/jest.config.js, packages/state/jest.config.js,
packages/torii-wasm/node.mjs, packages/create-dojo/tsup.config.ts and generator
code, packages/grpc/src/benchmarks/report.ts, scripts/fetch-raw-test-data.ts,
and example projects so the repo is safe to ship with the root "type": "module".
| "release": "bun run build:deps && bun run prepare:publish && changeset publish && bun run restore:packages --cleanup", | ||
| "release:dry-run": "bun run build:deps && bun run prepare:publish && changeset publish --dry-run && bun run restore:packages --cleanup", |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Always run restore even when publish fails (use trap/finally pattern).
With the current && chaining, if changeset publish fails, restore:packages --cleanup will not run, potentially leaving the repo in a modified state. Wrap the publish step so restore executes regardless of success, and preserve the original exit code.
Apply this diff to make restore run unconditionally while keeping prior behavior on success/failure:
- "release": "bun run build:deps && bun run prepare:publish && changeset publish && bun run restore:packages --cleanup",
- "release:dry-run": "bun run build:deps && bun run prepare:publish && changeset publish --dry-run && bun run restore:packages --cleanup",
+ "release": "bash -lc 'set -euo pipefail; trap \"bun run restore:packages --cleanup\" EXIT; bun run build:deps; bun run prepare:publish; changeset publish'",
+ "release:dry-run": "bash -lc 'set -euo pipefail; trap \"bun run restore:packages --cleanup\" EXIT; bun run build:deps; bun run prepare:publish; changeset publish --dry-run'"Notes:
- This guarantees cleanup runs even on failures or interrupts.
- The repo already uses bash in other scripts (e.g.,
clean), so this aligns with existing tooling.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "release": "bun run build:deps && bun run prepare:publish && changeset publish && bun run restore:packages --cleanup", | |
| "release:dry-run": "bun run build:deps && bun run prepare:publish && changeset publish --dry-run && bun run restore:packages --cleanup", | |
| "release": "bash -lc 'set -euo pipefail; trap \"bun run restore:packages --cleanup\" EXIT; bun run build:deps; bun run prepare:publish; changeset publish'", | |
| "release:dry-run": "bash -lc 'set -euo pipefail; trap \"bun run restore:packages --cleanup\" EXIT; bun run build:deps; bun run prepare:publish; changeset publish --dry-run'" |
🤖 Prompt for AI Agents
In package.json around lines 42-43, the release and release:dry-run scripts
chain commands with && so restore:packages --cleanup is skipped if changeset
publish fails; change each script to run in a bash context that registers a trap
or uses a finally-like pattern to run bun run restore:packages --cleanup on exit
regardless of success or failure and then re-exit with the original command's
exit code (i.e., run the build/prepare/publish steps, capture their exit status,
always run the restore cleanup, and then exit with the captured status) so
cleanup always runs but original success/failure is preserved.
| "dependencies": { | ||
| "@changesets/cli": "^2.29.5", | ||
| "@dojoengine/recs": "2.0.13" | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove @changesets/cli from dependencies (duplicate; belongs in devDependencies).
Completes the move suggested above.
"dependencies": {
- "@changesets/cli": "^2.29.5",
"@dojoengine/recs": "2.0.13"
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "dependencies": { | |
| "@changesets/cli": "^2.29.5", | |
| "@dojoengine/recs": "2.0.13" | |
| }, | |
| "dependencies": { | |
| "@dojoengine/recs": "2.0.13" | |
| }, |
🤖 Prompt for AI Agents
In package.json around lines 64 to 67, @changesets/cli is incorrectly listed
under "dependencies"; remove the "@changesets/cli" entry from the dependencies
object and add (or ensure it exists) under "devDependencies" with the same
version string (e.g. "^2.29.5"), then run npm/yarn install to update lockfile so
the package is recorded as a dev-only dependency.
Closes #
Introduced changes
Checklist
Summary by CodeRabbit