-
Notifications
You must be signed in to change notification settings - Fork 44
build: fix sdk npm packaging #2780
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
WalkthroughThe CI release timeout is increased to 60 minutes. In js-evo-sdk, the prepack script is removed. In wasm-sdk, build scripts are reorganized: a direct build script replaces build:dev, prepack is removed, and build.sh now delegates release builds to build-optimized.sh while preserving the non-release path. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Dev/CI
participant NPM Script as npm run build (wasm-sdk)
participant build.sh
participant build-optimized.sh
participant build-wasm.sh
participant Bundler as bundle.cjs
Dev/CI->>NPM Script: npm run build
NPM Script->>build.sh: ./scripts/build.sh
alt CARGO_BUILD_PROFILE=release
build.sh-->>build-optimized.sh: exec (replace process)
build-optimized.sh->>build-optimized.sh: Perform optimized release build
else non-release
build.sh->>build.sh: Determine OPT_LEVEL
build.sh->>build-wasm.sh: Invoke standard build
build-wasm.sh-->>build.sh: Done
end
NPM Script->>Bundler: node ./scripts/bundle.cjs
Bundler-->>Dev/CI: Build artifacts bundled
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ 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. Comment |
✅ gRPC Query Coverage Report
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/release.yml (1)
36-47
: Critical: publish may run without built artifacts after removing prepack.When
check-artifact.outputs.exists == 'true'
, all build/setup steps are skipped, but the publish step still runs. With prepack hooks removed in SDK packages, this risks publishing packages withoutdist/
. Fix by downloading the existing build artifact before publish or by always building.Apply this diff to download the artifact when it exists (before publishing):
- name: Configure NPM auth token run: yarn config set npmAuthToken ${{ secrets.NPM_TOKEN }} + - name: Download JS build artifacts (reuse) + if: ${{ steps.check-artifact.outputs.exists == 'true' }} + uses: actions/download-artifact@v4 + with: + name: js-build-${{ github.sha }} + path: . + + - name: Verify package builds exist + run: | + missing=0 + for pkg in packages/*/package.json; do + if jq -e '.private|not' "$pkg" >/dev/null; then + dir="$(dirname "$pkg")/dist" + [ -d "$dir" ] || { echo "Missing: $dir"; missing=1; } + fi + done + [ "$missing" -eq 0 ] || { echo "One or more dist/ folders are missing"; exit 1; } + - name: Publish NPM packages run: yarn workspaces foreach --all --no-private --parallel npm publish --tolerate-republish --access public --tag ${{ steps.tag.outputs.result }}Also applies to: 86-92, 126-128
🧹 Nitpick comments (3)
packages/js-evo-sdk/package.json (1)
32-32
: Prepack removal: confirm publish safety or add prepublishOnly.Since CI can skip builds when reusing artifacts, ensure
dist/
is present before publish. If you want belt-and-suspenders, add:"scripts": { "build": "rm -rf dist && tsc -p tsconfig.json && webpack --config webpack.config.cjs", + "prepublishOnly": "yarn build",
packages/wasm-sdk/scripts/build.sh (2)
13-17
: Release fast‑path is clean and explicit.Early
exec
to optimized build is good. Consider emitting a short log for traceability:if [ "${CARGO_BUILD_PROFILE:-}" = "release" ]; then - # exec replaces the current shell process; the script will not continue after this. + echo "[wasm-sdk] Using optimized release build" exec "$SCRIPT_DIR/build-optimized.sh" fi
21-23
: OPT_LEVEL heuristic comment vs. behavior.The code sets
minimal
when not CI or when profile isdev
. That matches local dev expectations; just ensure the comment reflects “minimal for dev/local (CI=false)”. No action needed if intentional.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/workflows/release.yml
(1 hunks)packages/js-evo-sdk/package.json
(1 hunks)packages/wasm-sdk/package.json
(1 hunks)packages/wasm-sdk/scripts/build.sh
(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
packages/wasm-sdk/**
📄 CodeRabbit inference engine (CLAUDE.md)
Keep WASM SDK docs in sync (run generate_docs.py) when updating the WASM SDK
Files:
packages/wasm-sdk/package.json
packages/wasm-sdk/scripts/build.sh
🧠 Learnings (3)
📚 Learning: 2025-09-03T14:42:29.958Z
Learnt from: thephez
PR: dashpay/platform#2754
File: packages/wasm-sdk/docs.html:1970-1971
Timestamp: 2025-09-03T14:42:29.958Z
Learning: In packages/wasm-sdk/, the docs.html file is auto-generated from api-definitions.json. Any documentation fixes should be made in api-definitions.json rather than directly in docs.html, as manual changes to docs.html would be overwritten during regeneration.
Applied to files:
packages/wasm-sdk/package.json
📚 Learning: 2025-09-07T22:18:50.883Z
Learnt from: CR
PR: dashpay/platform#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-07T22:18:50.883Z
Learning: Applies to packages/wasm-sdk/** : Keep WASM SDK docs in sync (run generate_docs.py) when updating the WASM SDK
Applied to files:
packages/wasm-sdk/package.json
📚 Learning: 2025-09-03T14:41:16.196Z
Learnt from: thephez
PR: dashpay/platform#2754
File: packages/wasm-sdk/AI_REFERENCE.md:766-766
Timestamp: 2025-09-03T14:41:16.196Z
Learning: In packages/wasm-sdk/, the AI_REFERENCE.md file is auto-generated from api-definitions.json. Any documentation fixes should be made in api-definitions.json rather than directly in AI_REFERENCE.md, as manual changes to AI_REFERENCE.md would be overwritten during regeneration.
Applied to files:
packages/wasm-sdk/package.json
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build JS packages / Build JS
🔇 Additional comments (2)
.github/workflows/release.yml (1)
25-25
: Timeout bump looks good.Extending to 60 minutes is reasonable for heavier Rust/WASM builds.
packages/wasm-sdk/package.json (1)
27-27
: Direct build script LGTM; aligns with release profile path.
yarn build
now routes throughscripts/build.sh
then bundles. Works withCARGO_BUILD_PROFILE=release
in the workflow. Please confirm no public API/doc changes; if any, regenerate WASM SDK docs (generate_docs.py) per repo guidelines.
Issue being fixed or feature implemented
prepack
in package.json and release workflow's build command we built new SDKs twice.What was done?
prepack
script from package.jsonCARGO_BUILD_PROFILE
equalsrelease
.How Has This Been Tested?
None
Breaking Changes
None
Checklist:
For repository code-owners and collaborators only
Summary by CodeRabbit