Conversation
WalkthroughAdds a new Changes
Sequence Diagram(s)(omitted — change is configuration-only and does not introduce multi-component control flow) Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
Use an overlay to make pkgs.wasm-bindgen-cli point to pkgs.wasm-bindgen-cli_0_2_100. This ensures all consumers of rainix.pkgs get the pinned version. The latest nixpkgs has wasm-bindgen-cli 0.2.108 which is incompatible with the rain.wasm dependency that requires 0.2.100.
findolor
force-pushed
the
2025-01-22-pin-wasm-bindgen-0.2.100
branch
from
188c660 to
b31ea48
Compare
![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
🤖 Fix all issues with AI agents
In `@flake.nix`:
- Around line 17-20: Add an explicit guard around the use of
wasm-bindgen-cli_0_2_100 inside the wasm-bindgen-overlay so nix evaluation fails
with a clear message if that attribute is missing; update the
wasm-bindgen-overlay (referencing the symbol wasm-bindgen-overlay and the
attribute wasm-bindgen-cli_0_2_100) to check for prev.wasm-bindgen-cli_0_2_100
and throw or call builtins.error with a descriptive message if it is
null/undefined, and adjust the overlays list (the overlays binding where
rust-overlay, foundry.overlay, solc.overlay, wasm-bindgen-overlay are combined)
accordingly; also verify the currently pinned nixpkgs revision in flake.lock to
confirm wasm-bindgen-cli_0_2_100 exists for all target systems and document that
check in the commit message.
| wasm-bindgen-overlay = final: prev: { | ||
| wasm-bindgen-cli = prev.wasm-bindgen-cli_0_2_100; | ||
| }; | ||
| overlays = [ (import rust-overlay) foundry.overlay solc.overlay wasm-bindgen-overlay ]; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Add an explicit guard if wasm-bindgen-cli_0_2_100 disappears.
Because the nixpkgs input is floating, a future nix flake update could drop or rename the versioned attribute and fail with a confusing evaluation error. Consider a clear guard/throw so failures are explicit.
Please verify in the currently pinned nixpkgs revision (flake.lock) that wasm-bindgen-cli_0_2_100 exists for all target systems.
♻️ Proposed guard for clearer failure
- wasm-bindgen-overlay = final: prev: {
- wasm-bindgen-cli = prev.wasm-bindgen-cli_0_2_100;
- };
+ wasm-bindgen-overlay = final: prev: {
+ wasm-bindgen-cli =
+ if prev ? wasm-bindgen-cli_0_2_100
+ then prev.wasm-bindgen-cli_0_2_100
+ else throw "nixpkgs missing wasm-bindgen-cli_0_2_100; pin nixpkgs or update rain.wasm";
+ };📝 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.
| wasm-bindgen-overlay = final: prev: { | |
| wasm-bindgen-cli = prev.wasm-bindgen-cli_0_2_100; | |
| }; | |
| overlays = [ (import rust-overlay) foundry.overlay solc.overlay wasm-bindgen-overlay ]; | |
| wasm-bindgen-overlay = final: prev: { | |
| wasm-bindgen-cli = | |
| if prev ? wasm-bindgen-cli_0_2_100 | |
| then prev.wasm-bindgen-cli_0_2_100 | |
| else throw "nixpkgs missing wasm-bindgen-cli_0_2_100; pin nixpkgs or update rain.wasm"; | |
| }; | |
| overlays = [ (import rust-overlay) foundry.overlay solc.overlay wasm-bindgen-overlay ]; |
🤖 Prompt for AI Agents
In `@flake.nix` around lines 17 - 20, Add an explicit guard around the use of
wasm-bindgen-cli_0_2_100 inside the wasm-bindgen-overlay so nix evaluation fails
with a clear message if that attribute is missing; update the
wasm-bindgen-overlay (referencing the symbol wasm-bindgen-overlay and the
attribute wasm-bindgen-cli_0_2_100) to check for prev.wasm-bindgen-cli_0_2_100
and throw or call builtins.error with a descriptive message if it is
null/undefined, and adjust the overlays list (the overlays binding where
rust-overlay, foundry.overlay, solc.overlay, wasm-bindgen-overlay are combined)
accordingly; also verify the currently pinned nixpkgs revision in flake.lock to
confirm wasm-bindgen-cli_0_2_100 exists for all target systems and document that
check in the commit message.
There was a problem hiding this comment.
0.2.100 works and we have no need to track latest. If a future nixpkgs removes this attribute, we'll deal with it then
There was a problem hiding this comment.
@findolor, understood! I'll note your preference for handling nixpkgs attribute changes reactively. The pinned version meets your current needs, and you're comfortable addressing any future nixpkgs changes if they arise.
✏️ Learnings added
Learnt from: findolor
Repo: rainlanguage/rainix PR: 101
File: flake.nix:17-20
Timestamp: 2026-01-22T13:45:52.516Z
Learning: In the rainix project, the team prefers to handle nixpkgs versioned attribute removals reactively rather than adding defensive guards. They don't need to track latest versions if a pinned version works for their use case.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
|
@coderabbitai assess this PR size classification for the totality of the PR with the following criterias and report it in your comment: S/M/L PR Classification Guidelines:This guide helps classify merged pull requests by effort and complexity rather than just line count. The goal is to assess the difficulty and scope of changes after they have been completed. Small (S)Characteristics:
Review Effort: Would have taken 5-10 minutes Examples:
Medium (M)Characteristics:
Review Effort: Would have taken 15-30 minutes Examples:
Large (L)Characteristics:
Review Effort: Would have taken 45+ minutes Examples:
Additional Factors to ConsiderWhen deciding between sizes, also consider:
Notes:
|
|
Tip For best results, initiate chat on the files or code changes. SIZE=S 🧠 Learnings used |
3 participants
Motivation
The recent
nix flake updatein rain.orderbook brought in nixpkgs with wasm-bindgen-cli 0.2.108, which is incompatible with the rain.wasm dependency that uses wasm-bindgen 0.2.100. The TryFromJsValue trait API changed between these versions, breaking wasm tests.Solution
Pin wasm-bindgen-cli to version 0.2.100 using the versioned package
pkgs.wasm-bindgen-cli_0_2_100that already exists in nixpkgs.Checks
By submitting this for review, I'm confirming I've done the following:
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.