From 7ee931e20aba6860f1b359aa5f596129484229b9 Mon Sep 17 00:00:00 2001 From: Paul Date: Tue, 28 Jul 2026 11:37:22 +0700 Subject: [PATCH] fix(hydrogen): handle deferred loader payloads safely --- .../README.md | 29 +++ .../plan.md | 80 +++++++ .../__tests__/is-same-loader-payload.test.ts | 215 ++++++++++++++++++ .../__tests__/sync-reused-instance.test.ts | 135 +++++++++++ .../src/utils/is-same-loader-payload.ts | 187 +++++++++++++++ .../src/utils/sync-reused-instance.ts | 19 +- 6 files changed, 657 insertions(+), 8 deletions(-) create mode 100644 .specs/2026-07-28--511-cycle-safe-reused-instance/README.md create mode 100644 .specs/2026-07-28--511-cycle-safe-reused-instance/plan.md create mode 100644 packages/hydrogen/__tests__/is-same-loader-payload.test.ts create mode 100644 packages/hydrogen/src/utils/is-same-loader-payload.ts diff --git a/.specs/2026-07-28--511-cycle-safe-reused-instance/README.md b/.specs/2026-07-28--511-cycle-safe-reused-instance/README.md new file mode 100644 index 00000000..f6da19b4 --- /dev/null +++ b/.specs/2026-07-28--511-cycle-safe-reused-instance/README.md @@ -0,0 +1,29 @@ +# Feature: Cycle-safe, identity-aware reused-instance sync + +| Field | Value | +| ---------------- | ------------------------------------------------------- | +| **Status** | completed | +| **Owner** | @paul | +| **Issue** | [#511](https://github.com/Weaverse/weaverse/issues/511) | +| **Branch** | `fix/511-cycle-safe-reused-instance` | +| **Created** | 2026-07-28 | +| **Last Updated** | 2026-07-28 | + +## Original Prompt + +> Analyze and fix https://github.com/Weaverse/weaverse/issues/511 so reused +> Hydrogen instances safely adopt deferred loader data during same-URL +> revalidation without render-time crashes or stale Promise-backed context. + +## Summary + +`syncReusedInstance()` compared loader payloads with bare `JSON.stringify()` +during render. `dataContext` carries live route loader data, so deferred values +are still unresolved `Promise`s: React 19 development builds attach an +enumerable, self-referencing `_debugInfo` to them and stringifying throws +`TypeError: Converting circular structure to JSON`. In production the same call +silently returns `{}` for every promise, so a *fresh* deferred value compares +equal to the previous one and the reused instance keeps serving stale context. +This spec replaces that comparison with `isSameLoaderPayload()`, a structural +comparison that treats promises/thenables and other opaque objects as atomic +identity-compared values and handles cyclic graphs coinductively. diff --git a/.specs/2026-07-28--511-cycle-safe-reused-instance/plan.md b/.specs/2026-07-28--511-cycle-safe-reused-instance/plan.md new file mode 100644 index 00000000..4eec1b1e --- /dev/null +++ b/.specs/2026-07-28--511-cycle-safe-reused-instance/plan.md @@ -0,0 +1,80 @@ +# Plan: cycle-safe, identity-aware reused-instance sync + +## Problem + +`syncReusedInstance()` runs during render on the instance-reuse branch +(same-URL revalidation in live/preview mode) and compared payloads with: + +```ts +JSON.stringify(weaverse.dataContext ?? null) !== + JSON.stringify(params.dataContext ?? null) +``` + +`createWeaverseDataContext()` copies each route match's **raw** `data` into +`dataContext`, so deferred loader values are still unresolved `Promise`s. + +Two distinct defects follow from one root cause — *JSON serialization is the +wrong equality model for a live, unserialized object graph*: + +1. **Crash (development).** React 19 attaches an enumerable, self-referencing + `_debugInfo` to deferred promises. `JSON.stringify` throws + `TypeError: Converting circular structure to JSON` at render time. +2. **Stale context (production).** Without that metadata, every promise + stringifies to `{}`. A fresh deferred value therefore compares *equal* to + the previous render's promise, `contextChanged` stays `false`, and the + reused instance keeps serving a settled/abandoned promise. + +The issue's proposed `'[[promise]]'` token fixes only (1). Verified: two +distinct promises produce identical output under both bare JSON and the +token form, so (2) survives. A blanket `try`/`catch` is also rejected — it +would treat every render as changed and call `triggerUpdate()` in a loop. + +## Approach + +Add `isSameLoaderPayload(left, right)` and use it for both comparisons. +`syncReusedInstance`'s control flow (assignment order, item notification, +`triggerUpdate`) is deliberately unchanged — only the equality test is swapped. + +Comparison semantics, chosen deliberately: + +| Case | Behavior | Why | +| --- | --- | --- | +| Promises / thenables | atomic, identity-compared | fresh promise = change; debug metadata never traversed | +| Opaque objects (`Map`, `Set`, `URL`, class instances, cross-realm promises) | atomic, identity-compared | JSON flattens all to `{}`, collapsing distinct async values | +| `toJSON()` bearers (`Date`) | compared via projection | matches wire behavior | +| Cycles | coinductive (assume equal on re-entry) | equal cycles equal, differing leaf still a change, never throws | +| `undefined`/function/symbol | omitted in objects, `null` in arrays | matches `JSON.stringify` | +| NaN / ±Infinity | normalized to `null` | matches `JSON.stringify` | +| Key order | irrelevant | removes the old "false mismatch costs a re-render" caveat | + +Thenables are duck-typed (`typeof value.then === 'function'`) rather than +`instanceof Promise` so cross-realm promises stay atomic. + +Cycle tracking is scoped to the **active comparison path** (a flat +`[a0,b0,a1,b1,...]` array), not every visited pair: memory stays proportional +to graph depth and the common all-equal walk allocates one array. Memoizing +equal pairs instead measured ~2x slower on a realistic 400-item page, which is +the case that runs on every render. No new dependency. + +## Files touched + +| File | Change | +| --- | --- | +| `packages/hydrogen/src/utils/is-same-loader-payload.ts` | new — the comparison + rationale | +| `packages/hydrogen/src/utils/sync-reused-instance.ts` | use it for both comparisons; refresh doc comment | +| `packages/hydrogen/__tests__/is-same-loader-payload.test.ts` | new — 22 semantic tests | +| `packages/hydrogen/__tests__/sync-reused-instance.test.ts` | +8 tests: circular promise, fresh identity, repeat sync, cycles, null/undefined, no double-notify | + +Internal module only: not exported from `src/utils/index.ts` or `src/index.ts`, +so the public API surface is unchanged (`api:check` passes clean). + +## Verification + +- RED first: 6 failed / 4 passed with the exact `Converting circular structure + to JSON` trace at `sync-reused-instance.ts:31`. +- GREEN: 34 focused tests; `@weaverse/hydrogen` 203 tests; repo test, typecheck, + Biome, build, and `api:check` all clean. +- Mutation-tested: 8 mutants against the final implementation, all killed + (one surviving mutant exposed dead code, which was removed). +- Differential parity harness vs `JSON.stringify` across 12 edge cases; the only + intended divergence is that cyclic input does not throw. diff --git a/packages/hydrogen/__tests__/is-same-loader-payload.test.ts b/packages/hydrogen/__tests__/is-same-loader-payload.test.ts new file mode 100644 index 00000000..d6829a74 --- /dev/null +++ b/packages/hydrogen/__tests__/is-same-loader-payload.test.ts @@ -0,0 +1,215 @@ +import { describe, expect, it } from 'vitest' +import { isSameLoaderPayload } from '../src/utils/is-same-loader-payload' + +/** Self-referencing route data: `root.shop.parent === root`. */ +function makeSelfCycle(shopName: string) { + let shop: Record = { name: shopName } + let root: Record = { shop } + shop.parent = root + return root +} + +describe('isSameLoaderPayload', () => { + it('should_report_equal_when_plain_payloads_match_with_different_key_order', () => { + let left = { id: 'page-1', items: [{ id: 'item-1', index: 0 }] } + let right = { items: [{ index: 0, id: 'item-1' }], id: 'page-1' } + + let result = isSameLoaderPayload(left, right) + + expect(result).toBe(true) + }) + + it('should_report_changed_when_a_nested_leaf_differs', () => { + let left = { page: { items: [{ id: 'item-1' }] } } + let right = { page: { items: [{ id: 'item-2' }] } } + + let result = isSameLoaderPayload(left, right) + + expect(result).toBe(false) + }) + + it('should_report_changed_when_array_lengths_differ', () => { + let left = { items: [1, 2] } + let right = { items: [1, 2, 3] } + + let result = isSameLoaderPayload(left, right) + + expect(result).toBe(false) + }) + + it('should_report_equal_when_arrays_hold_json_omitted_holes_in_the_same_slots', () => { + // JSON.stringify serializes both as `[null]`. + let result = isSameLoaderPayload([undefined], [() => undefined]) + + expect(result).toBe(true) + }) + + it('should_report_equal_when_only_json_omitted_properties_differ', () => { + let left = { name: 'Weaverse', onSelect: () => undefined } + let right = { name: 'Weaverse', missing: undefined } + + let result = isSameLoaderPayload(left, right) + + expect(result).toBe(true) + }) + + it('should_report_changed_when_the_right_side_adds_a_json_visible_key', () => { + let left = { name: 'Weaverse' } + let right = { name: 'Weaverse', locale: 'en' } + + let result = isSameLoaderPayload(left, right) + + expect(result).toBe(false) + }) + + it('should_report_equal_when_a_non_finite_number_faces_the_null_it_serializes_to', () => { + // A revalidated wire payload carries `null` where the in-memory value was + // NaN/Infinity; treating that as a change would re-render every render. + let result = isSameLoaderPayload({ ratio: Number.NaN }, { ratio: null }) + + expect(result).toBe(true) + }) + + it('should_report_equal_when_dates_carry_the_same_instant', () => { + let left = { publishedAt: new Date('2026-07-28T00:00:00.000Z') } + let right = { publishedAt: new Date('2026-07-28T00:00:00.000Z') } + + let result = isSameLoaderPayload(left, right) + + expect(result).toBe(true) + }) + + it('should_report_changed_when_a_date_moves_to_another_instant', () => { + let left = { publishedAt: new Date('2026-07-28T00:00:00.000Z') } + let right = { publishedAt: new Date('2026-07-29T00:00:00.000Z') } + + let result = isSameLoaderPayload(left, right) + + expect(result).toBe(false) + }) + + it('should_report_equal_when_the_same_promise_identity_is_reused', () => { + let deferred = Promise.resolve({ colors: [] }) + + let result = isSameLoaderPayload( + { root: { swatchesConfigs: deferred } }, + { root: { swatchesConfigs: deferred } } + ) + + expect(result).toBe(true) + }) + + it('should_report_changed_when_a_distinct_promise_resolves_to_equal_data', () => { + // Both stringify as `{}` — identity is the only signal that the loader + // handed us a fresh deferred value. + let result = isSameLoaderPayload( + { root: { swatchesConfigs: Promise.resolve({ colors: [] }) } }, + { root: { swatchesConfigs: Promise.resolve({ colors: [] }) } } + ) + + expect(result).toBe(false) + }) + + it('should_report_changed_when_distinct_thenables_stand_in_for_promises', () => { + // Cross-realm promises and hand-rolled thenables fail `instanceof Promise`; + // duck-typing keeps them atomic instead of collapsing them to `{}`. + // biome-ignore-start lint/suspicious/noThenProperty: exercising the thenable detection path requires a literal `then` + let result = isSameLoaderPayload( + { deferred: { then: () => undefined } }, + { deferred: { then: () => undefined } } + ) + // biome-ignore-end lint/suspicious/noThenProperty: see above + + expect(result).toBe(false) + }) + + it('should_report_changed_when_distinct_opaque_objects_hold_equal_entries', () => { + let result = isSameLoaderPayload( + { index: new Map([['a', 1]]) }, + { index: new Map([['a', 1]]) } + ) + + expect(result).toBe(false) + }) + + it('should_not_descend_into_react_debug_metadata_on_a_reused_promise', () => { + let deferred = Promise.resolve({ colors: [] }) + let debugEntry: Record = { awaited: {} } + ;(debugEntry.awaited as Record).value = deferred + ;(deferred as unknown as Record)._debugInfo = [debugEntry] + + let result = isSameLoaderPayload({ root: deferred }, { root: deferred }) + + expect(result).toBe(true) + }) + + it('should_report_equal_when_self_cycles_have_equal_leaves', () => { + let result = isSameLoaderPayload( + makeSelfCycle('Weaverse'), + makeSelfCycle('Weaverse') + ) + + expect(result).toBe(true) + }) + + it('should_report_changed_when_self_cycles_differ_at_a_leaf', () => { + let result = isSameLoaderPayload( + makeSelfCycle('Weaverse'), + makeSelfCycle('Pilot') + ) + + expect(result).toBe(false) + }) + + it('should_report_equal_when_a_self_cycle_is_bisimilar_to_a_mutual_cycle', () => { + let single: Record = {} + single.next = single + let first: Record = {} + let second: Record = { next: first } + first.next = second + + let result = isSameLoaderPayload(single, first) + + expect(result).toBe(true) + }) + + it('should_report_changed_when_a_cycle_faces_a_finite_chain', () => { + let cyclic: Record = {} + cyclic.next = cyclic + let finite = { next: { next: null } } + + let result = isSameLoaderPayload(cyclic, finite) + + expect(result).toBe(false) + }) + + it('should_report_equal_when_repeated_aliases_face_distinct_equal_objects', () => { + // Aliasing is not observable through JSON, so it is not a change signal. + let shared = { name: 'Weaverse' } + + let result = isSameLoaderPayload( + { a: shared, b: shared }, + { a: { name: 'Weaverse' }, b: { name: 'Weaverse' } } + ) + + expect(result).toBe(true) + }) + + it('should_report_changed_when_an_array_faces_a_plain_object', () => { + let result = isSameLoaderPayload({ items: [] }, { items: {} }) + + expect(result).toBe(false) + }) + + it('should_report_equal_when_both_sides_are_null', () => { + let result = isSameLoaderPayload(null, null) + + expect(result).toBe(true) + }) + + it('should_report_changed_when_null_faces_an_empty_object', () => { + let result = isSameLoaderPayload(null, {}) + + expect(result).toBe(false) + }) +}) diff --git a/packages/hydrogen/__tests__/sync-reused-instance.test.ts b/packages/hydrogen/__tests__/sync-reused-instance.test.ts index 173cf1d7..4d5777f2 100644 --- a/packages/hydrogen/__tests__/sync-reused-instance.test.ts +++ b/packages/hydrogen/__tests__/sync-reused-instance.test.ts @@ -48,6 +48,26 @@ function sync(instance: InstanceStub, params: WeaverseHydrogenParams) { syncReusedInstance(instance as unknown as WeaverseHydrogen, params) } +/** + * Deferred loader promise as React 19 development builds hand it to us: + * `_debugInfo` is enumerable and closes a cycle back onto the promise. + */ +function makeReactDeferredPromise(value: unknown): Promise { + let promise = Promise.resolve(value) + let debugEntry: Record = { awaited: {} } + ;(debugEntry.awaited as Record).value = promise + ;(promise as unknown as Record)._debugInfo = [debugEntry] + return promise +} + +/** Route data whose nested objects cross-link back to their parent. */ +function makeCyclicRouteData(shopName: string) { + let shop: Record = { name: shopName } + let root: Record = { shop } + shop.parent = root + return root +} + describe('syncReusedInstance', () => { it('should_apply_fresh_page_data_and_rerender_when_items_changed', () => { let instance = makeInstance() @@ -107,4 +127,119 @@ describe('syncReusedInstance', () => { expect(instance.itemInstances.get('item-1')?.setData).not.toHaveBeenCalled() expect(instance.triggerUpdate).not.toHaveBeenCalled() }) + + it('should_not_rerender_when_deferred_promise_identity_is_unchanged', () => { + let deferred = makeReactDeferredPromise({ colors: [] }) + let instance = makeInstance() + instance.dataContext = { root: { swatchesConfigs: deferred } } + let params = makeParams({ + dataContext: { root: { swatchesConfigs: deferred } }, + }) + + sync(instance, params) + + expect(instance.triggerUpdate).not.toHaveBeenCalled() + }) + + it('should_assign_fresh_context_when_deferred_promise_identity_changed', () => { + let instance = makeInstance() + instance.dataContext = { + root: { swatchesConfigs: makeReactDeferredPromise({ colors: [] }) }, + } + let params = makeParams({ + dataContext: { + root: { swatchesConfigs: makeReactDeferredPromise({ colors: [] }) }, + }, + }) + + sync(instance, params) + + expect(instance.dataContext).toBe(params.dataContext) + }) + + it('should_notify_item_stores_when_deferred_promise_identity_changed', () => { + let instance = makeInstance() + instance.dataContext = { + root: { swatchesConfigs: makeReactDeferredPromise({ colors: [] }) }, + } + let params = makeParams({ + dataContext: { + root: { swatchesConfigs: makeReactDeferredPromise({ colors: [] }) }, + }, + }) + + sync(instance, params) + + expect(instance.itemInstances.get('item-1')?.setData).toHaveBeenCalledWith( + {} + ) + expect(instance.triggerUpdate).toHaveBeenCalledTimes(1) + }) + + it('should_not_rerender_again_when_synced_twice_with_the_same_params', () => { + let instance = makeInstance() + instance.dataContext = { + root: { swatchesConfigs: makeReactDeferredPromise({ colors: [] }) }, + } + let params = makeParams({ + dataContext: { + root: { swatchesConfigs: makeReactDeferredPromise({ colors: [] }) }, + }, + }) + + sync(instance, params) + sync(instance, params) + + expect(instance.triggerUpdate).toHaveBeenCalledTimes(1) + }) + + it('should_not_rerender_when_cyclic_context_graphs_are_structurally_equal', () => { + let instance = makeInstance() + instance.dataContext = makeCyclicRouteData('Weaverse') + let params = makeParams({ dataContext: makeCyclicRouteData('Weaverse') }) + + sync(instance, params) + + expect(instance.triggerUpdate).not.toHaveBeenCalled() + }) + + it('should_rerender_when_cyclic_context_graphs_differ_at_a_leaf', () => { + let instance = makeInstance() + instance.dataContext = makeCyclicRouteData('Weaverse') + let params = makeParams({ dataContext: makeCyclicRouteData('Pilot') }) + + sync(instance, params) + + expect(instance.triggerUpdate).toHaveBeenCalledTimes(1) + }) + + it('should_not_rerender_when_absent_context_stays_absent', () => { + // `undefined` params context and a stored `null` are the same "no context" + // state — normalization must not manufacture a change. + let instance = makeInstance() + instance.dataContext = null + let params = makeParams({}) + ;(params as { dataContext?: unknown }).dataContext = undefined + + sync(instance, params) + + expect(instance.triggerUpdate).not.toHaveBeenCalled() + }) + + it('should_not_notify_items_twice_when_data_and_context_both_changed', () => { + // `setProjectData` already refreshes every item store; an extra + // `setData({})` pass would double-notify subscribers in one render. + let instance = makeInstance() + let params = makeParams({ + data: { id: 'page-1', rootId: 'root', items: [{ id: 'item-1' }] }, + dataContext: { cartCount: 7 }, + }) + params.data.items.push({ id: 'item-2' }) + + sync(instance, params) + + expect(instance.itemInstances.get('item-1')?.setData).not.toHaveBeenCalled() + expect(instance.setProjectData).toHaveBeenCalledTimes(1) + expect(instance.triggerUpdate).toHaveBeenCalledTimes(1) + }) }) diff --git a/packages/hydrogen/src/utils/is-same-loader-payload.ts b/packages/hydrogen/src/utils/is-same-loader-payload.ts new file mode 100644 index 00000000..964fe578 --- /dev/null +++ b/packages/hydrogen/src/utils/is-same-loader-payload.ts @@ -0,0 +1,187 @@ +/** + * Structural comparison used to detect whether a reused Weaverse instance + * must adopt a fresh loader payload. + * + * `JSON.stringify` cannot do this job on the client: + * + * - Route loader data reaches `dataContext` unserialized, so deferred values + * are still `Promise`s. React 19 development builds hang an enumerable, + * self-referencing `_debugInfo` off those promises, and stringifying one + * throws `TypeError: Converting circular structure to JSON` during render + * (issue #511). + * - In production every promise stringifies to `{}`, so a *fresh* deferred + * value looks unchanged and the reused instance keeps serving the previous + * render's promise. Tokenizing promises to a constant (`'[[promise]]'`) has + * the same defect — it is stable by construction, therefore blind to + * identity. + * + * Semantics, chosen deliberately: + * + * - **Promises/thenables are atomic**: compared by identity, never traversed. + * A fresh promise is a change; the same promise is not. Debug metadata is + * invisible to the comparison. + * - **Opaque objects are atomic**: anything that is not a plain object, an + * array, or `toJSON`-serializable (`Map`, `Set`, `URL`, `RegExp`, streams, + * async iterators, cross-realm promises, class instances) is compared by + * identity. `JSON.stringify` flattens all of them to `{}`, which would + * silently collapse two distinct async values into "unchanged". + * - **Cyclic graphs are compared coinductively**: a pair already being + * compared is assumed equal, so equal self/mutual cycles are equal while a + * differing leaf anywhere still reports a change. Nothing throws. + * - **JSON-visible fields only**: `undefined`/function/symbol properties are + * omitted from objects and read as `null` in arrays, and NaN/±Infinity read + * as `null` — matching the previous `JSON.stringify` behavior for plain wire + * payloads. Key order is irrelevant, which removes the old "false mismatch + * costs a re-render" caveat. + */ + +/** `JSON.stringify` drops these from objects and nulls them inside arrays. */ +function isJsonOmitted(value: unknown): boolean { + return ( + value === undefined || + typeof value === 'function' || + typeof value === 'symbol' + ) +} + +function isObjectLike(value: unknown): value is Record { + return value !== null && typeof value === 'object' +} + +/** Duck-typed so cross-realm promises and hand-rolled thenables also match. */ +function isThenable(value: Record): boolean { + return typeof value.then === 'function' +} + +function isPlainObject(value: Record): boolean { + const proto = Object.getPrototypeOf(value) + return proto === Object.prototype || proto === null +} + +/** Returns the `toJSON()` projection, or the value itself when there is none. */ +function toJsonValue(value: Record): unknown { + return typeof value.toJSON === 'function' + ? (value.toJSON as () => unknown)() + : value +} + +/** Maps NaN/±Infinity to `null`, matching how `JSON.stringify` renders them. */ +function toFiniteOrNull(value: unknown): unknown { + return typeof value === 'number' && !Number.isFinite(value) ? null : value +} + +/** Compares two loader payloads for JSON-visible structural equality. */ +export function isSameLoaderPayload(left: unknown, right: unknown): boolean { + // Pairs currently being compared, flattened as [a0, b0, a1, b1, ...]. + // Re-entering a pair means a cycle closed on both sides; assuming equality + // there is what makes equal cyclic graphs compare equal (bisimulation) + // while a differing leaf anywhere still reports a change. + // + // Scoped to the active path rather than every visited pair, so memory stays + // proportional to graph depth (single digits for loader payloads) instead of + // node count, and the common all-equal walk allocates one array. + // + // Tradeoff: a graph that shares one subtree through many nested parents + // re-walks it per path. Wire-serialized loader payloads are trees, so this + // does not arise; memoizing equal pairs instead measured ~2x slower on a + // realistic 400-item page, which is the case that actually runs per render. + const activePairs: object[] = [] + + function isSame(rawA: unknown, rawB: unknown): boolean { + if (Object.is(rawA, rawB)) { + return true + } + // `JSON.stringify` renders NaN/±Infinity as `null`; normalizing here keeps + // a non-finite number equal to the `null` a previous wire payload carried. + const a = toFiniteOrNull(rawA) + const b = toFiniteOrNull(rawB) + if (a === b) { + // Also accepts 0/-0, which `JSON.stringify` renders identically. + return true + } + if (!(isObjectLike(a) && isObjectLike(b))) { + return false + } + if (isThenable(a) || isThenable(b)) { + // Identity comparison above already failed: distinct async values. + return false + } + for (let index = 0; index < activePairs.length; index += 2) { + if (activePairs[index] === a && activePairs[index + 1] === b) { + return true + } + } + + activePairs.push(a, b) + try { + return isSameOpenPair(a, b) + } finally { + activePairs.length -= 2 + } + } + + function isSameOpenPair( + a: Record, + b: Record + ): boolean { + const aJson = toJsonValue(a) + const bJson = toJsonValue(b) + if (aJson !== a || bJson !== b) { + return isSame(aJson, bJson) + } + + if (Array.isArray(a) !== Array.isArray(b)) { + return false + } + if (Array.isArray(a) && Array.isArray(b)) { + return isSameArray(a, b) + } + if (!(isPlainObject(a) && isPlainObject(b))) { + // Opaque object with no JSON projection — identity is the only honest + // signal, and it already failed. + return false + } + return isSameObject(a, b) + } + + function isSameArray(a: unknown[], b: unknown[]): boolean { + if (a.length !== b.length) { + return false + } + for (let index = 0; index < a.length; index++) { + const aItem = isJsonOmitted(a[index]) ? null : a[index] + const bItem = isJsonOmitted(b[index]) ? null : b[index] + if (!isSame(aItem, bItem)) { + return false + } + } + return true + } + + function isSameObject( + a: Record, + b: Record + ): boolean { + let aKeyCount = 0 + for (const key of Object.keys(a)) { + if (isJsonOmitted(a[key])) { + continue + } + aKeyCount++ + // A JSON-visible value never compares equal to an omitted one, so a + // missing/undefined counterpart falls out of `isSame` as a change. + if (!isSame(a[key], b[key])) { + return false + } + } + let bKeyCount = 0 + for (const key of Object.keys(b)) { + if (!isJsonOmitted(b[key])) { + bKeyCount++ + } + } + return aKeyCount === bKeyCount + } + + return isSame(left, right) +} diff --git a/packages/hydrogen/src/utils/sync-reused-instance.ts b/packages/hydrogen/src/utils/sync-reused-instance.ts index 1761e944..31dce769 100644 --- a/packages/hydrogen/src/utils/sync-reused-instance.ts +++ b/packages/hydrogen/src/utils/sync-reused-instance.ts @@ -1,5 +1,6 @@ import type { WeaverseHydrogenParams } from '../types' import type { WeaverseHydrogen } from '../WeaverseHydrogenRoot' +import { isSameLoaderPayload } from './is-same-loader-payload' /** * Apply a fresh loader payload to a reused Weaverse instance. @@ -14,9 +15,11 @@ import type { WeaverseHydrogen } from '../WeaverseHydrogenRoot' * (unsaved drafts must not be clobbered) and applies updates via * `refreshStudio` instead. * - * Change detection uses JSON comparison — loader payloads are - * wire-serialized, so key order is stable between runs; a false mismatch - * only costs one extra re-render. + * Change detection uses {@link isSameLoaderPayload}: a structural comparison + * that treats promises/thenables and other opaque objects as atomic values + * compared by identity, and tolerates cyclic graphs. `dataContext` carries + * live route loader data — including unresolved deferred promises whose React + * development metadata is cyclic — so it cannot be JSON-serialized here. */ export function syncReusedInstance( weaverse: WeaverseHydrogen, @@ -25,11 +28,11 @@ export function syncReusedInstance( weaverse.requestInfo = params.requestInfo weaverse.internal = params.internal - let dataChanged = - JSON.stringify(weaverse.data) !== JSON.stringify(params.data) - let contextChanged = - JSON.stringify(weaverse.dataContext ?? null) !== - JSON.stringify(params.dataContext ?? null) + let dataChanged = !isSameLoaderPayload(weaverse.data, params.data) + let contextChanged = !isSameLoaderPayload( + weaverse.dataContext ?? null, + params.dataContext ?? null + ) if (contextChanged) { // Assign before notifying any item store: components resolve data