feat(dashboard): Add support for custom React providers in dashboard#4600
feat(dashboard): Add support for custom React providers in dashboard#4600alingabrieldm wants to merge 8 commits into
Conversation
|
Vendure Core — View preview |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughAdds a new "Custom Providers" dashboard feature and documentation. Introduces DashboardCustomProviderDefinition and registry-backed APIs (registerDashboardCustomProvider(s), getDashboardCustomProvidersRegistry), a renderProviders helper, and a CustomProviders React component. Hooks custom providers into the extension system via defineDashboardExtension and the global registry. AppProviders and AppLayout now render CustomProviders for 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
packages/dashboard/src/lib/framework/extension-api/custom-providers.ts (1)
74-77: UseReadonly<>for component props type.Please wrap the props object in
Readonly<>for consistency with dashboard typing rules.♻️ Proposed refactor
+type CustomProvidersProps = Readonly<{ + location: DashboardCustomProviderDefinition['location']; + children: ReactNode; +}>; + export const CustomProviders = ({ location, children, -}: { - location: DashboardCustomProviderDefinition['location']; - children: ReactNode; -}) => { +}: CustomProvidersProps) => {As per coding guidelines, "Set React component props objects to Readonly<> type for type safety".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/dashboard/src/lib/framework/extension-api/custom-providers.ts` around lines 74 - 77, Wrap the component's props object type in Readonly<> to follow dashboard typing rules: change the anonymous props type used in the arrow component (the object containing location: DashboardCustomProviderDefinition['location'] and children: ReactNode) to Readonly<{ location: DashboardCustomProviderDefinition['location']; children: ReactNode }>, updating the component signature where that props destructuring occurs in custom-providers.ts so the props are immutable per the guideline.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/docs/guides/extending-the-dashboard/custom-providers/index.mdx`:
- Around line 16-17: The file imports ReactNode directly from 'react' (the
import statement for ReactNode at Lines ~16-17 and again at ~27-28); update the
example to import ReactNode via the dashboard package public API instead (i.e.,
re-export the missing ReactNode type from vendure/dashboard and then change the
import to pull ReactNode from that package), so all extension examples only
import from vendure/dashboard and no third-party modules directly.
In `@packages/dashboard/src/lib/framework/extension-api/custom-providers.ts`:
- Around line 78-84: The memoized providers list in CustomProviders
(providersToRender computed via useMemo) misses the extensions bootstrap signal,
causing an empty list on first render; update the dependency array of the
useMemo that references getDashboardCustomProvidersRegistry() to include
extensionsLoaded from useDashboardExtensions() (in addition to reloadCount and
location) so providersToRender recomputes after extensions load, and update the
component props/type declarations to use Readonly<...> per the file's coding
guidelines (touch the CustomProviders props/type definitions and any exported
prop interfaces).
In
`@packages/dashboard/src/lib/framework/extension-api/define-dashboard-extension.ts`:
- Around line 122-124: The call to
registerDashboardCustomProviders(extension.customProviders) can silently
overwrite providers with duplicate ids; update the registration logic (in
custom-providers.ts / the registerDashboardCustomProviders function) to validate
ids before inserting: detect duplicates within the incoming
extension.customProviders and against the existing global provider registry, and
then either throw an informative error or emit a logged warning including the
conflicting provider id and both provider sources; do not perform the overwrite
without explicit handling. Ensure the check references the provider id field and
include the extension identifier when reporting conflicts so callers can resolve
collisions.
---
Nitpick comments:
In `@packages/dashboard/src/lib/framework/extension-api/custom-providers.ts`:
- Around line 74-77: Wrap the component's props object type in Readonly<> to
follow dashboard typing rules: change the anonymous props type used in the arrow
component (the object containing location:
DashboardCustomProviderDefinition['location'] and children: ReactNode) to
Readonly<{ location: DashboardCustomProviderDefinition['location']; children:
ReactNode }>, updating the component signature where that props destructuring
occurs in custom-providers.ts so the props are immutable per the guideline.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d39ad373-e21b-401e-8a8d-0ccce7e0b439
📒 Files selected for processing (9)
docs/docs/guides/extending-the-dashboard/custom-providers/index.mdxdocs/src/manifest.tspackages/dashboard/src/app/app-providers.tsxpackages/dashboard/src/lib/components/layout/app-layout.tsxpackages/dashboard/src/lib/framework/extension-api/custom-providers.tspackages/dashboard/src/lib/framework/extension-api/define-dashboard-extension.tspackages/dashboard/src/lib/framework/extension-api/extension-api-types.tspackages/dashboard/src/lib/framework/registry/registry-types.tspackages/dashboard/src/lib/index.ts
| import type { ReactNode } from 'react'; | ||
|
|
There was a problem hiding this comment.
Avoid direct react imports in extension examples.
The snippets at Line 16-Line 17 and Line 27-Line 28 import ReactNode from react. This conflicts with the extension import policy and should be rewritten to use the dashboard package public API (and re-export any missing type there first).
Based on learnings: Extensions must import everything from vendure/dashboard — never from third-party libraries directly. Always re-export new dependencies or components from this package's public API.
Also applies to: 27-28
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/docs/guides/extending-the-dashboard/custom-providers/index.mdx` around
lines 16 - 17, The file imports ReactNode directly from 'react' (the import
statement for ReactNode at Lines ~16-17 and again at ~27-28); update the example
to import ReactNode via the dashboard package public API instead (i.e.,
re-export the missing ReactNode type from vendure/dashboard and then change the
import to pull ReactNode from that package), so all extension examples only
import from vendure/dashboard and no third-party modules directly.
| // Register dashboard custom providers | ||
| registerDashboardCustomProviders(extension.customProviders); | ||
|
|
There was a problem hiding this comment.
Prevent silent custom provider ID collisions.
At Line 123, providers are registered without collision feedback. Since registration is keyed by id, duplicates can silently overwrite earlier providers, making extension composition order-dependent.
Suggested guard (in custom-providers.ts)
export function registerDashboardCustomProvider(customProvider: DashboardCustomProviderDefinition) {
globalRegistry.set('dashboardCustomProvidersRegistry', map => {
+ if (map.has(customProvider.id)) {
+ throw new Error(
+ `Duplicate dashboard custom provider id "${customProvider.id}". ` +
+ `Provider ids must be unique across extensions.`,
+ );
+ }
map.set(customProvider.id, {
...customProvider,
location: customProvider.location ?? 'app',
});
return map;
});
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@packages/dashboard/src/lib/framework/extension-api/define-dashboard-extension.ts`
around lines 122 - 124, The call to
registerDashboardCustomProviders(extension.customProviders) can silently
overwrite providers with duplicate ids; update the registration logic (in
custom-providers.ts / the registerDashboardCustomProviders function) to validate
ids before inserting: detect duplicates within the incoming
extension.customProviders and against the existing global provider registry, and
then either throw an informative error or emit a logged warning including the
conflicting provider id and both provider sources; do not perform the overwrite
without explicit handling. Ensure the check references the provider id field and
include the extension identifier when reporting conflicts so callers can resolve
collisions.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/dashboard/src/lib/framework/extension-api/custom-providers.ts`:
- Around line 35-46: getDashboardCustomProvidersRegistry currently exposes the
live Map and registerDashboardCustomProvider silently overwrites existing
entries, allowing last-writer-wins; change getDashboardCustomProvidersRegistry
to return an immutable/read-only copy (e.g., new Map(map) or Object.freeze
wrapper) so callers cannot mutate the live registry, and update
registerDashboardCustomProvider to perform a collision check inside its updater
(use map.has(customProvider.id)) and throw an error if the id already exists
before calling map.set; reference getDashboardCustomProvidersRegistry and
registerDashboardCustomProvider and the 'dashboardCustomProvidersRegistry'
registry key when making the changes.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f5492638-7611-4fcb-8fdb-cd3f9fd9fcae
📒 Files selected for processing (2)
docs/docs/guides/extending-the-dashboard/custom-providers/index.mdxpackages/dashboard/src/lib/framework/extension-api/custom-providers.ts
✅ Files skipped from review due to trivial changes (1)
- docs/docs/guides/extending-the-dashboard/custom-providers/index.mdx
| export function getDashboardCustomProvidersRegistry() { | ||
| return globalRegistry.get('dashboardCustomProvidersRegistry'); | ||
| } | ||
|
|
||
| export function registerDashboardCustomProvider(customProvider: DashboardCustomProviderDefinition) { | ||
| globalRegistry.set('dashboardCustomProvidersRegistry', map => { | ||
| map.set(customProvider.id, { | ||
| ...customProvider, | ||
| location: customProvider.location ?? 'app', | ||
| }); | ||
| return map; | ||
| }); |
There was a problem hiding this comment.
Enforce duplicate-id checks on every public write path.
registerDashboardCustomProviders() rejects collisions, but the two exports here still allow silent last-writer-wins behavior: getDashboardCustomProvidersRegistry() returns the live Map, and registerDashboardCustomProvider() overwrites existing entries with map.set(). That makes the “globally unique” invariant depend on which helper the caller uses.
Suggested hardening
-export function getDashboardCustomProvidersRegistry() {
+function getDashboardCustomProvidersRegistry() {
return globalRegistry.get('dashboardCustomProvidersRegistry');
}
+
+export function getRegisteredDashboardCustomProviders(): ReadonlyMap<
+ string,
+ DashboardCustomProviderDefinition
+> {
+ return getDashboardCustomProvidersRegistry();
+}
export function registerDashboardCustomProvider(customProvider: DashboardCustomProviderDefinition) {
+ if (getDashboardCustomProvidersRegistry().has(customProvider.id)) {
+ throw new Error(
+ `Duplicate dashboard custom provider ids detected: "${customProvider.id}". ` +
+ `Provider ids must be globally unique.`,
+ );
+ }
globalRegistry.set('dashboardCustomProvidersRegistry', map => {
map.set(customProvider.id, {
...customProvider,
location: customProvider.location ?? 'app',
});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/dashboard/src/lib/framework/extension-api/custom-providers.ts`
around lines 35 - 46, getDashboardCustomProvidersRegistry currently exposes the
live Map and registerDashboardCustomProvider silently overwrites existing
entries, allowing last-writer-wins; change getDashboardCustomProvidersRegistry
to return an immutable/read-only copy (e.g., new Map(map) or Object.freeze
wrapper) so callers cannot mutate the live registry, and update
registerDashboardCustomProvider to perform a collision check inside its updater
(use map.has(customProvider.id)) and throw an error if the id already exists
before calling map.set; reference getDashboardCustomProvidersRegistry and
registerDashboardCustomProvider and the 'dashboardCustomProvidersRegistry'
registry key when making the changes.
michaelbromley
left a comment
michaelbromley
left a comment
There was a problem hiding this comment.
Thanks for this PR — the approach is sound and follows the established extension registry pattern well. A few things to address before we can merge:
Target branch
This PR adds a new feature, so it should target the minor branch rather than master. The @since tag should be 3.7.0.
Documentation / docgen
The DashboardCustomProviderDefinition type needs proper JSDoc annotations to be picked up by our documentation generator. Currently the type has plain comments but is missing the required tags. Following the pattern established by DashboardToolbarItemDefinition and DashboardAlertDefinition:
- Top-level doc block on the type itself needs
@description,@docsCategory extensions-api,@docsPage Custom Providers(or similar), and@since 3.7.0. - Every property needs a
@descriptiontag — without it, properties are invisible to docgen. e.g.:/** * @description * A unique identifier for this custom provider. */ id: string;
- The
customProvidersfield onDashboardExtensionalso needs@since 3.7.0.
Docs precision: location descriptions
The location: 'layout' description (both in the type JSDoc and in the docs page) should clarify that it wraps only the <Outlet /> — not the sidebar or header. This distinction matters for use cases like error boundaries. Something like:
Wraps the main content area of the authenticated layout. The sidebar and header are outside this wrapper.
Tests (nice to have)
There are no tests for renderProviders (recursive nesting, ordering) or the duplicate ID detection in registerDashboardCustomProviders. The existing define-dashboard-extension.spec.ts would be the natural home for these. Not a blocker, but would be good to have for completeness.
|
Hi @michaelbromley, thanks for the feedback! I've added two more commits covering documentation and tests. Also, I've updated the target branch to |
2 participants
Description
This PR adds custom React provider support to Vendure Dashboard extensions. Extensions can now register
customProvidersviadefineDashboardExtension(), which are rendered at theapporlayoutlevel (with deterministic ordering), enabling cross-cutting concerns like context, feature flags, error boundaries, etc.Adds a new
Custom Providersguide underExtending the Dashboardand wires it into the docs navigation manifest.Breaking changes
None.
Checklist
📌 Always:
👍 Most of the time: