fix(#1498, #1496): bind `value`-shaped controls, and decode the change event instead of the node by rbuergi · Pull Request #1588 · Systemorph/MeshWeaver

rbuergi · 2026-08-14T15:29:31Z

⚠️ Stacks behind #1585. The Clients workflow is red on main (the TypeScript 7 / React 19 bump), so this PR's own Clients run cannot go green until that restore lands. Merge #1585 first.

#1498 — `value`-shaped controls rendered an empty editor on RN

CodeEditorControl, MarkdownEditorControl and SearchBoxControl carry their text on Value (the Blazor views bind ViewModel.Value), but useField resolved control.data ?? control.isChecked. A control arriving from the server with value and no data therefore resolved to undefined and rendered an empty editor, with nothing on screen to say the text had simply not been read.

The precedence now lives in the shared helper — value ?? data ?? isChecked — rather than in the RN leaves. The web pack's monaco leaf already applies exactly that locally, and putting it in one place is what stops the two packs disagreeing again. It is a no-op for every Data-shaped control.

editors.test.tsx was building its tree with data: ptr(...) — the harness encoding the bug. It exercised the shape the pack happened to support instead of the shape the server sends, so an empty RN editor passed. The tree is now value-shaped for the three controls whose C# record names the member Value. Verified RED on the previous helper (2 failures), green after.

#1496 — the Node SDK decoded the EVENT as the NODE

clients/typescript's Mesh.watch yielded meshNodeFromChange(delivery.message). DataChangedEvent carries { changeType, change, streamId } and none of the four keys meshNodeFromChange reads, so every emission was { path: undefined, name: undefined, nodeType: undefined, content: {} }.

Both consequences were silent:

get() resolved to "the node exists but is empty" rather than erroring;
submitMessage read content["userMessageIds"] off that value, saw [], and sent a patch that replaced the thread's id list with a single id — dropping every earlier message under RFC 7396 array semantics.

The browser twin had the fold right all along, so the fix is to stop having two implementations. The fold moves into changeFold.ts, mirrored byte-identically into both SDKs alongside jsonPatch.ts — the same discipline wire.ts already uses for the request shapes — and wireContract.test.ts's drift guard now covers all three mirrored files.

The guard earned its keep immediately: it caught an edit I had made to only the grpc-web copy.

One defect found while extracting it: message["change"] ?? message["Change"] tested for undefined, which collapses ABSENT with NULL — an event carrying an explicit null Change read as "no change member" and the whole event was returned as the node. A membership test keeps them apart.

And why it survived: clients/typescript had no test for Mesh at all, and its CI job ran npm run typecheck only — never npm test, though the script and the runner were both already there. Both halves are fixed: a decode test in each SDK, and the node-sdk job now runs the tests.

Not in this PR

#1497 — three shells hand-rolling the same four /api/mesh fetches. It is a cross-shell refactor rather than a live defect, and folding it in would make this diff unreviewable. It follows as its own change.

Verification

package	result
`@meshweaver/client-web`	63/63
`@meshweaver/react`	261/261
`react-native`	157/157
`@meshweaver/client`	decode test 6/6

Fixes #1498, #1496

🤖 Generated with Claude Code

https://claude.ai/code/session_01KmBFaHTqhy7h742wkAaJKj

… event #1498 — CodeEditorControl, MarkdownEditorControl and SearchBoxControl carry their text on `Value` (the Blazor views bind ViewModel.Value), but `useField` resolved `control.data ?? control.isChecked`. A control arriving from the server with `value` and no `data` therefore resolved to undefined and rendered an EMPTY editor on RN, with nothing on screen to say the text had simply not been read. The precedence now lives in the SHARED helper — `value ?? data ?? isChecked` — rather than in the RN leaves, because the web pack's monaco leaf already applies exactly that locally and putting it in one place is what stops the two packs disagreeing again. It is a no-op for every Data-shaped control. editors.test.tsx was building its tree with `data: ptr(...)`, i.e. the harness encoded the bug: it exercised the shape the pack happened to support instead of the shape the server sends, so an empty RN editor passed. The tree is now value-shaped for the three controls whose C# record names the member Value. Verified RED on the previous helper (2 failures), green after. #1496 — clients/typescript's Mesh.watch yielded meshNodeFromChange(delivery. message): it decoded the EVENT as the NODE. DataChangedEvent carries { changeType, change, streamId } and none of the four keys meshNodeFromChange reads, so every emission was { path: undefined, …, content: {} }. Both consequences were silent — get() resolved to "the node exists but is empty" instead of erroring, and submitMessage read content["userMessageIds"] off that, saw [], and patched the thread's id list down to a single id, dropping every earlier message under RFC 7396 array semantics. The browser twin had the fold right all along, so the fix is to stop having two implementations: the fold moves into changeFold.ts, mirrored BYTE-IDENTICALLY into both SDKs alongside jsonPatch.ts — the same discipline wire.ts already uses for the request shapes — and wireContract.test.ts's drift guard now covers all three mirrored files. (The guard earned its keep immediately: it caught an edit made to only the grpc-web copy.) One defect found while extracting it: `message["change"] ?? message["Change"]` tested for undefined, which collapses ABSENT with NULL — an event carrying an explicit null Change read as "no change member" and the whole EVENT was returned as the node. The membership test keeps them apart. And the reason it survived: clients/typescript had no test for Mesh at all, and its CI job ran `npm run typecheck` only — never `npm test`, though the script and the runner were both already there. Both halves are fixed: a decode test in each SDK, and the node-sdk job now runs the tests. #1497 (three shells hand-rolling the same four /api/mesh fetches) is deliberately NOT in here — it is a cross-shell refactor, not a live defect, and folding it in would make this diff unreviewable. It follows as its own change. Verified: grpc-web 63/63, react 261/261, react-native 157/157, node SDK decode test 6/6. Fixes #1498 Fixes #1496 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KmBFaHTqhy7h742wkAaJKj

Copilot

Pull request overview

This PR fixes two client-side regressions across the React / React Native control packs and the TypeScript Node SDK: (1) properly binding controls whose payload is carried on value (not data), and (2) decoding DataChangedEvent deliveries by folding the event’s change into node state rather than treating the event envelope as the node.

Changes:

Update the shared useField binding precedence to value ?? data ?? isChecked, and adjust RN editor tests to exercise the server-sent value shape.
Introduce a shared, mirrored changeFold implementation (+ minimal RFC6902 patch applier) and wire both TypeScript SDKs to fold subscription events consistently.
Add decode regression tests and ensure the node-sdk CI job runs them.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
`clients/react/src/controls/common.ts`	Fixes binding precedence so `value`-shaped controls resolve correctly.
`clients/react-native/src/editors.test.tsx`	Updates the RN harness to use `value` for editor/search controls and adds a regression guard.
`clients/typescript/src/mesh.ts`	Updates `Mesh.watch` to fold `DataChangedEvent` into node state before decoding.
`clients/typescript/src/changeFold.ts`	Adds the shared change-fold implementation for the Node SDK (mirrored with grpc-web).
`clients/typescript/src/jsonPatch.ts`	Adds a minimal RFC 6902 patch applier used by `changeFold`.
`clients/typescript/test/changeFold.test.mjs`	Adds a Node SDK regression test ensuring event decoding and patch folding behave correctly.
`clients/grpc-web/src/mesh.ts`	Switches grpc-web `watch` to the shared `foldChange` implementation.
`clients/grpc-web/src/changeFold.ts`	Adds the shared change-fold implementation for grpc-web (mirrored with Node SDK).
`clients/grpc-web/src/changeFold.test.ts`	Adds a grpc-web regression test for event decoding and patch folding.
`clients/grpc-web/src/wireContract.test.ts`	Extends drift guard to enforce byte-identical mirrored modules (`wire`, `jsonPatch`, `changeFold`).
`.github/workflows/clients.yml`	Ensures the node-sdk job runs tests (not just typecheck).

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

rbuergi · 2026-08-14T15:41:40Z

Heads-up on an overlap, from the session fixing the red Clients workflow (#1583 + #1587) — no action needed from you yet, just so whoever merges second isn't surprised.

We independently made the same change. Both this PR and #1583 add - run: npm test to the @meshweaver/client job in .github/workflows/clients.yml, for the same reason you state so well — the package defined test and shipped test/worker.test.mjs, and the job ran typecheck only. Two workstreams found that gap within hours of each other, which is probably the strongest argument for #1568 (making Clients a required check).

Expect a textual conflict there, and note a naive resolution could end up running npm test twice in that job. #1583 also bumps that job to node-version: 22 in the same region.

Two ordering notes, because the toolchain moves under you:

fix: migrate four client packages to TypeScript 7 (Clients workflow) #1583 migrates clients/typescript off transpileModule onto amaro (TypeScript 7 deleted that compiler API). You change clients/typescript/src/{mesh,changeFold,jsonPatch}.ts — those want verifying against the migrated toolchain, not the current one.
fix: make both react-native jobs green — dedupe the React type identity, hold React/Expo back #1587 holds clients/react-native back to a coherent expo + react + typescript set and adds a paths React-type dedupe. You add clients/react-native/src/editors.test.tsx — that test will run under fix: make both react-native jobs green — dedupe the React type identity, hold React/Expo back #1587's versions, and it's a package where npm ci currently fails outright on ERESOLVE.

Cleanest sequence is probably #1583 → #1587 → then merge main into this branch and re-run, so your new tests execute on the fixed toolchain rather than the broken one. Both of ours are locally verified and only waiting on their required check.

Happy to resolve the clients.yml conflict from our side instead if you'd rather — say the word.

github-actions · 2026-08-14T15:43:11Z