Repository navigation
run npm stage publish instead of npm publish in changesets - #13245
Conversation
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (1)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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. Comment |
commit: |
There was a problem hiding this comment.
Pull request overview
This PR updates the Changesets-based release flow to use npm’s staged publishing (npm stage publish) instead of immediately publishing (npm publish), enabling an approval step after tags/releases are created.
Changes:
- Adds a PATH-injected
npmshim that rewritesnpm publishtonpm stage publishfor Changesets publishing. - Updates the
changeset-publishscript to prepend the shim directory toPATHduringchangeset publish. - Updates the release workflow to upgrade npm to a version that supports staged publishing.
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
scripts/changesets/wrap-staged/npm |
New bash shim that rewrites npm publish to npm stage publish. |
package.json |
Updates changeset-publish to run Changesets with the shim-injected PATH. |
.gitignore |
Ensures the shim file named npm isn’t ignored by the existing npm ignore rule. |
.github/workflows/release.yml |
Installs a newer npm version required for npm stage publish before running the release action. |
| export PATH="${PATH#*:}" | ||
|
|
||
| if [ "${1:-}" = "publish" ]; then | ||
| shift | ||
| exec npm stage publish "$@" | ||
| fi | ||
|
|
||
| exec npm "$@" |
There was a problem hiding this comment.
Yeah no, we don't need that for a stopgap 😅
| "docmodel": "npm run clean && node config/build.ts --step=prepareDist --step=addExports --step=typescript --step=inlineInheritDoc --step=deprecateInternals && node config/apiExtractor.ts --main-only --generate docModel", | ||
| "changeset-check": "changeset status --verbose --since=origin/main", | ||
| "changeset-publish": "npm pkg set 'workspaces[0]'=dist && npm run build && changeset publish", | ||
| "changeset-publish": "npm pkg set 'workspaces[0]'=dist && npm run build && PATH=\"$PWD/scripts/changesets/wrap-staged:$PATH\" changeset publish", |
There was a problem hiding this comment.
Wtf. If the npm version is wrong you get an error message 🤷
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/changesets/wrap-staged/npm`:
- Line 10: The export line export PATH="${PATH#*:}" can yield an empty PATH if
PATH is unset/empty; change it to defensively handle that case by either
wrapping it in a conditional that only updates PATH when PATH is non-empty
(e.g., check [ -n "$PATH" ] around the export) or use parameter expansion with a
safe default value for PATH so that PATH is never set to an empty string; update
the export PATH="${PATH#*:}" occurrence accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: ca936f72-a7ff-4813-a399-31b0ff63ac3d
📒 Files selected for processing (5)
.github/workflows/knip.yml.github/workflows/release.yml.gitignorepackage.jsonscripts/changesets/wrap-staged/npm
| # PATH entry, so we pop that entry to find the real npm. | ||
| set -euo pipefail | ||
|
|
||
| export PATH="${PATH#*:}" |
There was a problem hiding this comment.
Potential edge case: PATH could become empty.
The pattern ${PATH#*:} removes the prefix up to and including the first colon. If PATH contains no colon (e.g., if the original PATH was empty when prepended in package.json), this would result in an empty string, causing the npm command to fail.
While unlikely in CI environments, consider a more defensive pattern:
🛡️ Defensive PATH handling
-export PATH="${PATH#*:}"
+# Remove the first directory from PATH (the shim directory)
+export PATH="${PATH#*:}"
+if [ -z "$PATH" ]; then
+ echo "Error: PATH became empty after removing shim directory" >&2
+ exit 1
+fiOr use a more explicit approach:
-export PATH="${PATH#*:}"
+# Remove the shim directory (first entry) from PATH
+IFS=':' read -ra path_array <<< "$PATH"
+export PATH="$(IFS=:; echo "${path_array[*]:1}")"📝 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.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/changesets/wrap-staged/npm` at line 10, The export line export
PATH="${PATH#*:}" can yield an empty PATH if PATH is unset/empty; change it to
defensively handle that case by either wrapping it in a conditional that only
updates PATH when PATH is non-empty (e.g., check [ -n "$PATH" ] around the
export) or use parameter expansion with a safe default value for PATH so that
PATH is never set to an empty string; update the export PATH="${PATH#*:}"
occurrence accordingly.
There's no real workflow around changesets and npm staged publishing yet - this is discussed in changesets/changesets#2025.
So far, this will just run
npm stage publishinstead ofnpm publish, but already create tags and everything, before we approve it in the npm CLI or website. If we reject, a version number would be kinda "burned". But it feels like a good start.Summary by CodeRabbit