From 1969b3255fc769937cb1aa0ce7e5f631b398f366 Mon Sep 17 00:00:00 2001 From: hhimanshu <6589036+hhimanshu@users.noreply.github.com> Date: Tue, 1 Sep 2026 19:11:17 +1200 Subject: [PATCH] feat(wasm-workbook): expose removeName on the JsWorkbook binding MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `Workbook::remove_name` has existed in Rust since named-range CRUD landed, but the wasm binding only exposed `defineName`/`redefineName` — a JS consumer could define and redefine a name but never remove one. Deleting a named range from a JS-facing document model left the engine still resolving the old name: a stale value with no error, until a full rebuild forced a resync. `removeName(name)` delegates to `Workbook::remove_name`, which returns `Option` and never fails (removing an unknown name is an intentional no-op), so the binding returns nothing rather than forcing an artificial `Result<(), JsError>` with a dead error arm — matching `clear()`'s existing silent-no-op convention rather than `defineName`/`redefineName`'s (which really can fail). The generated `.d.ts` still types it `removeName(name: string): void`, identical in shape to its two siblings. Adds `crates/wasm-workbook/tests/wasm_surface.rs`, gated to `wasm32` and run via `wasm-pack test --node`, mirroring `crates/wasm/tests/wasm_surface.rs`. It defines a name, uses it in a formula, removes it through the actual binding, recalcs, and asserts the formula now resolves to `#NAME?` instead of the stale value. Confirmed the test does not compile against the pre-fix binding (no such method) and passes after. closes #973 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01HAt4Keq1m4fEyHPmPjHSJ7 --- crates/wasm-workbook/README.jsr.md | 2 +- crates/wasm-workbook/src/lib.rs | 7 +++ crates/wasm-workbook/tests/wasm_surface.rs | 63 ++++++++++++++++++++++ 3 files changed, 71 insertions(+), 1 deletion(-) create mode 100644 crates/wasm-workbook/tests/wasm_surface.rs diff --git a/crates/wasm-workbook/README.jsr.md b/crates/wasm-workbook/README.jsr.md index 33ac9ef0e0..20de6a98cf 100644 --- a/crates/wasm-workbook/README.jsr.md +++ b/crates/wasm-workbook/README.jsr.md @@ -56,7 +56,7 @@ integration, so Deno instantiates the WebAssembly as part of the module graph | `clear(sheet, a1)` | Clear a cell | | `recalc(contextJson)` | Recalculate in dependency order; returns changes as a JSON string. The context must supply `timestamp_ms`, `timezone` and `rng_seed` — all three, or it throws | | `resolved(sheet, a1)` | The computed value of a cell, as a JSON string | -| `defineName(name, ref)` / `redefineName(name, ref)` | Named ranges | +| `defineName(name, ref)` / `redefineName(name, ref)` / `removeName(name)` | Named ranges | | `precedentsOf(sheet, a1, maxDepth?, maxNodes?)` | What a cell reads — cells, ranges, names and unresolved refs | | `dependentsOf(sheet, a1, maxDepth?, maxNodes?)` | What reads a cell, i.e. what breaks if you change it | | `toJSON()` / `JsWorkbook.fromJSON(s)` | Serialise and restore | diff --git a/crates/wasm-workbook/src/lib.rs b/crates/wasm-workbook/src/lib.rs index 9a3ac30f62..a1570ebe4f 100644 --- a/crates/wasm-workbook/src/lib.rs +++ b/crates/wasm-workbook/src/lib.rs @@ -237,6 +237,13 @@ impl JsWorkbook { .map_err(|e| JsError::new(&e.to_string())) } + /// Removes a workbook-scoped named range, if one exists. A `name` that + /// does not exist is a silent no-op, matching [`clear`](Self::clear). + #[wasm_bindgen(js_name = removeName)] + pub fn remove_name(&mut self, name: &str) { + self.inner.remove_name(name); + } + /// Defines a workbook-scoped table (issue #868): `ref_str`'s first row /// becomes the table's header row, so formulas can use `Table[Column]` /// (whole-column) and `Table[@Column]` / unqualified `[@Column]` diff --git a/crates/wasm-workbook/tests/wasm_surface.rs b/crates/wasm-workbook/tests/wasm_surface.rs new file mode 100644 index 0000000000..bcd1df8888 --- /dev/null +++ b/crates/wasm-workbook/tests/wasm_surface.rs @@ -0,0 +1,63 @@ +//! End-to-end WASM-surface tests (issue #973), exercising the real +//! wasm-bindgen ABI. Run with `wasm-pack test --node crates/wasm-workbook` +//! (or `--headless --chrome`). +//! +//! Gated to `wasm32` so the native `cargo nextest` run (CI's test job) skips +//! it; the native shape coverage for `JsWorkbook`'s other methods lives in +//! `round_trip.rs`, `table_bindings.rs` and `dependency_graph.rs`, which test +//! through `truecalc_workbook::Workbook` directly because a `JsValue`-touching +//! `JsWorkbook` method aborts when called outside a real wasm runtime. CI +//! builds the wasm package via `wasm-pack build` but does not currently run +//! `wasm-pack test`, so these are developer-facing checks of the live ABI — +//! see `crates/wasm/tests/wasm_surface.rs` for the same pattern on the +//! calc-only binding. +#![cfg(target_arch = "wasm32")] + +use truecalc_wasm_workbook::JsWorkbook; +use wasm_bindgen_test::*; + +/// Parses `resolved()`'s tagged-JSON string into a `serde_json::Value` for +/// field-level assertions — robust to the untested key order `serde_json`'s +/// default (non-`preserve_order`) map emits. +fn resolved_json(wb: &JsWorkbook, sheet: &str, a1: &str) -> serde_json::Value { + let s = wb + .resolved(sheet, a1) + .unwrap() + .as_string() + .expect("resolved() returns a JSON string for a non-empty cell"); + serde_json::from_str(&s).expect("resolved() returns valid JSON") +} + +/// `removeName` (issue #973) is missing from the JS surface entirely before +/// this change — this test does not compile against the pre-fix ABI. Once it +/// exists: defining a name, referencing it in a formula, then removing the +/// name through the binding and recalculating must turn the formula's stale +/// resolved value into `#NAME?`, the same way deleting the name and +/// rebuilding the workbook from scratch already does. +#[wasm_bindgen_test] +fn remove_name_through_binding_invalidates_dependent_formula() { + let mut wb = JsWorkbook::new("sheets"); + wb.add_sheet("Sheet1").unwrap(); + wb.set("Sheet1", "B1", "10").unwrap(); + wb.set("Sheet1", "B2", "20").unwrap(); + wb.define_name("Total", "Sheet1!B1:B2").unwrap(); + wb.set("Sheet1", "A1", "=SUM(Total)").unwrap(); + + wb.recalc(r#"{"timestamp_ms":0,"timezone":"UTC","rng_seed":0}"#) + .unwrap(); + let before = resolved_json(&wb, "Sheet1", "A1"); + assert_eq!(before["type"], "number", "sanity check: {before}"); + assert_eq!(before["value"], 30.0, "sanity check: {before}"); + + wb.remove_name("Total"); + wb.recalc(r#"{"timestamp_ms":0,"timezone":"UTC","rng_seed":0}"#) + .unwrap(); + + let after = resolved_json(&wb, "Sheet1", "A1"); + assert_eq!( + after["type"], "error", + "the removed name must no longer resolve — a stale `30` here is \ + exactly the silent-wrong-value bug issue #973 reports: {after}" + ); + assert_eq!(after["error"], "#NAME?", "got {after}"); +}