feat(vue): add @modular-vue/runtime zones and route data (PR-23) - #66
Open
kibertoad wants to merge 2 commits into
Open
feat(vue): add @modular-vue/runtime zones and route data (PR-23)#66kibertoad wants to merge 2 commits into
kibertoad wants to merge 2 commits into
Conversation
Add zones.ts, active-zones.ts, and route-data.ts to @modular-vue/runtime, the Vue analogs of the same-named files in react-router-runtime. All three read useRoute().matched, take zones/static data off each record's meta (vue-router's analog of React Router's handle, per the PR-20 route-meta convention), and funnel through core's mergeRouteStaticData with a createRouteDataOverrideWarner dev warning. useActiveZones layers the active module's descriptor zones over route zones, module-wins. The runtime index now exports useZones, useActiveZones, useRouteData. Widen the warner contract in @modular-frontend/core, as its own "new runtimes should add their package name here" note anticipates: RouteDataRuntimeLabel gains "@modular-vue/runtime", RouteDataFieldLabel gains "meta", and readMatchId falls back to a vue-router record's name then path (vue-router matched records carry no id). The composables return a ComputedRef driven by the reactive useRoute(), so the merged map recomputes on navigation; useActiveZones takes activeModuleId as a MaybeRefOrGetter so a tab switcher's changing id re-drives the merge. Tests mock useRoute (mirroring the React suites mocking useMatches) and assert on .value. 46 new tests across zones (7), active-zones (7), route-data (9), slots (23); package total 108. Full workspace typecheck (120 tasks) and vite build pass; React/TanStack runtime suites stay green.
📝 WalkthroughWalkthroughThis PR extends the shared route-data override-warning and merge utilities in frontend-core to support vue-router semantics (match id/name/path fallbacks, index-based dedup disambiguation), adds new vue-router-runtime composables ( ChangesVue-router runtime support
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
…h position Two fixes to the shared override warner surfaced while reviewing the @modular-vue/runtime port (PR-23): - readMatchId falls back to a vue-router record's `path`, which is not unique: nameless index routes report the same `path` as their parent. Two override sites that share a path pair collapsed to one dedup key, so the second real clobber was silently suppressed, and the message read as a route overriding itself. mergeRouteStaticData now threads each match's position in the matched hierarchy into the override info, and the warner folds it into the dedup key and (when the ids would otherwise collide) the message. React Router / TanStack output is unchanged since their ids are already unique. - Update stale JSDoc in route-data-warn.ts that still described two runtimes and omitted the `meta` field label after @modular-vue/runtime and "meta" were added.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
packages/frontend-core/src/route-data-warn.ts (1)
85-90: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low value
ambiguousonly guardspreviousIndex, notnextIndex.If a future caller ever supplies
previousIndexwithoutnextIndex(or vice versa),ambiguouswould still evaluate true and the label with the missing index would render as"... (match undefined)". Currently unreachable sincemergeRouteStaticDataalways sets both together, but worth hardening for future callers of this exported helper.🛡️ Suggested defensive check
- const ambiguous = prevId === nextId && info.previousIndex !== undefined; + const ambiguous = + prevId === nextId && info.previousIndex !== undefined && info.nextIndex !== undefined;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/frontend-core/src/route-data-warn.ts` around lines 85 - 90, The route warning label logic in route-data-warn.ts should defensively require both indices before treating a route as ambiguous. Update the ambiguous check in the helper that builds prevLabel and nextLabel so it only becomes true when prevId equals nextId and both info.previousIndex and info.nextIndex are defined, preventing “match undefined” from appearing for partial callers. Keep the existing behavior for mergeRouteStaticData unchanged, but harden the exported helper against future inputs.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/vue-support-tracker.md`:
- Line 205: The test-count summary in the Vue support tracker has a
running-total mismatch: the per-file counts in the PR-23 summary add up
correctly, but the stated package total does not match the prior total from
PR-22. Update the summary text in docs/vue-support-tracker.md so the cumulative
package total is arithmetically consistent with the earlier “package total 58”
and the 46 new tests, and keep the existing per-file counts and feature notes
unchanged.
In `@packages/frontend-core/src/route-data-warn.ts`:
- Around line 33-38: Update the JSDoc in route-data-warn so it matches the
actual dedup logic in the route matcher warning helper: the hierarchy position
is always included in the dedup key via the prevPart/nextPart handling, not only
when ids collide. Keep the collision-specific wording only for the message label
behavior tied to the ambiguous case, and adjust the text near the route match/id
description so future readers understand the difference between the dedup key
and the displayed message.
In `@packages/vue-router-runtime/src/route-data.ts`:
- Around line 5-84: The JSDoc for useRouteData is currently attached to the
internal onOverride constant instead of the exported composable, so move that
doc block to immediately precede export function useRouteData and keep
onOverride undocumented; this will ensure tooling associates the documentation
with the public API symbol rather than the helper constant.
In `@packages/vue-router-runtime/src/zones.ts`:
- Around line 6-61: The JSDoc intended for useZones is attached to the internal
onOverride constant because it immediately precedes that declaration. Move the
createRouteDataOverrideWarner("`@modular-vue/runtime`", "useZones", "meta") const
above the doc block, or place the doc directly above export function useZones so
the language service and docs generators associate it with useZones.
---
Nitpick comments:
In `@packages/frontend-core/src/route-data-warn.ts`:
- Around line 85-90: The route warning label logic in route-data-warn.ts should
defensively require both indices before treating a route as ambiguous. Update
the ambiguous check in the helper that builds prevLabel and nextLabel so it only
becomes true when prevId equals nextId and both info.previousIndex and
info.nextIndex are defined, preventing “match undefined” from appearing for
partial callers. Keep the existing behavior for mergeRouteStaticData unchanged,
but harden the exported helper against future inputs.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 176a34cc-c782-4ebc-a725-95b47a7189ec
📒 Files selected for processing (13)
docs/vue-support-tracker.mdpackages/frontend-core/src/route-data-warn.test.tspackages/frontend-core/src/route-data-warn.tspackages/frontend-core/src/route-data.test.tspackages/frontend-core/src/route-data.tspackages/vue-router-runtime/src/active-zones.test.tspackages/vue-router-runtime/src/active-zones.tspackages/vue-router-runtime/src/index.tspackages/vue-router-runtime/src/route-data.test.tspackages/vue-router-runtime/src/route-data.tspackages/vue-router-runtime/src/slots.test.tspackages/vue-router-runtime/src/zones.test.tspackages/vue-router-runtime/src/zones.ts
| - The React hooks return a plain object recomputed each render; the Vue composables return a `ComputedRef` driven by the reactive `useRoute()`, so the merged map recomputes on navigation. Consumers read `.value` (or let the template auto-unwrap). `useActiveZones` additionally accepts `activeModuleId` as a `MaybeRefOrGetter` and reads it via `toValue`, so a tab switcher's changing id re-drives the merge — the reactive analog of React reading the argument each render. | ||
| - The zones/active-zones/route-data tests mock `vue-router`'s `useRoute` (mirroring the React suites mocking `useMatches`) rather than booting a real router, and assert on `.value`. The `slots.test.ts` port pulls `buildSlotsManifest` / `collectDynamicSlotFactories` / `evaluateDynamicSlots` from `@modular-frontend/core` and `createSlotsSignal` from `@modular-vue/vue` (the runtime re-export barrel), matching the React source case-for-case. | ||
|
|
||
| Error-message prefixes are `[@modular-vue/runtime]`. Acceptance: met. Deepest-wins merge, undefined-skips-inherit, and module-over-route precedence match the React suites; two added reactivity tests cover route-change recompute (`useZones`) and reactive `activeModuleId` re-merge (`useActiveZones`). 46 new tests across `zones.test.ts` (7), `active-zones.test.ts` (7), `route-data.test.ts` (9), `slots.test.ts` (23); package total 108. Full workspace typecheck (120 tasks) and `vite build` (JS + dts) pass; externals (`vue`, `vue-router`, `@modular-frontend/core`, `@modular-vue/vue`, `@modular-vue/core`) stay unbundled. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Test-count arithmetic doesn't add up.
PR-22 states a "package total 58" and PR-23 says "46 new tests ... package total 108". 58 + 46 = 104, not 108 — a 4-test discrepancy. The individual per-file counts (7+7+9+23=46) are internally consistent, so the "108" running total looks like the error.
🔧 Proposed fix
-Error-message prefixes are `[`@modular-vue/runtime`]`. Acceptance: met. Deepest-wins merge, undefined-skips-inherit, and module-over-route precedence match the React suites; two added reactivity tests cover route-change recompute (`useZones`) and reactive `activeModuleId` re-merge (`useActiveZones`). 46 new tests across `zones.test.ts` (7), `active-zones.test.ts` (7), `route-data.test.ts` (9), `slots.test.ts` (23); package total 108. Full workspace typecheck (120 tasks) and `vite build` (JS + dts) pass; externals (`vue`, `vue-router`, `@modular-frontend/core`, `@modular-vue/vue`, `@modular-vue/core`) stay unbundled.
+Error-message prefixes are `[`@modular-vue/runtime`]`. Acceptance: met. Deepest-wins merge, undefined-skips-inherit, and module-over-route precedence match the React suites; two added reactivity tests cover route-change recompute (`useZones`) and reactive `activeModuleId` re-merge (`useActiveZones`). 46 new tests across `zones.test.ts` (7), `active-zones.test.ts` (7), `route-data.test.ts` (9), `slots.test.ts` (23); package total 104. Full workspace typecheck (120 tasks) and `vite build` (JS + dts) pass; externals (`vue`, `vue-router`, `@modular-frontend/core`, `@modular-vue/vue`, `@modular-vue/core`) stay unbundled.📝 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.
Suggested change
| Error-message prefixes are `[@modular-vue/runtime]`. Acceptance: met. Deepest-wins merge, undefined-skips-inherit, and module-over-route precedence match the React suites; two added reactivity tests cover route-change recompute (`useZones`) and reactive `activeModuleId` re-merge (`useActiveZones`). 46 new tests across `zones.test.ts` (7), `active-zones.test.ts` (7), `route-data.test.ts` (9), `slots.test.ts` (23); package total 108. Full workspace typecheck (120 tasks) and `vite build` (JS + dts) pass; externals (`vue`, `vue-router`, `@modular-frontend/core`, `@modular-vue/vue`, `@modular-vue/core`) stay unbundled. | |
| Error-message prefixes are `[`@modular-vue/runtime`]`. Acceptance: met. Deepest-wins merge, undefined-skips-inherit, and module-over-route precedence match the React suites; two added reactivity tests cover route-change recompute (`useZones`) and reactive `activeModuleId` re-merge (`useActiveZones`). 46 new tests across `zones.test.ts` (7), `active-zones.test.ts` (7), `route-data.test.ts` (9), `slots.test.ts` (23); package total 104. Full workspace typecheck (120 tasks) and `vite build` (JS + dts) pass; externals (`vue`, `vue-router`, `@modular-frontend/core`, `@modular-vue/vue`, `@modular-vue/core`) stay unbundled. |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/vue-support-tracker.md` at line 205, The test-count summary in the Vue
support tracker has a running-total mismatch: the per-file counts in the PR-23
summary add up correctly, but the stated package total does not match the prior
total from PR-22. Update the summary text in docs/vue-support-tracker.md so the
cumulative package total is arithmetically consistent with the earlier “package
total 58” and the 46 new tests, and keep the existing per-file counts and
feature notes unchanged.
Comment on lines
+33
to
+38
| * are read off the match object at warn time: React Router and TanStack | ||
| * Router expose a stable `id`/`routeId` on `useMatches()` entries, and | ||
| * vue-router's matched records fall back to their `name` or `path`. Because | ||
| * a vue-router `path` is not unique (nameless index routes share their | ||
| * parent's `path`), the match's position in the hierarchy is folded into | ||
| * both the dedup key and the message when the ids would otherwise collide. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
JSDoc overstates when position-folding applies to the dedup key.
The docstring says the match position is folded into both the dedup key and the message when the ids would otherwise collide, but in the implementation the dedup key always folds in the index whenever it's defined (prevPart/nextPart), regardless of whether prevId === nextId. Only the message label folding is conditioned on collision (ambiguous). This could mislead a future reader into assuming the dedup key stays untouched for non-colliding ids.
✏️ Suggested wording fix
- * a vue-router `path` is not unique (nameless index routes share their
- * parent's `path`), the match's position in the hierarchy is folded into
- * both the dedup key and the message when the ids would otherwise collide.
+ * a vue-router `path` is not unique (nameless index routes share their
+ * parent's `path`), the match's position is always folded into the dedup
+ * key (when known), and is additionally surfaced in the message only when
+ * the ids would otherwise collide.📝 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.
Suggested change
| * are read off the match object at warn time: React Router and TanStack | |
| * Router expose a stable `id`/`routeId` on `useMatches()` entries, and | |
| * vue-router's matched records fall back to their `name` or `path`. Because | |
| * a vue-router `path` is not unique (nameless index routes share their | |
| * parent's `path`), the match's position in the hierarchy is folded into | |
| * both the dedup key and the message when the ids would otherwise collide. | |
| * are read off the match object at warn time: React Router and TanStack | |
| * Router expose a stable `id`/`routeId` on `useMatches()` entries, and | |
| * vue-router's matched records fall back to their `name` or `path`. Because | |
| * a vue-router `path` is not unique (nameless index routes share their | |
| * parent's `path`), the match's position is always folded into the dedup | |
| * key (when known), and is additionally surfaced in the message only when | |
| * the ids would otherwise collide. |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/frontend-core/src/route-data-warn.ts` around lines 33 - 38, Update
the JSDoc in route-data-warn so it matches the actual dedup logic in the route
matcher warning helper: the hierarchy position is always included in the dedup
key via the prevPart/nextPart handling, not only when ids collide. Keep the
collision-specific wording only for the message label behavior tied to the
ambiguous case, and adjust the text near the route match/id description so
future readers understand the difference between the dedup key and the displayed
message.
Comment on lines
+5
to
+84
| /** | ||
| * Read merged `meta` values from the currently matched route hierarchy — | ||
| * the "non-component zone" escape hatch. | ||
| * | ||
| * `useZones` is the component-typed channel: each value must be a | ||
| * `UiComponent | undefined` so the shell can render it in a layout | ||
| * region. That constraint is a useful rail 95% of the time, but it gets in | ||
| * the way for non-component metadata the module wants to attach to a route — | ||
| * a header variant enum, a page title string, an analytics event name, a | ||
| * per-route feature flag. `useRouteData` is the relaxed-typing counterpart: | ||
| * same deepest-wins merge over `route.meta`, no constraint on values. | ||
| * | ||
| * Two composables, two channels: keep components in `meta` fields consumed by | ||
| * `useZones`, keep metadata in fields consumed by `useRouteData`. They can | ||
| * co-exist in the same `meta` object because they read the same match | ||
| * values; each composable only surfaces the keys you've declared in its type. | ||
| * | ||
| * @example | ||
| * ```ts | ||
| * // Declare both shapes explicitly — zones for renderable components, | ||
| * // route data for everything else. | ||
| * interface AppZones { | ||
| * HeaderActions?: UiComponent | ||
| * DetailPanel?: UiComponent | ||
| * } | ||
| * interface AppRouteData { | ||
| * headerVariant?: "portal" | "project" | "setup" | ||
| * pageTitle?: string | ||
| * } | ||
| * | ||
| * // A route can contribute to both: | ||
| * meta: { | ||
| * HeaderActions: ProjectActions, // → useZones<AppZones>() | ||
| * headerVariant: "project" as const, // → useRouteData<AppRouteData>() | ||
| * } | ||
| * | ||
| * // Layout reads each channel with its own typing: | ||
| * const zones = useZones<AppZones>() | ||
| * const routeData = useRouteData<AppRouteData>() | ||
| * // routeData.value.headerVariant, routeData.value.pageTitle | ||
| * ``` | ||
| * | ||
| * ## Merge semantics | ||
| * | ||
| * Walks matched records root-to-leaf, deepest match wins per key. | ||
| * `undefined` values at a deeper level don't override an ancestor — | ||
| * **omit the key (or set it to `undefined`) to inherit**. Set the key | ||
| * to `null` to **explicitly clear** an ancestor's value; the consuming | ||
| * shell decides how to render `null` (typically: as if the field was | ||
| * never set, but distinct from "still loading"). | ||
| * | ||
| * In dev (NODE_ENV !== "production"), this composable logs a deduped | ||
| * `console.warn` whenever a deeper match overrides a key already set by | ||
| * an ancestor. The warning is intended to catch accidental clobbers of | ||
| * shell-owned route data (e.g. `headerVariant`); ignore it when the | ||
| * override is intentional. | ||
| * | ||
| * ## Returned object contains all meta keys, not just declared ones | ||
| * | ||
| * The merged value is the raw merged `meta` — TypeScript narrows what you | ||
| * can *access* via `TRouteData`, but every key present across matches is | ||
| * still there at runtime. If a route declared a component zone (e.g. | ||
| * `HeaderActions`) on the same `meta` object, it appears here too. | ||
| * | ||
| * This is intentional: the two composables (`useZones` / `useRouteData`) | ||
| * don't have to coordinate on key sets, so a migration can split meta fields | ||
| * between them incrementally. The consequence is that code that iterates the | ||
| * merged value (`Object.keys(useRouteData().value)`, `JSON.stringify`, etc.) | ||
| * will see component entries mixed with data entries — read by declared key, | ||
| * not by iteration. | ||
| * | ||
| * ## Return value | ||
| * | ||
| * Returns a `ComputedRef` driven by `useRoute()`, so the merged data | ||
| * recomputes when navigation changes the matched hierarchy. Read | ||
| * `routeData.value` in script, or let the template auto-unwrap it. | ||
| */ | ||
| const onOverride = createRouteDataOverrideWarner("@modular-vue/runtime", "useRouteData", "meta"); | ||
|
|
||
| export function useRouteData<TRouteData extends object>(): ComputedRef<Partial<TRouteData>> { |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Same JSDoc-misattachment issue as zones.ts.
The doc block (Lines 5-81) is directly above const onOverride = ... (Line 82) instead of export function useRouteData (Line 84), so tooling attaches it to the internal const rather than the exported composable.
🔧 Proposed fix
-/**
- * Read merged `meta` values from the currently matched route hierarchy —
- * ...
- */
-const onOverride = createRouteDataOverrideWarner("`@modular-vue/runtime`", "useRouteData", "meta");
-
-export function useRouteData<TRouteData extends object>(): ComputedRef<Partial<TRouteData>> {
+const onOverride = createRouteDataOverrideWarner("`@modular-vue/runtime`", "useRouteData", "meta");
+
+/**
+ * Read merged `meta` values from the currently matched route hierarchy —
+ * ...
+ */
+export function useRouteData<TRouteData extends object>(): ComputedRef<Partial<TRouteData>> {📝 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.
Suggested change
| /** | |
| * Read merged `meta` values from the currently matched route hierarchy — | |
| * the "non-component zone" escape hatch. | |
| * | |
| * `useZones` is the component-typed channel: each value must be a | |
| * `UiComponent | undefined` so the shell can render it in a layout | |
| * region. That constraint is a useful rail 95% of the time, but it gets in | |
| * the way for non-component metadata the module wants to attach to a route — | |
| * a header variant enum, a page title string, an analytics event name, a | |
| * per-route feature flag. `useRouteData` is the relaxed-typing counterpart: | |
| * same deepest-wins merge over `route.meta`, no constraint on values. | |
| * | |
| * Two composables, two channels: keep components in `meta` fields consumed by | |
| * `useZones`, keep metadata in fields consumed by `useRouteData`. They can | |
| * co-exist in the same `meta` object because they read the same match | |
| * values; each composable only surfaces the keys you've declared in its type. | |
| * | |
| * @example | |
| * ```ts | |
| * // Declare both shapes explicitly — zones for renderable components, | |
| * // route data for everything else. | |
| * interface AppZones { | |
| * HeaderActions?: UiComponent | |
| * DetailPanel?: UiComponent | |
| * } | |
| * interface AppRouteData { | |
| * headerVariant?: "portal" | "project" | "setup" | |
| * pageTitle?: string | |
| * } | |
| * | |
| * // A route can contribute to both: | |
| * meta: { | |
| * HeaderActions: ProjectActions, // → useZones<AppZones>() | |
| * headerVariant: "project" as const, // → useRouteData<AppRouteData>() | |
| * } | |
| * | |
| * // Layout reads each channel with its own typing: | |
| * const zones = useZones<AppZones>() | |
| * const routeData = useRouteData<AppRouteData>() | |
| * // routeData.value.headerVariant, routeData.value.pageTitle | |
| * ``` | |
| * | |
| * ## Merge semantics | |
| * | |
| * Walks matched records root-to-leaf, deepest match wins per key. | |
| * `undefined` values at a deeper level don't override an ancestor — | |
| * **omit the key (or set it to `undefined`) to inherit**. Set the key | |
| * to `null` to **explicitly clear** an ancestor's value; the consuming | |
| * shell decides how to render `null` (typically: as if the field was | |
| * never set, but distinct from "still loading"). | |
| * | |
| * In dev (NODE_ENV !== "production"), this composable logs a deduped | |
| * `console.warn` whenever a deeper match overrides a key already set by | |
| * an ancestor. The warning is intended to catch accidental clobbers of | |
| * shell-owned route data (e.g. `headerVariant`); ignore it when the | |
| * override is intentional. | |
| * | |
| * ## Returned object contains all meta keys, not just declared ones | |
| * | |
| * The merged value is the raw merged `meta` — TypeScript narrows what you | |
| * can *access* via `TRouteData`, but every key present across matches is | |
| * still there at runtime. If a route declared a component zone (e.g. | |
| * `HeaderActions`) on the same `meta` object, it appears here too. | |
| * | |
| * This is intentional: the two composables (`useZones` / `useRouteData`) | |
| * don't have to coordinate on key sets, so a migration can split meta fields | |
| * between them incrementally. The consequence is that code that iterates the | |
| * merged value (`Object.keys(useRouteData().value)`, `JSON.stringify`, etc.) | |
| * will see component entries mixed with data entries — read by declared key, | |
| * not by iteration. | |
| * | |
| * ## Return value | |
| * | |
| * Returns a `ComputedRef` driven by `useRoute()`, so the merged data | |
| * recomputes when navigation changes the matched hierarchy. Read | |
| * `routeData.value` in script, or let the template auto-unwrap it. | |
| */ | |
| const onOverride = createRouteDataOverrideWarner("@modular-vue/runtime", "useRouteData", "meta"); | |
| export function useRouteData<TRouteData extends object>(): ComputedRef<Partial<TRouteData>> { | |
| const onOverride = createRouteDataOverrideWarner("`@modular-vue/runtime`", "useRouteData", "meta"); | |
| /** | |
| * Read merged `meta` values from the currently matched route hierarchy — | |
| * the "non-component zone" escape hatch. | |
| * | |
| * `useZones` is the component-typed channel: each value must be a | |
| * `UiComponent | undefined` so the shell can render it in a layout | |
| * region. That constraint is a useful rail 95% of the time, but it gets in | |
| * the way for non-component metadata the module wants to attach to a route — | |
| * a header variant enum, a page title string, an analytics event name, a | |
| * per-route feature flag. `useRouteData` is the relaxed-typing counterpart: | |
| * same deepest-wins merge over `route.meta`, no constraint on values. | |
| * | |
| * Two composables, two channels: keep components in `meta` fields consumed by | |
| * `useZones`, keep metadata in fields consumed by `useRouteData`. They can | |
| * co-exist in the same `meta` object because they read the same match | |
| * values; each composable only surfaces the keys you've declared in its type. | |
| * | |
| * `@example` | |
| * |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/vue-router-runtime/src/route-data.ts` around lines 5 - 84, The JSDoc
for useRouteData is currently attached to the internal onOverride constant
instead of the exported composable, so move that doc block to immediately
precede export function useRouteData and keep onOverride undocumented; this will
ensure tooling associates the documentation with the public API symbol rather
than the helper constant.
Comment on lines
+6
to
+61
| /** | ||
| * Read zone components contributed by the currently matched route hierarchy. | ||
| * | ||
| * Zones ride on vue-router's `meta` (the analog of React Router's `handle`) | ||
| * on individual routes. This composable walks all matched records from root | ||
| * to leaf via `useRoute().matched` and returns a merged map where the deepest | ||
| * match wins for each zone key. | ||
| * | ||
| * @example | ||
| * // In the shell layout's <script setup>: | ||
| * const zones = useZones<AppZones>() | ||
| * | ||
| * // In the template: | ||
| * // <main><router-view /></main> | ||
| * // <aside><component :is="zones.detailPanel" v-if="zones.detailPanel" /></aside> | ||
| * | ||
| * @example | ||
| * // In a module's createRoutes(): | ||
| * { | ||
| * path: ':userId', | ||
| * component: UserDetailPage, | ||
| * meta: { | ||
| * detailPanel: UserDetailSidebar, | ||
| * } satisfies ModuleRouteMeta<AppZones>, | ||
| * } | ||
| * | ||
| * ## Ownership and overrides | ||
| * | ||
| * Zones merge with deepest-wins semantics: a descendant route that declares | ||
| * the same zone key as an ancestor silently replaces it. That is the | ||
| * intended escape hatch for "this section overrides the default panel" — | ||
| * but it is also the failure mode when a descendant route accidentally | ||
| * declares a zone key the shell layout owns (e.g. `HeaderTitle`, | ||
| * `HeaderActions`). | ||
| * | ||
| * In dev (NODE_ENV !== "production"), this composable logs a deduped | ||
| * `console.warn` whenever a deeper match overrides a zone already set by | ||
| * an ancestor. Use that warning to catch unintended clobbers; if the | ||
| * override is intentional you can ignore it. | ||
| * | ||
| * To **inherit** an ancestor's zone, omit the key (or set to `undefined`). | ||
| * To **explicitly clear** an ancestor's zone at a deeper route, set the | ||
| * key to `null`. Don't redeclare a shell-owned zone "just to be safe" — | ||
| * that's exactly the pattern the override warning is designed to flag. | ||
| * | ||
| * ## Return value | ||
| * | ||
| * Returns a `ComputedRef` driven by `useRoute()`, so the merged map | ||
| * recomputes when navigation changes the matched hierarchy. The React | ||
| * binding returns a plain object per render; Vue's reactive route makes a | ||
| * `computed` the faithful analog. Read `zones.value` in script, or let the | ||
| * template auto-unwrap it. | ||
| */ | ||
| const onOverride = createRouteDataOverrideWarner("@modular-vue/runtime", "useZones", "meta"); | ||
|
|
||
| export function useZones<TZones extends ZoneMapOf<TZones>>(): ComputedRef<Partial<TZones>> { |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
JSDoc for useZones is misattached to the internal onOverride const.
The large doc block (Lines 6-58) sits directly above const onOverride = ... (Line 59), not above export function useZones (Line 61). Editors/TS language service and doc generators (TypeDoc) associate a comment with the immediately following declaration, so this doc currently documents the unexported onOverride constant — hovering useZones in an IDE will show nothing, defeating the purpose of this extensive doc.
🔧 Proposed fix: move the const above the JSDoc
-/**
- * Read zone components contributed by the currently matched route hierarchy.
- * ...
- */
-const onOverride = createRouteDataOverrideWarner("`@modular-vue/runtime`", "useZones", "meta");
-
-export function useZones<TZones extends ZoneMapOf<TZones>>(): ComputedRef<Partial<TZones>> {
+const onOverride = createRouteDataOverrideWarner("`@modular-vue/runtime`", "useZones", "meta");
+
+/**
+ * Read zone components contributed by the currently matched route hierarchy.
+ * ...
+ */
+export function useZones<TZones extends ZoneMapOf<TZones>>(): ComputedRef<Partial<TZones>> {📝 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.
Suggested change
| /** | |
| * Read zone components contributed by the currently matched route hierarchy. | |
| * | |
| * Zones ride on vue-router's `meta` (the analog of React Router's `handle`) | |
| * on individual routes. This composable walks all matched records from root | |
| * to leaf via `useRoute().matched` and returns a merged map where the deepest | |
| * match wins for each zone key. | |
| * | |
| * @example | |
| * // In the shell layout's <script setup>: | |
| * const zones = useZones<AppZones>() | |
| * | |
| * // In the template: | |
| * // <main><router-view /></main> | |
| * // <aside><component :is="zones.detailPanel" v-if="zones.detailPanel" /></aside> | |
| * | |
| * @example | |
| * // In a module's createRoutes(): | |
| * { | |
| * path: ':userId', | |
| * component: UserDetailPage, | |
| * meta: { | |
| * detailPanel: UserDetailSidebar, | |
| * } satisfies ModuleRouteMeta<AppZones>, | |
| * } | |
| * | |
| * ## Ownership and overrides | |
| * | |
| * Zones merge with deepest-wins semantics: a descendant route that declares | |
| * the same zone key as an ancestor silently replaces it. That is the | |
| * intended escape hatch for "this section overrides the default panel" — | |
| * but it is also the failure mode when a descendant route accidentally | |
| * declares a zone key the shell layout owns (e.g. `HeaderTitle`, | |
| * `HeaderActions`). | |
| * | |
| * In dev (NODE_ENV !== "production"), this composable logs a deduped | |
| * `console.warn` whenever a deeper match overrides a zone already set by | |
| * an ancestor. Use that warning to catch unintended clobbers; if the | |
| * override is intentional you can ignore it. | |
| * | |
| * To **inherit** an ancestor's zone, omit the key (or set to `undefined`). | |
| * To **explicitly clear** an ancestor's zone at a deeper route, set the | |
| * key to `null`. Don't redeclare a shell-owned zone "just to be safe" — | |
| * that's exactly the pattern the override warning is designed to flag. | |
| * | |
| * ## Return value | |
| * | |
| * Returns a `ComputedRef` driven by `useRoute()`, so the merged map | |
| * recomputes when navigation changes the matched hierarchy. The React | |
| * binding returns a plain object per render; Vue's reactive route makes a | |
| * `computed` the faithful analog. Read `zones.value` in script, or let the | |
| * template auto-unwrap it. | |
| */ | |
| const onOverride = createRouteDataOverrideWarner("@modular-vue/runtime", "useZones", "meta"); | |
| export function useZones<TZones extends ZoneMapOf<TZones>>(): ComputedRef<Partial<TZones>> { | |
| const onOverride = createRouteDataOverrideWarner("`@modular-vue/runtime`", "useZones", "meta"); | |
| /** | |
| * Read zone components contributed by the currently matched route hierarchy. | |
| * | |
| * Zones ride on vue-router's `meta` (the analog of React Router's `handle`) | |
| * on individual routes. This composable walks all matched records from root | |
| * to leaf via `useRoute().matched` and returns a merged map where the deepest | |
| * match wins for each zone key. | |
| * | |
| * `@example` | |
| * // In the shell layout's <script setup>: | |
| * const zones = useZones<AppZones>() | |
| * | |
| * // In the template: | |
| * // <main><router-view /></main> | |
| * // <aside><component :is="zones.detailPanel" v-if="zones.detailPanel" /></aside> | |
| * | |
| * `@example` | |
| * // In a module's createRoutes(): | |
| * { | |
| * path: ':userId', | |
| * component: UserDetailPage, | |
| * meta: { | |
| * detailPanel: UserDetailSidebar, | |
| * } satisfies ModuleRouteMeta<AppZones>, | |
| * } | |
| * | |
| * ## Ownership and overrides | |
| * | |
| * Zones merge with deepest-wins semantics: a descendant route that declares | |
| * the same zone key as an ancestor silently replaces it. That is the | |
| * intended escape hatch for "this section overrides the default panel" — | |
| * but it is also the failure mode when a descendant route accidentally | |
| * declares a zone key the shell layout owns (e.g. `HeaderTitle`, | |
| * `HeaderActions`). | |
| * | |
| * In dev (NODE_ENV !== "production"), this composable logs a deduped | |
| * `console.warn` whenever a deeper match overrides a zone already set by | |
| * an ancestor. Use that warning to catch unintended clobbers; if the | |
| * override is intentional you can ignore it. | |
| * | |
| * To **inherit** an ancestor's zone, omit the key (or set to `undefined`). | |
| * To **explicitly clear** an ancestor's zone at a deeper route, set the | |
| * key to `null`. Don't redeclare a shell-owned zone "just to be safe" — | |
| * that's exactly the pattern the override warning is designed to flag. | |
| * | |
| * ## Return value | |
| * | |
| * Returns a `ComputedRef` driven by `useRoute()`, so the merged map | |
| * recomputes when navigation changes the matched hierarchy. The React | |
| * binding returns a plain object per render; Vue's reactive route makes a | |
| * `computed` the faithful analog. Read `zones.value` in script, or let the | |
| * template auto-unwrap it. | |
| */ | |
| export function useZones<TZones extends ZoneMapOf<TZones>>(): ComputedRef<Partial<TZones>> { |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/vue-router-runtime/src/zones.ts` around lines 6 - 61, The JSDoc
intended for useZones is attached to the internal onOverride constant because it
immediately precedes that declaration. Move the
createRouteDataOverrideWarner("`@modular-vue/runtime`", "useZones", "meta") const
above the doc block, or place the doc directly above export function useZones so
the language service and docs generators associate it with useZones.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part 3 of the
@modular-vue/runtimeport: the zone and route-data composables. Next row in the Vue initiative after PR-22 (#65).What
Adds three composables to
@modular-vue/runtime, each the Vue analog of the same-named file inreact-router-runtime:zones.ts—useZones<TZones>(). React source:react-router-runtime/src/zones.ts.active-zones.ts—useActiveZones<TZones>(activeModuleId?). React source:active-zones.ts.route-data.ts—useRouteData<TRouteData>(). React source:route-data.ts.All three walk
useRoute().matched, read zones/static data off each record'smeta(vue-router's analog of React Router'shandle, per the PR-20route-meta.tsconvention), and merge deepest-wins through core'smergeRouteStaticDatawith acreateRouteDataOverrideWarnerdev warning.useActiveZoneslayers the active module's descriptorzonesover the route zones (module-wins). The runtime index now exports all three.Core change
Two additive changes in
@modular-frontend/core, both anticipated by the warner's own "new runtimes should add their package name here" note:RouteDataRuntimeLabelgains"@modular-vue/runtime"andRouteDataFieldLabelgains"meta", so the Vue composables pass the same compile-time-checked labels the React/TanStack runtimes do.readMatchIdnow falls back to a vue-router record'sname, thenpath, afterid/routeId(vue-router matched records carry noid). Covered by a new frontend-core test.Deviations from the React source (all forced by the framework)
ComputedRefdriven by the reactiveuseRoute(), so the merged map recomputes on navigation.useActiveZonestakesactiveModuleIdas aMaybeRefOrGetter(read viatoValue) so a tab switcher's changing id re-drives the merge.vue-router'suseRoute(mirroring the React suites mockinguseMatches) and assert on.value.slots.test.tspulls the pure helpers from@modular-frontend/coreandcreateSlotsSignalfrom@modular-vue/vue, matching the React source case-for-case.Tests
46 new tests across
zones.test.ts(7),active-zones.test.ts(7),route-data.test.ts(9),slots.test.ts(23); package total 108. Two added reactivity tests cover route-change recompute (useZones) and reactiveactiveModuleIdre-merge (useActiveZones). Full workspace typecheck (120 tasks) andvite build(JS + dts) pass; React and TanStack runtime suites stay green.Tracker (
docs/vue-support-tracker.md) updated: PR-23 marked done with the full write-up.Summary by CodeRabbit
New Features
Bug Fixes