fix(hydrogen): route-scope WeaverseHydrogenRoot weaverseData lookup (#451)

[paul-phan]

Fixes #451.

What changed

WeaverseHydrogenRoot previously resolved weaverseData by walking useMatches() and returning the deepest match's payload. Because useMatches() returns the same array for every component on the page, every <WeaverseContent /> instance resolved to the same value — making it impossible for two instances (e.g. a layout-loaded page + a child-loaded page) to render different Weaverse pages on the same URL.

This PR restores the route-scoped primitive via useLoaderData() for the weaverseData lookup, while keeping the existing useMatches()-driven createWeaverseDataContext infrastructure untouched (it legitimately needs all matches to build the route-keyed data-connector context).

How it resolves now

A pure helper pickWeaverseData() implements a three-tier policy:

1. Explicit data prop (new — escape hatch) — supports the typed Route.ComponentProps.loaderData framework-mode pattern and gives consumers full control.
2. Own route's loader data via useLoaderData() — React Router's route-scoped primitive; this is what makes layout + child compositions work.
3. Ancestor walk through useMatches() — back-compat fallback for the index-without-loader pattern (e.g. an index.tsx that omits weaverseData and inherits from its layout).

Back-compat guarantee (verified)

• Unmodified themes (zero-prop <WeaverseContent />) work identically via Tier 2 for the common single-instance pattern.
• Index leaves without loaders continue to fall back to ancestor weaverseData via Tier 3.
• Deferred (Promise) loaders pass through unchanged at every tier.
• data? prop is optional; existing call sites compile unchanged (verified via generated .d.ts).
• Only intentional behaviour change: two <WeaverseHydrogenRoot> instances on the same URL now each render their own route's page instead of both rendering the deepest one — exactly the WeaverseHydrogenRoot resolves only the deepest match's weaverseData, breaking nested layout + child Weaverse pages #451 fix.

The pilot wrapper (templates/pilot/app/weaverse/index.tsx) is intentionally not modified in this PR. The fix works end-to-end for unmodified pilots; updating the wrapper to forward an explicit data prop is a separate optional follow-up.

Tests

18 new selector unit tests in packages/hydrogen/__tests__/pick-weaverse-data.test.ts:

• All three tiers (explicit prop incl. null + Promise, own loader data, ancestor walk)
• Non-object / null / undefined edge cases at every tier
• 5 dedicated back-compat scenarios mirroring legacy pilot route shapes
• Explicit WeaverseHydrogenRoot resolves only the deepest match's weaverseData, breaking nested layout + child Weaverse pages #451 regression assertion: two instances with different ownLoaderData diverge despite an identical matches array

Local verification: pnpm run biome && pnpm run typecheck && pnpm run test — 91/91 hydrogen tests pass.

Background — root cause history

The buggy resolver was introduced in commit a21c8d5d (Sep 23 2025, "feat: enhance data connector with deep recursive replacement and performance optimizations"), which migrated from useLoaderData() to a useMatches() walk under the banner "60-70% memory improvement". Reading the diff, the real architectural motivation was building a flat route-keyed data context for {{root.layout.shop.name}}-style templates — which legitimately needs all matches. The weaverseData lookup was collapsed into the same walk only because matches was already in scope; that's the regression. The memory improvement claim appears only in CHANGELOGs (no benchmark in the repo) and pertains to the data-context rebuild, not to the weaverseData lookup specifically.

Commit 4dfcbea7 (Nov 12 2025) later reversed the iteration direction, flipping the failure mode from "layout page renders twice" to "child page renders twice" — which is what #451 reports.

Second commit

chore: replace bun with pnpm in releasing-weaverse-sdks skill — follow-up to PR #450's package manager migration; this skill file was missed.

[claude]

Claude finished @paul-phan's task in 2m 34s —— View job

---

Code Review: fix(hydrogen): route-scope WeaverseHydrogenRoot weaverseData lookup (#451)

• Gather context and understand the PR
• Read changed files
• Analyze implementation correctness and design
• Check tests for coverage and correctness
• Post review feedback

---

Summary

This is a well-motivated, carefully scoped fix. The root cause diagnosis is accurate (the useMatches() walk introduced in a21c8d5d collapsed a route-scoped lookup into a tree-global one), the fix is minimally invasive, and the back-compat contract is solid. The test suite is particularly strong.

---

packages/hydrogen/src/utils/pick-weaverse-data.ts

Design: three-tier policy — correct and clean.

The tier hierarchy is the right call:

• Tier 1 (explicit prop) gives consumers an escape hatch without requiring any infrastructure.
• Tier 2 (useLoaderData()) is the route-scoped primitive that fixes WeaverseHydrogenRoot resolves only the deepest match's weaverseData, breaking nested layout + child Weaverse pages #451.
• Tier 3 (ancestor walk) preserves the legacy index-without-loader pattern.

hasWeaverseData — the undefined exclusion is correct but worth one extra note.

// pick-weaverse-data.ts:77-84
return (value as Record<string, unknown>).weaverseData !== undefined

Excluding undefined is intentional and well-commented: it prevents { weaverseData: undefined } from short-circuiting Tier 2 and swallowing the ancestor walk. One subtle edge case: if a loader returns { weaverseData: null }, hasWeaverseData returns true (because null !== undefined). pickWeaverseData then returns null from Tier 2, and the component falls through to render <ErrorComponent /> rather than the "suppress rendering" path. This is unlikely to matter in practice (no loader would intentionally return weaverseData: null), but it could be worth a one-line comment alongside the existing note.

Tier 3 walk direction — verified correct.

matches in React Router is ordered root→leaf, so matches[length-1] is the deepest/leaf match. The walk from length-1 → 0 correctly finds the nearest ancestor first (or the deepest sibling for the "two WeaverseContent instances" protection that Tier 2 provides).

---

packages/hydrogen/src/WeaverseHydrogenRoot.tsx

Hook call order — correct.

All three hooks (useMatches, useLoaderData, useMemo) are called unconditionally before any conditional returns. The dataProp === null early return is placed correctly after the hooks. ✓

The dataProp === null guard — semantically necessary.

// WeaverseHydrogenRoot.tsx:426-428
if (dataProp === null) {
  return null
}

Without this, dataProp === null would resolve to weaverseData === null, and the component would fall through to <ErrorComponent error={{ message: 'No Weaverse data found…' }} />. The explicit guard correctly distinguishes "intentionally suppressed" from "auto-resolution found nothing". Good.

useMemo dependency array — correct.

const weaverseData = useMemo(
  () => pickWeaverseData(dataProp, ownLoaderData, matches),
  [dataProp, ownLoaderData, matches]
)

useLoaderData() and useMatches() both return stable references (React Router memoizes them), so this will only recompute when actual data changes. ✓

One minor observation: useLoaderData() without a route ID argument.

React Router's useLoaderData() (no argument) returns data for the route that rendered the calling component. In a layout route, this is the layout's loader data; in a child route, it's the child's. This is exactly the semantics required for #451. Worth noting this assumption in a comment next to the useLoaderData() call — the existing comment is good but doesn't explicitly call out why the no-arg form works vs. useRouteLoaderData(id). The PR description covers this clearly though.

---

packages/hydrogen/__tests__/pick-weaverse-data.test.ts

The test suite is excellent:

• All three tiers covered, including the weaverseData: undefined regression (the in operator trap at both Tier 2 and Tier 3 — lines 80-89, 132-139).
• The canonical pilot route shape (root + current route in matches, both patterns).
• The legacy index-without-loader pattern (lines 187-202).
• Deferred (Promise) loaders pass through unchanged.
• The explicit WeaverseHydrogenRoot resolves only the deepest match's weaverseData, breaking nested layout + child Weaverse pages #451 regression assertion (lines 228-250): two instances with different ownLoaderData but the same matches array diverge — this is the most important test.

One edge case not tested: dataProp set to a non-null falsy value like 0 or false. These would satisfy dataProp !== undefined in Tier 1 and return the falsy value, which WeaverseHydrogenRoot would then evaluate as falsy in if (weaverseData) and render <ErrorComponent />. This is probably not a real-world scenario (the type is WeaverseLoaderData | Promise<WeaverseLoaderData> | null), but it's worth knowing the behavior.

---

.claude/skills/releasing-weaverse-sdks/SKILL.md

Straightforward bun → pnpm migration: command substitutions, lockfile name (bun.lock → pnpm-lock.yaml), and the mistake table. All correct and consistent with the package manager migration in #450.

---

Verdict

Approve. The fix is correct, minimal, and well-tested. The three-tier policy cleanly addresses the root cause without breaking any existing consumers. The only things I'd consider for follow-up (not blockers):

1. A note in hasWeaverseData about the weaverseData: null case (returns true → falls through to error path).
2. The pilot wrapper forward of an explicit data prop mentioned in the PR description — tracked as an optional follow-up.

[hta218]

Thanks Paul for the thorough fix and excellent write-up! 🙌

The route-scoped useLoaderData() approach is the right call, and the 3-tier pickWeaverseData() selector keeps full back-compat while cleanly solving #451. The pure, hook-free helper with 18 unit tests covering every tier, edge case, and the regression scenario is exactly how this should be done.

One non-blocking follow-up to consider: the pure selector is well covered, but the actual useLoaderData() route-scoping (the crux of the fix) isn't exercised by an automated test — a future integration test with createMemoryRouter + nested routes would lock it down. Fine to merge now since the RR v7 behavior is well-established.

Also remember to land the new data prop + #451 behavior change in packages/hydrogen/CHANGELOG.md at release time.

Approving — great work! 🚀