feat(vue): allow vue-router 5 as a peer; document layer consumer seam - #87
Conversation
Widen the `vue-router` peer range on @modular-vue/{core,runtime,nuxt,testing}
from `^4.5.0` to `^4.5.0 || ^5.0.0`. vue-router 5 is API-compatible for the
route grafting the runtime uses (addRoute / beforeEach / RouteRecordRaw), and a
production consumer on Nuxt (which now ships vue-router 5) otherwise gets an
unmet-peer warning on install. Dev dependency stays on ^4.5.0 — bumping the test
matrix to v5 is a separate maintainer decision.
Also document the Nuxt-layer consumer-contribution pattern in
docs/framework-mode-nuxt.md: a layer that owns a base registry and lets each
`extends`ing deployment add its own modules via a `registerAppModule` seam, why
this needs Option B (the module's `~/modular/registry` path resolves to the
consumer's srcDir under a layer), and the `enforce: "post"` plugin-ordering that
makes consumer registration run before the layer resolves.
Surfaced by the cat-factory production adoption (slice 0 of its modular-vue
adoption).
📝 WalkthroughWalkthroughThe PR documents Nuxt layer module composition and expands Vue Router peer dependency support from v4.5.0-only to include v5.0.0 across four Vue packages. ChangesNuxt integration support
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/framework-mode-nuxt.md`:
- Around line 239-257: Replace the module-level contributed array in the
registerAppModule/buildRegistry flow with request-scoped state stored in a
WeakMap keyed by the synchronous useNuxtApp() result. Ensure both functions
retrieve the current Nuxt app, register and read contributions from that app’s
collection, and preserve firstParty modules while preventing state from leaking
or duplicating across SSR requests.
- Around line 293-298: Update the SSR caveat paragraph in the Nuxt
framework-mode documentation to state that the WeakMap-backed,
useNuxtApp()-based request-scoped state pattern is safe under SSR, while
retaining the simpler client-only behavior and duplicate-id validation details.
🪄 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: ab524cf7-5ed2-4927-b553-0385d1d8d518
📒 Files selected for processing (5)
docs/framework-mode-nuxt.mdpackages/vue-core/package.jsonpackages/vue-nuxt/package.jsonpackages/vue-runtime/package.jsonpackages/vue-testing/package.json
| ```ts | ||
| // layer: app/modular/registry.ts | ||
| import { createRegistry } from "@modular-vue/runtime"; | ||
| import type { AnyModuleDescriptor } from "@modular-vue/core"; | ||
|
|
||
| const firstParty: readonly AnyModuleDescriptor[] = [/* the layer's own modules */]; | ||
| const contributed: AnyModuleDescriptor[] = []; | ||
|
|
||
| /** Consumers call this from their own plugin, before the layer resolves. */ | ||
| export function registerAppModule(module: AnyModuleDescriptor): void { | ||
| contributed.push(module); | ||
| } | ||
|
|
||
| export function buildRegistry() { | ||
| const registry = createRegistry({}); | ||
| for (const m of [...firstParty, ...contributed]) registry.register(m); | ||
| return registry; | ||
| } | ||
| ``` |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
Fix cross-request state leak in SSR.
The module-level contributed array will grow indefinitely across requests in Nuxt's default SSR mode. Because plugins execute on every request, the array will accumulate duplicate references (causing a memory leak) and will throw a duplicate-ID error starting on the second request, breaking the app in production.
To make this universally safe for SSR without triggering TypeScript interface augmentation errors, tie the state to the current request using a WeakMap keyed by nuxtApp. Both registerAppModule and buildRegistry run within Nuxt plugin contexts, so useNuxtApp() works synchronously.
🔧 Proposed fix for an SSR-safe plugin pattern
// layer: app/modular/registry.ts
+import { useNuxtApp } from "`#app`";
import { createRegistry } from "`@modular-vue/runtime`";
import type { AnyModuleDescriptor } from "`@modular-vue/core`";
const firstParty: readonly AnyModuleDescriptor[] = [/* the layer's own modules */];
-const contributed: AnyModuleDescriptor[] = [];
+const contributed = new WeakMap<object, AnyModuleDescriptor[]>();
/** Consumers call this from their own plugin, before the layer resolves. */
export function registerAppModule(module: AnyModuleDescriptor): void {
- contributed.push(module);
+ const nuxtApp = useNuxtApp();
+ if (!contributed.has(nuxtApp)) contributed.set(nuxtApp, []);
+ contributed.get(nuxtApp)!.push(module);
}
export function buildRegistry() {
const registry = createRegistry({});
- for (const m of [...firstParty, ...contributed]) registry.register(m);
+ const nuxtApp = useNuxtApp();
+ const requestContributed = contributed.get(nuxtApp) || [];
+ for (const m of [...firstParty, ...requestContributed]) registry.register(m);
return registry;
}📝 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.
| ```ts | |
| // layer: app/modular/registry.ts | |
| import { createRegistry } from "@modular-vue/runtime"; | |
| import type { AnyModuleDescriptor } from "@modular-vue/core"; | |
| const firstParty: readonly AnyModuleDescriptor[] = [/* the layer's own modules */]; | |
| const contributed: AnyModuleDescriptor[] = []; | |
| /** Consumers call this from their own plugin, before the layer resolves. */ | |
| export function registerAppModule(module: AnyModuleDescriptor): void { | |
| contributed.push(module); | |
| } | |
| export function buildRegistry() { | |
| const registry = createRegistry({}); | |
| for (const m of [...firstParty, ...contributed]) registry.register(m); | |
| return registry; | |
| } | |
| ``` |
🤖 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/framework-mode-nuxt.md` around lines 239 - 257, Replace the module-level
contributed array in the registerAppModule/buildRegistry flow with
request-scoped state stored in a WeakMap keyed by the synchronous useNuxtApp()
result. Ensure both functions retrieve the current Nuxt app, register and read
contributions from that app’s collection, and preserve firstParty modules while
preventing state from leaking or duplicating across SSR requests.
| For an `ssr: false` layer this is the simple case: the plugin runs once on the | ||
| client, so the module-level `contributed` array and a singleton registry are | ||
| fine (the per-request-factory rule only bites under SSR). Registering the same | ||
| id twice throws at resolve via duplicate-id validation, which is the guard you | ||
| want. | ||
|
|
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Update SSR caveats to reflect request-scoped state.
If you apply the WeakMap and useNuxtApp() fix to the code snippet above, this paragraph should be updated to reflect that the pattern is now completely safe for SSR.
📝 Proposed documentation update
-For an `ssr: false` layer this is the simple case: the plugin runs once on the
-client, so the module-level `contributed` array and a singleton registry are
-fine (the per-request-factory rule only bites under SSR). Registering the same
-id twice throws at resolve via duplicate-id validation, which is the guard you
-want.
+By tying the contributed modules to the `nuxtApp` instance via a `WeakMap`, the
+list is safely scoped to the current request in SSR, preventing memory leaks and
+cross-request state pollution. If a consumer accidentally registers the same
+module id twice in their plugin, duplicate-id validation will safely throw at
+resolve, which is the guard you want.📝 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.
| For an `ssr: false` layer this is the simple case: the plugin runs once on the | |
| client, so the module-level `contributed` array and a singleton registry are | |
| fine (the per-request-factory rule only bites under SSR). Registering the same | |
| id twice throws at resolve via duplicate-id validation, which is the guard you | |
| want. | |
| By tying the contributed modules to the `nuxtApp` instance via a `WeakMap`, the | |
| list is safely scoped to the current request in SSR, preventing memory leaks and | |
| cross-request state pollution. If a consumer accidentally registers the same | |
| module id twice in their plugin, duplicate-id validation will safely throw at | |
| resolve, which is the guard you want. |
🤖 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/framework-mode-nuxt.md` around lines 293 - 298, Update the SSR caveat
paragraph in the Nuxt framework-mode documentation to state that the
WeakMap-backed, useNuxtApp()-based request-scoped state pattern is safe under
SSR, while retaining the simpler client-only behavior and duplicate-id
validation details.
Bump to the released versions carrying the vue-router `^4.5.0 || ^5.0.0` peer (kibertoad/modular-react#87): @modular-vue/core@^1.0.1, runtime@^1.0.1, nuxt@^0.1.1. Closes the slice-0 co-evolution tail — `pnpm peers check` is now clean against Nuxt's vue-router 5. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* feat(app): modular-vue registry in the layer (adoption slice 0) Wire a @modular-vue/runtime registry and a client install plugin into the @cat-factory/app Nuxt layer behind zero behaviour change — slice 0 of the modular-vue adoption (docs/initiatives/modular-vue-adoption.md). This is the frontend analogue of the backend's public registries (registerAgentKind / registerGate): one registry into which first-party feature modules AND a consumer deployment's own modules register through the same seam. - app/modular/registry.ts: createAppRegistry() builds the registry from the first-party modules plus everything a consumer contributed via registerAppModule(); one tiny non-rendering first-party module anchors the pipeline for now. - app/utils/modular.ts: re-exports registerAppModule so a consumer gets it as a layer auto-import (same ergonomics as the layer's stores/composables). - app/plugins/modular.client.ts: installs the registry with installModularApp. enforce: 'post' so a consumer's default-order registration plugin runs BEFORE the layer resolves. Nothing reads the manifest yet — behaviour-neutral. - app/modular/registry.spec.ts: covers first-party + consumer registration and the duplicate-id guard. Consumer seam decision: Option B (the layer ships its own plugin), not the serializable Nuxt module — that module's ~/modular/registry path resolves to the consumer's srcDir under extends, and it can't take the slotFilter slice 1 needs. Raise the workspace vue pin 3.5.39 -> 3.5.40 (the @modular-vue/* peer floor; one singleton preserved) and add @modular-vue/* + @modular-frontend/* to minimumReleaseAgeExclude (namespaces we own). Co-evolution: cat-factory is on vue-router 5 (Nuxt owns it) while @modular-vue/* peer-require ^4.5.0 — a non-fatal warning (our zero-route path never touches the router). Filed upstream kibertoad/modular-react#87 to widen the peer to ^4.5.0 || ^5.0.0; re-adopt = bump to that release once published. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs: cite PR #1193 in the modular-vue slice 0 tracker row * chore(app): re-adopt @modular-vue with widened vue-router peer Bump to the released versions carrying the vue-router `^4.5.0 || ^5.0.0` peer (kibertoad/modular-react#87): @modular-vue/core@^1.0.1, runtime@^1.0.1, nuxt@^0.1.1. Closes the slice-0 co-evolution tail — `pnpm peers check` is now clean against Nuxt's vue-router 5. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
What
Two changes surfaced by the cat-factory production adoption (slice 0 of its modular-vue adoption):
vue-routerpeer on@modular-vue/{core,runtime,nuxt,testing}from^4.5.0to^4.5.0 || ^5.0.0. A consumer on current Nuxt now gets vue-router 5, which produces an unmet-peer warning against the^4.5.0-only range. vue-router 5 is API-compatible for what the runtime uses (addRoute/beforeEach/RouteRecordRaw).docs/framework-mode-nuxt.md: a layer that owns a base registry and lets eachextendsing deployment add its own modules through aregisterAppModule-style seam.Why the doc section
The zero-config Nuxt module (Option A) resolves its
registry: "~/modular/registry"path against the consumer's srcDir under a layer, so the layer's base modules and the consumer's additions don't compose. The documented pattern uses Option B (the layer ships its own plugin) plusenforce: "post"so the consumer's default-order registration plugin runs before the layer resolves the registry.Scope notes
^4.5.0— moving the test matrix to v5 is a separate maintainer call, kept out of this compat PR.🤖 Generated with Claude Code
Summary by CodeRabbit
Compatibility
Documentation