Prepare WASM bindings for release #1176
Conversation
WalkthroughThis pull request introduces significant changes to the Changes
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
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: 6
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (3)
bindings_wasm/.yarn/releases/yarn-4.5.1.cjsis excluded by!**/.yarn/**bindings_wasm/package-lock.jsonis excluded by!**/package-lock.jsonbindings_wasm/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (8)
- .github/workflows/release-wasm-bindings.yml (1 hunks)
- bindings_wasm/.gitignore (1 hunks)
- bindings_wasm/.node-version (1 hunks)
- bindings_wasm/.nvmrc (1 hunks)
- bindings_wasm/.yarnrc.yml (1 hunks)
- bindings_wasm/CHANGELOG.md (1 hunks)
- bindings_wasm/package.json (3 hunks)
- xtask/src/build.rs (0 hunks)
💤 Files with no reviewable changes (1)
- xtask/src/build.rs
✅ Files skipped from review due to trivial changes (4)
- bindings_wasm/.gitignore
- bindings_wasm/.node-version
- bindings_wasm/.nvmrc
- bindings_wasm/.yarnrc.yml
🧰 Additional context used
🪛 actionlint
.github/workflows/release-wasm-bindings.yml
13-13: label "warp-ubuntu-latest-x64-16x" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2022", "windows-2019", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-22.04", "ubuntu-20.04", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-14.0", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "macos-13.0", "macos-12-xl", "macos-12-xlarge", "macos-12-large", "macos-12", "macos-12.0", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file
(runner-label)
🪛 LanguageTool
bindings_wasm/CHANGELOG.md
[uncategorized] ~5-~5: Dieses Wort müsste großgeschrieben werden. Überlegen Sie, es zu ändern.
Context: ... @xmtp/wasm-bindings ## 0.0.1 Initial release(AI_DE_GGEC_REPLACEMENT_ORTHOGRAPHY_LOWERCASE)
🔇 Additional comments (6)
bindings_wasm/package.json (5)
26-30: Build script changes look good.The build scripts have been properly updated to:
- Use Yarn instead of npm
- Clean up release artifacts
- Ensure builds run before publishing
54-55: LGTM: Package manager specification.The explicit
packageManagerfield is a good practice that ensures consistent tooling across the project.
44-46: Breaking change: Node.js version requirement.The minimum Node.js version has been increased to 20. This is a breaking change that should be prominently documented in the CHANGELOG.
#!/bin/bash # Check if CHANGELOG mentions Node.js version update echo "Checking CHANGELOG for Node.js version update..." fd CHANGELOG --exec rg -i "node.*20|node.*version"
33-42: Verify the simplified dist structure.The package structure has been simplified with all outputs going to the dist root. Please ensure this works correctly for all target environments.
#!/bin/bash # Check the actual structure of the dist directory after build echo "Checking if the build creates the expected structure..." yarn build ls -la dist/
2-3: Verify package name change impact.The package has been renamed from
@xmtp/client-bindings-wasmto@xmtp/wasm-bindings. This is a breaking change that will affect any existing dependencies.✅ Verification successful
Let me try a different approach to verify the package name change impact by searching for any imports or references to either package name in the codebase.
This should help us understand if there are any existing dependencies or references to either package name in the codebase.
Based on the search results, I can now generate a final response:
Package name change appears safe to proceed
The package rename from
@xmtp/client-bindings-wasmto@xmtp/wasm-bindingsappears to be a safe change because:
- The old package name
@xmtp/client-bindings-wasmis not listed as a dependency in any other package.json files in the codebase- References to the package name only appear in files within the
bindings_wasmdirectory itself (package.json, yarn.lock, and CHANGELOG.md)- This appears to be the first official release (v0.0.1) of the package, suggesting it hasn't been published or consumed by other packages yet
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for any references to the old package name in the codebase echo "Searching for old package name references..." rg "@xmtp/client-bindings-wasm" # Check if this package is a dependency in other package.json files echo "Checking dependencies in other package.json files..." fd package.json --exec jq -r '.dependencies | select(."@xmtp/client-bindings-wasm" != null)' fd package.json --exec jq -r '.devDependencies | select(."@xmtp/client-bindings-wasm" != null)'Length of output: 368
Script:
#!/bin/bash # Search for any references to either package name in the codebase echo "Searching for package name references..." rg -l "@xmtp/(client-bindings-wasm|wasm-bindings)" # Look for package.json files and check their content echo "Checking package.json files..." fd package.json --type f --exec cat {}Length of output: 3246
.github/workflows/release-wasm-bindings.yml (1)
13-13: Verify custom runner label.The runner label
warp-ubuntu-latest-x64-16xis not a standard GitHub-hosted runner label.If this is a custom self-hosted runner, please document this requirement. Otherwise, consider using a standard GitHub-hosted runner like
ubuntu-latest.🧰 Tools
🪛 actionlint
13-13: label "warp-ubuntu-latest-x64-16x" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2022", "windows-2019", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-22.04", "ubuntu-20.04", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-14.0", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "macos-13.0", "macos-12-xl", "macos-12-xlarge", "macos-12-large", "macos-12", "macos-12.0", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file
(runner-label)
| - name: Setup node | ||
| uses: actions/setup-node@v4 | ||
| with: | ||
| node-version-file: ".node-version" | ||
| cache-dependency-path: "bindings_wasm/yarn.lock" | ||
| cache: "yarn" | ||
| env: | ||
| SKIP_YARN_COREPACK_CHECK: "1" | ||
|
|
There was a problem hiding this comment.
Add NPM registry configuration for publishing.
The Node.js setup should include registry configuration for publishing:
- name: Setup node
uses: actions/setup-node@v4
with:
node-version-file: ".node-version"
cache-dependency-path: "bindings_wasm/yarn.lock"
cache: "yarn"
+ registry-url: 'https://registry.npmjs.org'
env:
SKIP_YARN_COREPACK_CHECK: "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.
| - name: Setup node | |
| uses: actions/setup-node@v4 | |
| with: | |
| node-version-file: ".node-version" | |
| cache-dependency-path: "bindings_wasm/yarn.lock" | |
| cache: "yarn" | |
| env: | |
| SKIP_YARN_COREPACK_CHECK: "1" | |
| - name: Setup node | |
| uses: actions/setup-node@v4 | |
| with: | |
| node-version-file: ".node-version" | |
| cache-dependency-path: "bindings_wasm/yarn.lock" | |
| cache: "yarn" | |
| registry-url: 'https://registry.npmjs.org' | |
| env: | |
| SKIP_YARN_COREPACK_CHECK: "1" | |
| release: | ||
| permissions: | ||
| id-token: write | ||
| runs-on: warp-ubuntu-latest-x64-16x |
There was a problem hiding this comment.
Update permissions for NPM publishing.
The current permissions block might be insufficient for NPM publishing. Consider adding necessary permissions:
permissions:
id-token: write
+ contents: read
+ packages: write📝 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: | |
| permissions: | |
| id-token: write | |
| runs-on: warp-ubuntu-latest-x64-16x | |
| release: | |
| permissions: | |
| id-token: write | |
| contents: read | |
| packages: write | |
| runs-on: warp-ubuntu-latest-x64-16x |
🧰 Tools
🪛 actionlint
13-13: label "warp-ubuntu-latest-x64-16x" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2022", "windows-2019", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-22.04", "ubuntu-20.04", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-14.0", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "macos-13.0", "macos-12-xl", "macos-12-xlarge", "macos-12-large", "macos-12", "macos-12.0", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file
(runner-label)
| on: | ||
| workflow_dispatch: | ||
|
|
||
| env: | ||
| CARGO_TERM_COLOR: always | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add version control parameters to the workflow trigger.
Consider adding input parameters to control the release version and ensure intentional releases:
on:
workflow_dispatch:
+ inputs:
+ version:
+ description: 'Version to release (e.g., 1.0.0)'
+ required: true
+ dry_run:
+ description: 'Perform a dry run without publishing'
+ type: boolean
+ default: false📝 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.
| on: | |
| workflow_dispatch: | |
| env: | |
| CARGO_TERM_COLOR: always | |
| on: | |
| workflow_dispatch: | |
| inputs: | |
| version: | |
| description: 'Version to release (e.g., 1.0.0)' | |
| required: true | |
| dry_run: | |
| description: 'Perform a dry run without publishing' | |
| type: boolean | |
| default: false | |
| env: | |
| CARGO_TERM_COLOR: always |
| - name: Cache | ||
| uses: Swatinem/rust-cache@v2 | ||
| with: | ||
| workspaces: | | ||
| . | ||
| bindings_wasm | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Optimize Rust caching configuration.
The current cache configuration could be more specific:
- name: Cache
uses: Swatinem/rust-cache@v2
with:
workspaces: |
- .
- bindings_wasm
+ bindings_wasm:target
shared-key: "wasm-bindings"
+ cache-directories: |
+ bindings_wasm/node_modulesCommittable suggestion was skipped due to low confidence.
| - name: Build | ||
| working-directory: bindings_wasm | ||
| run: yarn build | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add build validation step.
Consider adding validation after the build step to ensure the output is correct:
- name: Build
working-directory: bindings_wasm
run: yarn build
+
+ - name: Validate build output
+ working-directory: bindings_wasm
+ run: |
+ if [ ! -f "dist/index.js" ] || [ ! -f "dist/index.d.ts" ]; then
+ echo "Error: Build output files are missing"
+ exit 1
+ fi📝 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.
| - name: Build | |
| working-directory: bindings_wasm | |
| run: yarn build | |
| - name: Build | |
| working-directory: bindings_wasm | |
| run: yarn build | |
| - name: Validate build output | |
| working-directory: bindings_wasm | |
| run: | | |
| if [ ! -f "dist/index.js" ] || [ ! -f "dist/index.d.ts" ]; then | |
| echo "Error: Build output files are missing" | |
| exit 1 | |
| fi |
| - name: Publish to NPM | ||
| uses: JS-DevTools/npm-publish@v3 | ||
| with: | ||
| token: ${{ secrets.NPM_TOKEN }} | ||
| package: bindings_wasm | ||
| env: | ||
| GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance the publishing step with validation and documentation.
The publishing step could be improved with additional checks and documentation:
- name: Publish to NPM
uses: JS-DevTools/npm-publish@v3
with:
token: ${{ secrets.NPM_TOKEN }}
package: bindings_wasm
+ dry-run: ${{ inputs.dry_run }}
+ check-version: true
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+
+ - name: Create GitHub Release
+ if: ${{ !inputs.dry_run }}
+ uses: actions/create-release@v1
+ env:
+ GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+ with:
+ tag_name: wasm-v${{ inputs.version }}
+ release_name: WASM Bindings v${{ inputs.version }}
+ body_path: bindings_wasm/CHANGELOG.md
+ draft: falseCommittable suggestion was skipped due to low confidence.
rygine
force-pushed
the
rygine/wasm-update
branch
from
ad025b3
to
a7bf6a3
Compare
* Update node version * Update .gitignore * Use yarn instead of NPM * Add WASM bindings release workflow * Update build process * Update build command * Add CHANGELOG * Add empty line to
Summary
Summary by CodeRabbit
New Features
@xmtp/wasm-bindingspackage (version 0.0.1).Updates
package.json.Bug Fixes