feat(panels): subject-keyed panels (render-all, predicate-gated) - #94
Conversation
…icate-gated) Implements the counter-proposal from the cat-factory slice-4 triage (#93): a render-all, subject-keyed, open-contribution panel surface — the sibling of the pick-one component-pairing helpers — landed engine-first with both bindings in the same train, under the name "panels" (not "zones"). Engine (@modular-frontend/core): - PanelEntry<TSubject>, PanelGroupHandle<TSubject>, definePanelGroup - resolvePanels: pure resolver (null subject -> empty; when(subject) filter; stable order sort; duplicate-id throw with first-wins/last-wins escape hatch mirroring resolveComponentRegistry) - Re-exported by @modular-react/core (export *) and @modular-vue/core Vue (@modular-vue/vue, re-exported from @modular-vue/core): - usePanels (computed over the slots source + subject; prefers the reactive slots source), <PanelsOutlet> (render-all, subject injected as prop + provide, per-panel ModuleErrorBoundary, #empty / #wrap slots), usePanelSubject React (@modular-react/react): - usePanels, <PanelsOutlet>, usePanelSubject / PanelSubjectContext — same surface with useMemo + context Contributions ride the existing module -> slot path; no descriptor change, no new package, no registry plugin, no /testing subpath, no Nuxt manifest threading — all subsumed per the triage. Docs: - docs/subject-panels.md guide (entry shape, resolver, both hosts, the inspector recipe, the Vue reactivity caveat) - compositions README "pick by problem shape" table extended 3 -> 4 primitives; cross-links from reactive-slots-vue.md and remote-capability-manifests.md; triage doc marked resolved Housekeeping (Gap D): widen the Vue family peer ranges (incl. compositions and testing) to admit @modular-frontend/core@^0.3.0. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QULkEZq9fop9povfXdMecL
|
Warning Review limit reached
Next review available in: 34 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughIntroduces subject-keyed panels with typed core resolution, React and Vue hosts, subject injection, predicate filtering, ordering, duplicate handling, remount semantics, documentation, and a React Router inspector example with end-to-end tests. ChangesSubject-keyed panels
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant InspectorShell
participant PanelsOutlet
participant resolvePanels
participant PanelModules
User->>InspectorShell: select a board subject
InspectorShell->>PanelsOutlet: provide subject and panel group
PanelsOutlet->>resolvePanels: resolve contributed entries
resolvePanels->>PanelModules: evaluate when(subject)
PanelModules-->>resolvePanels: return matching panels
resolvePanels-->>PanelsOutlet: return ordered entries
PanelsOutlet-->>InspectorShell: render all matching panels
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
…ots source, shared dedup, boundary labels Code-review + architecture follow-ups on the subject-keyed panels PR: - resolvePanels validates duplicate ids BEFORE the null-subject guard, so a registration bug surfaces deterministically on first resolve — including the usual initial state where nothing is selected yet. New engine tests lock it. - The duplicate-id collapse is now one shared implementation (collapseEntriesById in component-registry.ts) behind both resolveComponentRegistry and resolvePanels, so the two reductions' duplicate stance cannot drift; the redundant post-filter copy in resolvePanels is gone. - Vue usePanels tracks BOTH runtime slot sources — the reactive computed and the recalculateSlots() signal Ref — serving whichever produced the more recent evaluation. Panels contributed via dynamicSlots now update on either path, matching the React host (whose single context carries both). Covered by a both-directions test. - Vue usePanels accepts MaybeRefOrGetter for group and onDuplicate, resolved inside the computed; <PanelsOutlet> passes getters, so its props are live after mount instead of frozen at setup (tested via a post-mount onDuplicate flip). - ModuleErrorBoundary (both bindings) gains an optional `label` prop (default "Module"); <PanelsOutlet> passes "Panel" so a crashed panel isn't mislabeled a module. - React usePanels uses a module-level empty-entries constant so the memo stays stable for uncontributed groups. - New host tests in both bindings: subjectKey remount semantics (state kept without it, remounted with it) and onDuplicate forwarding. - Docs: resolver semantics reordered; new "when vs dynamicSlots" section; props.subject override documented; Vue caveat now covers the contribution side; CHANGELOG gains the missing Unreleased entry. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W5fr1q1oVcmjg9s1zibMXP
Close the doc-discoverability and API-reference gaps around the panels feature: - README.md: add a "Subject-keyed panels" row to the documentation index so the guide is discoverable alongside the sibling primitives. - packages/frontend-core/README.md: document the engine exports (definePanelGroup, resolvePanels, PanelEntry, PanelGroupHandle). - packages/react/README.md: document the React host (usePanels, PanelsOutlet, usePanelSubject, PanelSubjectContext). - packages/vue/README.md: document the Vue host (usePanels, PanelsOutlet, usePanelSubject, panelSubjectKey), with the reactivity-caveat pointer. - docs/subject-panels.md: add a concrete end-to-end walkthrough tying the scattered fragments into one inspector-rail scenario — subject type, group handle, first-party + consumer contributions, and a trace of exactly which panels render (ordered, filtered) across selections, including the open-contribution case where a consumer panel appears with no host edit. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NV2ACUkMCvCgquryU3P5Cb
…uter) Add examples/react-router/inspector-panels — a self-contained pnpm workspace demonstrating the panels feature end to end, matching the sibling examples' conventions (app-shared / modules / shell + Playwright e2e): - app-shared: the BoardBlock subject type, a sample board, and the shared definePanelGroup handle. - modules/inspector-core (first-party): an always-on `identity` panel plus a `frontend-config` panel gated to frame-level frontend blocks; demonstrates both the injected `subject` prop and the `usePanelSubject` context reader. - modules/acme-extras (consumer): a namespaced `acme:security-report` panel for its own `acme-secure` block type, added with no edit to the host — the open-contribution property. - shell: registry wiring plus the board host and the <PanelsOutlet> inspector rail (subjectKey / empty / wrap), and a smoke.spec.ts that asserts the exact filtered/ordered render across selections, the empty state, and re-resolution. Verified: typecheck, vite build, and the Playwright e2e (5/5) all pass. Listed in examples/README.md. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NV2ACUkMCvCgquryU3P5Cb
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (4)
packages/vue/src/panels.test.ts (1)
110-131: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winOrdering assertion doesn't actually verify order.
Both
twoPanelsentries renderProbe, whose text issubject.type, which is identical for both panels here ("frontend"). ThetoEqual(["frontend", "frontend"])assertion would pass even if the resolver stopped sorting byorderentirely, so the "order 10 before order 20" claim in the comment is unverified.(Requires `Probe` to render a `data-id` derived from the entry, e.g. via a prop threaded through.)♻️ Proposed fix using a per-entry data attribute
- it("renders every matching panel, ordered, with the subject as a prop", () => { - seen.length = 0; - const subject: Block = { level: "frame", type: "frontend" }; - const wrapper = mountOutlet(subject); - // order 10 (always) before order 20 (frontend) - expect(wrapper.findAll(".panel").map((n) => n.text())).toEqual(["frontend", "frontend"]); + it("renders every matching panel, ordered, with the subject as a prop", () => { + seen.length = 0; + const subject: Block = { level: "frame", type: "frontend" }; + const wrapper = mountOutlet(subject); + // order 10 (always) before order 20 (frontend) + expect(wrapper.findAll(".panel").map((n) => n.attributes("data-id"))).toEqual([ + "always", + "frontend", + ]);🤖 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/src/panels.test.ts` around lines 110 - 131, Update the ordering test around mountOutlet and the twoPanels entries so rendered output identifies which panel entry produced each node, rather than relying on identical subject.type text. Thread each entry’s id into Probe or otherwise expose it as a data-id, then assert the rendered panel ids are ["always", "frontend"] to verify order 10 precedes order 20 while preserving the subject-prop assertions.examples/react-router/inspector-panels/shell/package.json (1)
13-24: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
@react-router-modules/*dependencies useworkspace:*instead of concrete version ranges. Both manifests declare the published router-module packages withworkspace:*, contradicting this monorepo's established convention (per retrieved learnings) of reservingworkspace:*for app-internal packages and using concrete ranges (e.g.,^2.0.0) forreact-router-modules/*/tanstack-react-modules/*; local linking still works via.npmrc'slink-workspace-packages/prefer-workspace-packagessettings.
examples/react-router/inspector-panels/shell/package.json#L13-L24: change"@react-router-modules/core": "workspace:*"and"@react-router-modules/runtime": "workspace:*"to concrete version ranges.examples/react-router/inspector-panels/modules/inspector-core/package.json#L17-L21: change"@react-router-modules/core": "workspace:*"to a concrete version range.🤖 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 `@examples/react-router/inspector-panels/shell/package.json` around lines 13 - 24, Replace the `@react-router-modules/core` and `@react-router-modules/runtime` workspace:* specifications in examples/react-router/inspector-panels/shell/package.json (lines 13-24) with concrete version ranges, and replace `@react-router-modules/core` workspace:* in examples/react-router/inspector-panels/modules/inspector-core/package.json (lines 17-21) likewise. Preserve workspace:* for app-internal packages and align the concrete ranges with the published router-module package versions.Source: Learnings
examples/react-router/inspector-panels/shell/e2e/smoke.spec.ts (1)
33-39: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win
renderedPanelssnapshot lacks Playwright's auto-retry, unlike neighboring assertions.Every other assertion in this file (
toHaveText,toBeVisible,toHaveCount) auto-retries;renderedPanelsis a one-shotevaluateAllinvoked immediately after a click (lines 55-59, 71-73, 84-88, 99-109), so a render that lags a tick behind the click could produce a stale/flaky read.♻️ Proposed fix using `expect.poll`
-async function renderedPanels(page: Page): Promise<string[]> { - return page - .getByTestId("inspector") - .locator("[data-panel]") - .evaluateAll((els) => els.map((el) => el.getAttribute("data-panel") ?? "")); -} +async function renderedPanels(page: Page): Promise<string[]> { + return page + .getByTestId("inspector") + .locator("[data-panel]") + .evaluateAll((els) => els.map((el) => el.getAttribute("data-panel") ?? "")); +} + +async function expectRenderedPanels(page: Page, expected: string[]) { + await expect.poll(() => renderedPanels(page)).toEqual(expected); +}Then replace
expect(await renderedPanels(page)).toEqual([...])call sites withawait expectRenderedPanels(page, [...]).🤖 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 `@examples/react-router/inspector-panels/shell/e2e/smoke.spec.ts` around lines 33 - 39, Update the renderedPanels-based assertions to use Playwright auto-retry: add an expectRenderedPanels helper that polls renderedPanels(page) and asserts the expected panel IDs, then replace every direct expect(await renderedPanels(page)).toEqual(...) call site with await expectRenderedPanels(page, [...]).packages/react/src/panels.test.tsx (1)
56-71: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winOrdering assertion doesn't actually verify order.
Both panels render
subject.type, which is identical for both entries here, so["frontend", "frontend"]would pass regardless of whetheralways(order 10) orfrontend(order 20) rendered first. The comment on line 66 claims this proves ordering, but the assertion can't distinguish entry position.♻️ Proposed fix using the `wrap` render-prop to surface entry id
it("renders every matching panel, ordered, with the subject as a prop", () => { const { container } = render( - <PanelsOutlet group={group} subject={{ level: "frame", type: "frontend" }} />, + <PanelsOutlet + group={group} + subject={{ level: "frame", type: "frontend" }} + wrap={({ entry, children }) => ( + <div data-id={entry.id}>{children}</div> + )} + />, { wrapper: withSlots(twoPanels) }, ); - // order 10 (always) before order 20 (frontend); both get subject.type. - expect([...container.querySelectorAll(".panel")].map((n) => n.textContent)).toEqual([ - "frontend", - "frontend", - ]); + // order 10 (always) before order 20 (frontend). + expect([...container.querySelectorAll("[data-id]")].map((n) => n.getAttribute("data-id"))).toEqual([ + "always", + "frontend", + ]); });🤖 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/react/src/panels.test.tsx` around lines 56 - 71, Update the “renders every matching panel, ordered...” test to make panel output identify the slot entry, using the available wrap render-prop or equivalent entry-id marker. Keep the expected sequence asserting the order-10 “always” panel before the order-20 “frontend” panel, while still verifying both receive the subject.
🤖 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/subject-panels.md`:
- Around line 137-149: Update the React import in the definePanelGroup example
to use `@modular-react/core` instead of `@modular-react/react`; leave the Vue
import example unchanged.
In `@packages/vue/README.md`:
- Around line 25-34: Update the subject-keyed panels bullet in the panels
documentation so the phrase beginning “The Vue host over the framework-neutral
engine” includes an appropriate verb and reads as a complete sentence, while
preserving the existing links and technical details.
In `@packages/vue/src/panels.ts`:
- Around line 165-172: Clarify the `subjectKey` contract used by `keyFor`:
narrow the prop type to the function form if remount behavior is intended to
depend on the subject, or document the static string form’s separate purpose if
it must remain supported. Ensure the type and documentation do not imply that a
static string changes with `subject`.
---
Nitpick comments:
In `@examples/react-router/inspector-panels/shell/e2e/smoke.spec.ts`:
- Around line 33-39: Update the renderedPanels-based assertions to use
Playwright auto-retry: add an expectRenderedPanels helper that polls
renderedPanels(page) and asserts the expected panel IDs, then replace every
direct expect(await renderedPanels(page)).toEqual(...) call site with await
expectRenderedPanels(page, [...]).
In `@examples/react-router/inspector-panels/shell/package.json`:
- Around line 13-24: Replace the `@react-router-modules/core` and
`@react-router-modules/runtime` workspace:* specifications in
examples/react-router/inspector-panels/shell/package.json (lines 13-24) with
concrete version ranges, and replace `@react-router-modules/core` workspace:* in
examples/react-router/inspector-panels/modules/inspector-core/package.json
(lines 17-21) likewise. Preserve workspace:* for app-internal packages and align
the concrete ranges with the published router-module package versions.
In `@packages/react/src/panels.test.tsx`:
- Around line 56-71: Update the “renders every matching panel, ordered...” test
to make panel output identify the slot entry, using the available wrap
render-prop or equivalent entry-id marker. Keep the expected sequence asserting
the order-10 “always” panel before the order-20 “frontend” panel, while still
verifying both receive the subject.
In `@packages/vue/src/panels.test.ts`:
- Around line 110-131: Update the ordering test around mountOutlet and the
twoPanels entries so rendered output identifies which panel entry produced each
node, rather than relying on identical subject.type text. Thread each entry’s id
into Probe or otherwise expose it as a data-id, then assert the rendered panel
ids are ["always", "frontend"] to verify order 10 precedes order 20 while
preserving the subject-prop assertions.
🪄 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: d9b527e6-0d1b-47b5-b0ff-6e5696161059
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (53)
CHANGELOG.mdREADME.mddocs/reactive-slots-vue.mddocs/remote-capability-manifests.mddocs/subject-keyed-zones-triage.mddocs/subject-panels.mdexamples/README.mdexamples/react-router/inspector-panels/README.mdexamples/react-router/inspector-panels/app-shared/package.jsonexamples/react-router/inspector-panels/app-shared/src/app-types.tsexamples/react-router/inspector-panels/app-shared/src/board.tsexamples/react-router/inspector-panels/app-shared/src/index.tsexamples/react-router/inspector-panels/app-shared/tsconfig.jsonexamples/react-router/inspector-panels/modules/acme-extras/package.jsonexamples/react-router/inspector-panels/modules/acme-extras/src/index.tsxexamples/react-router/inspector-panels/modules/acme-extras/tsconfig.jsonexamples/react-router/inspector-panels/modules/inspector-core/package.jsonexamples/react-router/inspector-panels/modules/inspector-core/src/index.tsxexamples/react-router/inspector-panels/modules/inspector-core/tsconfig.jsonexamples/react-router/inspector-panels/shell/e2e/smoke.spec.tsexamples/react-router/inspector-panels/shell/index.htmlexamples/react-router/inspector-panels/shell/package.jsonexamples/react-router/inspector-panels/shell/playwright.config.tsexamples/react-router/inspector-panels/shell/src/components/Home.tsxexamples/react-router/inspector-panels/shell/src/components/Layout.tsxexamples/react-router/inspector-panels/shell/src/main.tsxexamples/react-router/inspector-panels/shell/tsconfig.jsonexamples/react-router/inspector-panels/shell/vite.config.tspackages/compositions/README.mdpackages/frontend-core/README.mdpackages/frontend-core/src/component-registry.tspackages/frontend-core/src/index.tspackages/frontend-core/src/panels.test-d.tspackages/frontend-core/src/panels.test.tspackages/frontend-core/src/panels.tspackages/react/README.mdpackages/react/src/error-boundary.tsxpackages/react/src/index.tspackages/react/src/panels.test.tsxpackages/react/src/panels.tsxpackages/vue-compositions/package.jsonpackages/vue-core/package.jsonpackages/vue-core/src/index.tspackages/vue-journeys/package.jsonpackages/vue-nuxt/package.jsonpackages/vue-runtime/package.jsonpackages/vue-testing/package.jsonpackages/vue/README.mdpackages/vue/package.jsonpackages/vue/src/error-boundary.tspackages/vue/src/index.tspackages/vue/src/panels.test.tspackages/vue/src/panels.ts
… fix README fragment, clarify subjectKey string form - docs/subject-panels.md: import definePanelGroup from @modular-react/core (the /react host layer does not re-export it); both examples - packages/vue/README.md: complete the sentence fragment in the panels bullet - panels docstrings (React + Vue) and docs: clarify that subjectKey is re-read each render, so the string form is a caller-computed discriminator (a literal constant never drives remounts), while the function form derives it from the subject Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SDhW7GZMkb4pSCwKVch4tC
What & why
Implements the counter-proposal from the cat-factory slice-4 triage (#93,
docs/subject-keyed-zones-triage.md): a render-all, subject-keyed, open-contribution panel surface — the sibling of the pick-one component-pairing helpers. A named region whose module-contributed panels are selected at render time by a runtime subject, gated by per-panelwhen(subject)predicates, ordered, and rendered all-matching. Any module (first-party or a consumer's) contributes through the existingmodule.slotspath.Landed the way the triage directed — engine-first, both bindings in the same train, under the name panels (the triage rejects a third "zones" meaning with prejudice). The three open maintainer decisions all resolved to the recommended option: name
panels; React host in the same train; duplicate-idthrowby default with afirst-wins/last-winsescape hatch.Engine (
@modular-frontend/core)PanelEntry<TSubject>,PanelGroupHandle<TSubject>,definePanelGroup(slotKey)resolvePanels(entries, subject, opts?)— pure resolver: null subject → empty;when(subject)filter; stableordersort (ties keep contribution order); duplicate-id throw, mirroringresolveComponentRegistry@modular-react/core(export *) and@modular-vue/coreVue (
@modular-vue/vue, re-exported from@modular-vue/core)usePanels(computedover the slots source + subject; prefers the reactive slots source),<PanelsOutlet>(render-all; subject injected as a prop and viaprovide; per-panelModuleErrorBoundary;#empty/#wrapslots),usePanelSubjectReact (
@modular-react/react)usePanels,<PanelsOutlet>,usePanelSubject/PanelSubjectContext— the same surface withuseMemo+ contextNo descriptor change, no new package, no registry plugin, no
/testingsubpath, no Nuxt manifest threading — all subsumed by the module → slot path, per the triage. Also widens the Vue family peer ranges (incl.compositionsandtesting) to admit@modular-frontend/core@^0.3.0(the triage's Gap-D housekeeping).How it was verified
panels.test.ts+panels.test-d.ts; Vuepanels.test.ts(mounts<PanelsOutlet>— ordering, subject injection,#empty/#wrap, error-boundary containment,usePanelSubject, reactive re-resolution); Reactpanels.test.tsx(parallel coverage).pnpm turbo run test— 79/79 tasks pass;pnpm turbo run typecheck— 152/152 pass.pnpm lint:fix(oxfmt--write .+ oxlint--fix .) run over the repo; the new files produce no lint warnings.Checklist
pnpm lintpasses (typecheck + oxfmt + oxlint).pnpm test).docs/subject-panels.md; compositions README "pick by problem shape" table extended 3 → 4 primitives; cross-links fromreactive-slots-vue.mdandremote-capability-manifests.md; triage doc marked resolved.@modular-frontend/core, and both bindings ship their hosts in this same PR (Vue@modular-vue/vue+ React@modular-react/react), keeping the engine-first, both-bindings rule intact. Angular getsinjectPanelswhen its gate opens — the engine part is already done for it.🤖 Generated with Claude Code
https://claude.ai/code/session_01QULkEZq9fop9povfXdMecL
Generated by Claude Code
Summary by CodeRabbit
New Features
Documentation
Tests