Skip to content

temp: patch wasm-pack to point to latest binary#5037

Draft
abcxff wants to merge 1 commit into
counter-latency/require-engine-ping-healthfrom
05-11-fix_wasm_point_wasm-pack_build_to_new_wasm-bindgen_repo
Draft

temp: patch wasm-pack to point to latest binary#5037
abcxff wants to merge 1 commit into
counter-latency/require-engine-ping-healthfrom
05-11-fix_wasm_point_wasm-pack_build_to_new_wasm-bindgen_repo

Conversation

@abcxff
Copy link
Copy Markdown
Contributor

@abcxff abcxff commented May 11, 2026

Description

Please include a summary of the changes and the related issue. Please also include relevant motivation and context.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes.

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

@abcxff
Copy link
Copy Markdown
Contributor Author

abcxff commented May 11, 2026

See wasm-bindgen/wasm-pack#1577

@claude
Copy link
Copy Markdown
Contributor

claude Bot commented May 11, 2026

PR #5037 Review: temp: patch wasm-pack to point to latest binary

Overview: This PR makes two distinct changes: (1) adds a pnpm patch for wasm-pack@0.14.0 that corrects the binary download URL, changing the GitHub author field in binary.js from "drager" to "wasm-bindgen"; and (2) disables the build-wasm CI job entirely and removes wasm artifact steps from the publish job.

Issues and Concerns:

Binary URL correctness needs verification. The patch redirects binary downloads to github.com/wasm-bindgen/wasm-pack instead of github.com/drager/wasm-pack. Please confirm that wasm-bindgen/wasm-pack is the correct canonical upstream for v0.14.0 releases before merging, and that the binaries can be trusted. Downloading from a potentially wrong repository without checksum verification is a supply-chain risk.

Disabling build-wasm is contradictory with the patch. If the patch correctly fixes the wasm-pack download URL, the build-wasm CI job should work after applying it. The PR simultaneously adds a fix and disables the thing the fix is supposed to fix, without explaining why. This suggests there is a separate undocumented reason for disabling the job. Either the patch is sufficient and build-wasm should be re-enabled, or the patch is not enough and the PR description should explain what else is broken.

Wasm package will not be published. Removing build-wasm from the publish job needs array means @rivetkit/rivetkit-wasm will not ship in any release while this change is in effect. No fallback is provided for downstream consumers.

Incomplete description. The PR is DRAFT, the template is entirely unfilled, and the title has a temp: prefix. The root cause (why does wasm-pack 0.14.0 use the wrong author string?), the relationship between the two changes, and the follow-up plan to re-enable wasm builds should all be documented before merging.

Code Quality: The pnpm patch is minimal and correctly structured; using patchedDependencies in package.json is the right mechanism for fixing a third-party package bug. The pnpm-lock.yaml changes are correct and consistent with the patch registration. A brief inline comment in publish.yaml explaining why the job is disabled would help future maintainers.

Summary: The patch approach is technically sound, but the PR disables wasm publishing without explaining why the patch alone is not sufficient to restore the build-wasm CI job. The description must be filled in before merging, and the binary URL change should be verified against the canonical wasm-pack upstream to rule out supply-chain risk.

@abcxff abcxff changed the title fix(wasm): point wasm-pack build to new wasm-bindgen repo temp: remove wasm build step May 11, 2026
@abcxff abcxff changed the title temp: remove wasm build step temp: remove wasm publish step May 11, 2026
@abcxff abcxff force-pushed the 05-11-fix_wasm_point_wasm-pack_build_to_new_wasm-bindgen_repo branch from e872ef5 to d644728 Compare May 11, 2026 19:23
@abcxff abcxff changed the title temp: remove wasm publish step temp: patch wasm-pack to point to latest binary May 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant