From bc9c4df70c1e049c093f388cc4324534237cfb0f Mon Sep 17 00:00:00 2001 From: Vlad Babich Date: Fri, 10 Apr 2026 11:33:00 -0600 Subject: [PATCH 01/25] feat: Middleware plugin infrastructure for widget chaining Add support for middleware plugins that can wrap and enhance widget plugins. Multiple middleware plugins can target the same widget type and are chained in registration order (first registered = outermost wrapper). - Add WidgetMiddlewarePlugin interface with isMiddleware marker - Add WidgetMiddlewareComponentProps and WidgetMiddlewarePanelProps - Add isWidgetMiddlewarePlugin() type guard - Update WidgetLoaderPlugin to collect and chain middleware plugins - Update WidgetView to support middleware chaining - Add unit tests for middleware chaining behavior --- .../src/WidgetLoaderPlugin.test.tsx | 167 +++++++++++++ .../src/WidgetLoaderPlugin.tsx | 230 ++++++++++++++++-- packages/plugin/src/PluginTypes.test.ts | 45 ++++ packages/plugin/src/PluginTypes.ts | 71 ++++++ packages/plugin/src/WidgetView.tsx | 86 ++++++- 5 files changed, 570 insertions(+), 29 deletions(-) diff --git a/packages/dashboard-core-plugins/src/WidgetLoaderPlugin.test.tsx b/packages/dashboard-core-plugins/src/WidgetLoaderPlugin.test.tsx index b383b5aa90..63b5c4619f 100644 --- a/packages/dashboard-core-plugins/src/WidgetLoaderPlugin.test.tsx +++ b/packages/dashboard-core-plugins/src/WidgetLoaderPlugin.test.tsx @@ -4,7 +4,9 @@ import { PluginType, PluginsContext, type WidgetPlugin, + type WidgetMiddlewarePlugin, type WidgetComponentProps, + type WidgetMiddlewareComponentProps, } from '@deephaven/plugin'; import { Provider } from 'react-redux'; import { Dashboard, PanelEvent } from '@deephaven/dashboard'; @@ -267,3 +269,168 @@ describe('component wrapper', () => { ); }); }); + +describe('middleware plugin chaining', () => { + function TestMiddlewareWrapper({ + Component, + ...props + }: WidgetMiddlewareComponentProps) { + return ( +
+ MiddlewareWrapper + {/* eslint-disable-next-line react/jsx-props-no-spreading */} + +
+ ); + } + + function TestMiddlewareWrapperTwo({ + Component, + ...props + }: WidgetMiddlewareComponentProps) { + return ( +
+ MiddlewareWrapperTwo + {/* eslint-disable-next-line react/jsx-props-no-spreading */} + +
+ ); + } + + const testMiddlewarePlugin: WidgetMiddlewarePlugin = { + name: 'test-middleware', + type: PluginType.WIDGET_PLUGIN, + component: TestMiddlewareWrapper, + supportedTypes: 'test-widget', + isMiddleware: true, + }; + + const testMiddlewarePluginTwo: WidgetMiddlewarePlugin = { + name: 'test-middleware-two', + type: PluginType.WIDGET_PLUGIN, + component: TestMiddlewareWrapperTwo, + supportedTypes: 'test-widget', + isMiddleware: true, + }; + + it('chains middleware plugin around base widget', async () => { + const layoutManager = createAndMountDashboard([ + ['test-widget-plugin', testWidgetPlugin], + ['test-middleware-plugin', testMiddlewarePlugin], + ]); + + act( + () => + layoutManager?.eventHub.emit(PanelEvent.OPEN, { + widget: { type: 'test-widget' }, + }) + ); + + // Both the middleware wrapper and the base widget should be rendered + expect(screen.queryAllByText('MiddlewareWrapper').length).toBe(1); + expect(screen.queryAllByText('TestWidget').length).toBe(1); + + // The widget should be inside the middleware wrapper + const wrapper = screen.getByTestId('middleware-wrapper'); + expect(wrapper).toContainElement(screen.getByText('TestWidget')); + }); + + it('chains multiple middleware plugins in registration order', async () => { + const layoutManager = createAndMountDashboard([ + ['test-widget-plugin', testWidgetPlugin], + ['test-middleware-plugin', testMiddlewarePlugin], + ['test-middleware-plugin-two', testMiddlewarePluginTwo], + ]); + + act( + () => + layoutManager?.eventHub.emit(PanelEvent.OPEN, { + widget: { type: 'test-widget' }, + }) + ); + + // All components should be rendered + expect(screen.queryAllByText('MiddlewareWrapper').length).toBe(1); + expect(screen.queryAllByText('MiddlewareWrapperTwo').length).toBe(1); + expect(screen.queryAllByText('TestWidget').length).toBe(1); + + // Middleware should be chained in registration order (first middleware is outermost) + const wrapperOne = screen.getByTestId('middleware-wrapper'); + const wrapperTwo = screen.getByTestId('middleware-wrapper-two'); + expect(wrapperOne).toContainElement(wrapperTwo); + expect(wrapperTwo).toContainElement(screen.getByText('TestWidget')); + }); + + it('middleware registered before base plugin is still applied', async () => { + const layoutManager = createAndMountDashboard([ + ['test-middleware-plugin', testMiddlewarePlugin], + ['test-widget-plugin', testWidgetPlugin], + ]); + + act( + () => + layoutManager?.eventHub.emit(PanelEvent.OPEN, { + widget: { type: 'test-widget' }, + }) + ); + + // Middleware should wrap the base widget + expect(screen.queryAllByText('MiddlewareWrapper').length).toBe(1); + expect(screen.queryAllByText('TestWidget').length).toBe(1); + const wrapper = screen.getByTestId('middleware-wrapper'); + expect(wrapper).toContainElement(screen.getByText('TestWidget')); + }); + + it('middleware without base plugin is not rendered', async () => { + const layoutManager = createAndMountDashboard([ + [ + 'test-middleware-only', + { + ...testMiddlewarePlugin, + supportedTypes: 'middleware-only-type', + }, + ], + ]); + + act( + () => + layoutManager?.eventHub.emit(PanelEvent.OPEN, { + widget: { type: 'middleware-only-type' }, + }) + ); + + // Nothing should be rendered since there's no base plugin + expect(screen.queryAllByText('MiddlewareWrapper').length).toBe(0); + }); + + it('base plugin replacement keeps middleware chain', async () => { + const layoutManager = createAndMountDashboard([ + ['test-widget-plugin', testWidgetPlugin], + ['test-middleware-plugin', testMiddlewarePlugin], + [ + 'test-widget-plugin-two', + { + name: 'test-widget-plugin-two', + type: PluginType.WIDGET_PLUGIN, + component: TestWidgetTwo, + supportedTypes: 'test-widget', + }, + ], + ]); + + act( + () => + layoutManager?.eventHub.emit(PanelEvent.OPEN, { + widget: { type: 'test-widget' }, + }) + ); + + // The second base plugin should replace the first, but middleware should still apply + expect(screen.queryAllByText('MiddlewareWrapper').length).toBe(1); + expect(screen.queryAllByText('TestWidget').length).toBe(0); + expect(screen.queryAllByText('TestWidgetTwo').length).toBe(1); + + const wrapper = screen.getByTestId('middleware-wrapper'); + expect(wrapper).toContainElement(screen.getByText('TestWidgetTwo')); + }); +}); diff --git a/packages/dashboard-core-plugins/src/WidgetLoaderPlugin.tsx b/packages/dashboard-core-plugins/src/WidgetLoaderPlugin.tsx index 7623d1fd09..54cf308860 100644 --- a/packages/dashboard-core-plugins/src/WidgetLoaderPlugin.tsx +++ b/packages/dashboard-core-plugins/src/WidgetLoaderPlugin.tsx @@ -14,14 +14,110 @@ import { import Log from '@deephaven/log'; import { isWidgetPlugin, + isWidgetMiddlewarePlugin, usePlugins, type WidgetPlugin, + type WidgetMiddlewarePlugin, + type WidgetComponentProps, + type WidgetPanelProps, + type WidgetMiddlewarePanelProps, } from '@deephaven/plugin'; import { WidgetPanel } from './panels'; import { type WidgetPanelDescriptor } from './panels/WidgetPanelTypes'; const log = Log.module('WidgetLoaderPlugin'); +/** + * Information about a widget type including its base plugin and any middleware. + */ +interface WidgetTypeInfo { + /** The base plugin that handles this widget type */ + basePlugin: WidgetPlugin; + /** Middleware plugins to apply, in order from outermost to innermost */ + middleware: WidgetMiddlewarePlugin[]; +} + +/** + * Creates a component that chains middleware around a base component. + * Each middleware wraps the next, with the base component at the innermost layer. + */ +function createChainedComponent( + baseComponent: React.ComponentType>, + middleware: WidgetMiddlewarePlugin[] +): React.ComponentType> { + if (middleware.length === 0) { + return baseComponent; + } + + // Build the chain from inside out (base component is innermost) + // Middleware is ordered outermost to innermost, so we reverse to build from inside out + return [...middleware] + .reverse() + .reduce>>( + (WrappedComponent, middlewarePlugin) => { + const MiddlewareComponent = middlewarePlugin.component; + + function ChainedComponent(props: WidgetComponentProps) { + return ( + // eslint-disable-next-line react/jsx-props-no-spreading + + ); + } + ChainedComponent.displayName = `${middlewarePlugin.name}(${ + (WrappedComponent as React.ComponentType).displayName ?? + (WrappedComponent as React.ComponentType).name ?? + 'Component' + })`; + return ChainedComponent; + }, + baseComponent + ); +} + +/** + * Creates a panel component that chains middleware around a base panel component. + * Each middleware panel wraps the next, with the base panel at the innermost layer. + */ +function createChainedPanelComponent( + basePanelComponent: React.ComponentType>, + middleware: WidgetMiddlewarePlugin[] +): React.ComponentType> { + // Filter to middleware that has a panelComponent and extract just the panel components + type MiddlewareWithPanel = WidgetMiddlewarePlugin & { + panelComponent: React.ComponentType>; + }; + const panelMiddleware = middleware.filter( + (m): m is MiddlewareWithPanel => m.panelComponent != null + ); + + if (panelMiddleware.length === 0) { + return basePanelComponent; + } + + // Build the chain from inside out (base panel is innermost) + return [...panelMiddleware] + .reverse() + .reduce>>( + (WrappedPanel, middlewarePlugin) => { + const { panelComponent: MiddlewarePanelComponent } = middlewarePlugin; + + function ChainedPanel(props: WidgetPanelProps) { + return ( + // eslint-disable-next-line react/jsx-props-no-spreading + + ); + } + ChainedPanel.displayName = `${middlewarePlugin.name}Panel(${ + (WrappedPanel as React.ComponentType).displayName ?? + (WrappedPanel as React.ComponentType).name ?? + 'Panel' + })`; + return ChainedPanel; + }, + basePanelComponent + ); +} + export function WrapWidgetPlugin( plugin: WidgetPlugin ): React.ForwardRefExoticComponent> { @@ -76,6 +172,10 @@ export function WrapWidgetPlugin( * Does not open panels for widgets that are not supported by any plugins. * Does not open panels for widgets that are a component of a larger widget or UI element. * + * Supports plugin chaining via middleware plugins. When multiple plugins + * support the same widget type, middleware plugins are chained around + * the base plugin in registration order. + * * @param props Dashboard plugin props * @returns React element */ @@ -83,28 +183,83 @@ export function WidgetLoaderPlugin( props: Partial ): JSX.Element | null { const plugins = usePlugins(); + + /** + * Build a map of widget types to their plugin chain info. + * For each type, we have a base plugin and a list of middleware to apply. + */ const supportedTypes = useMemo(() => { - const typeMap = new Map(); + const typeMap = new Map(); + plugins.forEach(plugin => { if (!isWidgetPlugin(plugin)) { return; } + const isMiddleware = isWidgetMiddlewarePlugin(plugin); + [plugin.supportedTypes].flat().forEach(supportedType => { - if (supportedType != null && supportedType !== '') { - if (typeMap.has(supportedType)) { - log.warn( - `Multiple WidgetPlugins handling type ${supportedType}. Replacing ${typeMap.get( - supportedType - )?.name} with ${plugin.name} to handle ${supportedType}` + if (supportedType == null || supportedType === '') { + return; + } + + const existing = typeMap.get(supportedType); + + if (isMiddleware) { + // Add middleware to existing chain or create pending chain + if (existing != null) { + existing.middleware.push(plugin); + log.debug( + `Adding middleware ${plugin.name} to chain for type ${supportedType}` + ); + } else { + // No base plugin yet, create entry with just middleware + // The base plugin will be set when a non-middleware plugin is registered + typeMap.set(supportedType, { + // Use a placeholder that will be replaced + basePlugin: plugin as unknown as WidgetPlugin, + middleware: [plugin], + }); + log.debug( + `Creating pending middleware chain for type ${supportedType} with ${plugin.name}` ); } - typeMap.set(supportedType, plugin); + } else { + // Non-middleware plugin: becomes the base plugin + if (existing != null) { + if (!isWidgetMiddlewarePlugin(existing.basePlugin)) { + // Already have a base plugin, warn about replacement + log.warn( + `Multiple WidgetPlugins handling type ${supportedType}. ` + + `Replacing ${existing.basePlugin.name} with ${plugin.name} as base plugin` + ); + } + // Keep existing middleware, update the base plugin + existing.basePlugin = plugin; + } else { + typeMap.set(supportedType, { + basePlugin: plugin, + middleware: [], + }); + } + log.debug(`Set base plugin ${plugin.name} for type ${supportedType}`); } }); }); - return typeMap; + // Filter out entries that only have middleware (no base plugin) + const validEntries = new Map(); + typeMap.forEach((info, type) => { + if (!isWidgetMiddlewarePlugin(info.basePlugin)) { + validEntries.set(type, info); + } else { + log.warn( + `No base plugin found for type ${type}, middleware will not be applied` + ); + } + }); + + return validEntries; }, [plugins]); assertIsDashboardPluginProps(props); @@ -118,8 +273,8 @@ export function WidgetLoaderPlugin( widget, }: PanelOpenEventDetail) => { const { type } = widget; - const plugin = type != null ? supportedTypes.get(type) : null; - if (plugin == null) { + const typeInfo = type != null ? supportedTypes.get(type) : null; + if (typeInfo == null) { return; } const name = widget.name ?? type; @@ -134,7 +289,7 @@ export function WidgetLoaderPlugin( const config: ReactComponentConfig = { type: 'react-component', - component: plugin.name, + component: typeInfo.basePlugin.name, props: panelProps, title: name, id: panelId, @@ -147,14 +302,55 @@ export function WidgetLoaderPlugin( ); useEffect(() => { - const deregisterFns = [...new Set(supportedTypes.values())].map(plugin => { - const { panelComponent } = plugin; - if (panelComponent == null) { - return registerComponent(plugin.name, WrapWidgetPlugin(plugin)); + // Get unique base plugins (a plugin may handle multiple types) + const uniquePluginInfos = new Map(); + supportedTypes.forEach((info, type) => { + // Use the base plugin name as the key to get unique plugins + if (!uniquePluginInfos.has(info.basePlugin.name)) { + uniquePluginInfos.set(info.basePlugin.name, info); + } else { + // Merge middleware from multiple type registrations for the same base plugin + const existingInfo = uniquePluginInfos.get(info.basePlugin.name); + if (existingInfo != null) { + info.middleware.forEach(m => { + if (!existingInfo.middleware.includes(m)) { + existingInfo.middleware.push(m); + } + }); + } } - return registerComponent(plugin.name, panelComponent); }); + const deregisterFns = [...uniquePluginInfos.values()].map( + ({ basePlugin, middleware }) => { + const { panelComponent } = basePlugin; + + if (panelComponent == null) { + // No panel component - chain the widget components and wrap in default panel + const chainedComponent = createChainedComponent( + basePlugin.component, + middleware + ); + const wrappedPlugin: WidgetPlugin = { + ...basePlugin, + component: chainedComponent, + }; + return registerComponent( + basePlugin.name, + WrapWidgetPlugin(wrappedPlugin) + ); + } + + // Has panel component - chain both component and panel + const chainedPanelComponent = createChainedPanelComponent( + panelComponent, + middleware + ); + + return registerComponent(basePlugin.name, chainedPanelComponent); + } + ); + return () => { deregisterFns.forEach(deregister => deregister()); }; diff --git a/packages/plugin/src/PluginTypes.test.ts b/packages/plugin/src/PluginTypes.test.ts index 9ded422672..d76a11afe8 100644 --- a/packages/plugin/src/PluginTypes.test.ts +++ b/packages/plugin/src/PluginTypes.test.ts @@ -7,7 +7,10 @@ import { isTablePlugin, isThemePlugin, isWidgetPlugin, + isWidgetMiddlewarePlugin, type Plugin, + type WidgetPlugin, + type WidgetMiddlewarePlugin, isPlugin, } from './PluginTypes'; @@ -51,3 +54,45 @@ describe('isPlugin', () => { expect(isPlugin({})).toBe(false); }); }); + +describe('isWidgetMiddlewarePlugin', () => { + const baseWidgetPlugin: WidgetPlugin = { + name: 'test-widget', + type: PluginType.WIDGET_PLUGIN, + component: () => null, + supportedTypes: 'test-type', + }; + + const middlewarePlugin: WidgetMiddlewarePlugin = { + name: 'test-middleware', + type: PluginType.WIDGET_PLUGIN, + component: () => null, + supportedTypes: 'test-type', + isMiddleware: true, + }; + + it('returns true for middleware plugins', () => { + expect(isWidgetMiddlewarePlugin(middlewarePlugin)).toBe(true); + }); + + it('returns false for regular widget plugins', () => { + expect(isWidgetMiddlewarePlugin(baseWidgetPlugin)).toBe(false); + }); + + it('returns false for widget plugins with isMiddleware set to false', () => { + const notMiddleware = { + ...baseWidgetPlugin, + isMiddleware: false, + }; + expect(isWidgetMiddlewarePlugin(notMiddleware)).toBe(false); + }); + + it('returns false for non-widget plugins', () => { + expect( + isWidgetMiddlewarePlugin({ + name: 'test', + type: PluginType.DASHBOARD_PLUGIN, + }) + ).toBe(false); + }); +}); diff --git a/packages/plugin/src/PluginTypes.ts b/packages/plugin/src/PluginTypes.ts index 82c9b54010..8c046e1c70 100644 --- a/packages/plugin/src/PluginTypes.ts +++ b/packages/plugin/src/PluginTypes.ts @@ -144,6 +144,77 @@ export interface WidgetPanelProps extends WidgetComponentProps { glEventHub: EventEmitter; } +/** + * Props passed to middleware components that wrap a base widget. + * Extends WidgetComponentProps with the wrapped component. + */ +export interface WidgetMiddlewareComponentProps + extends WidgetComponentProps { + /** + * The next component in the middleware chain. + * Middleware should render this component to continue the chain. + */ + Component: React.ComponentType>; +} + +/** + * Props passed to middleware panel components that wrap a base panel. + * Extends WidgetPanelProps with the wrapped panel component. + */ +export interface WidgetMiddlewarePanelProps + extends WidgetPanelProps { + /** + * The next panel component in the middleware chain. + * Middleware should render this component to continue the chain. + */ + Component: React.ComponentType>; +} + +/** + * A middleware plugin that can wrap and enhance another widget plugin. + * Middleware plugins are chained together in registration order, + * with each middleware wrapping the next in the chain. + * + * The middleware pattern allows plugins to: + * - Add additional UI elements around a widget + * - Intercept and modify props before they reach the wrapped component + * - Provide additional context or state to the wrapped component + * - Add menu items or other extensions to the widget + */ +export interface WidgetMiddlewarePlugin + extends Omit, 'component' | 'panelComponent'> { + /** + * Marks this plugin as middleware that should be chained + * with other plugins of the same supportedTypes. + */ + isMiddleware: true; + + /** + * The middleware component that wraps the base widget component. + * Receives the wrapped component as a prop and should render it. + */ + component: React.ComponentType>; + + /** + * The middleware panel component that wraps the base panel component. + * If omitted, only the component middleware will be applied. + */ + panelComponent?: React.ComponentType>; +} + +/** + * Type guard to check if a plugin is a middleware plugin. + */ +export function isWidgetMiddlewarePlugin( + plugin: PluginModule +): plugin is WidgetMiddlewarePlugin { + return ( + isWidgetPlugin(plugin) && + 'isMiddleware' in plugin && + plugin.isMiddleware === true + ); +} + export interface WidgetPlugin extends Plugin { type: typeof PluginType.WIDGET_PLUGIN; /** diff --git a/packages/plugin/src/WidgetView.tsx b/packages/plugin/src/WidgetView.tsx index 6e92c17ff7..c7376c4241 100644 --- a/packages/plugin/src/WidgetView.tsx +++ b/packages/plugin/src/WidgetView.tsx @@ -1,6 +1,12 @@ import React, { useMemo } from 'react'; import usePlugins from './usePlugins'; -import { isWidgetPlugin } from './PluginTypes'; +import { + isWidgetPlugin, + isWidgetMiddlewarePlugin, + type WidgetPlugin, + type WidgetMiddlewarePlugin, + type WidgetComponentProps, +} from './PluginTypes'; export type WidgetViewProps = { /** Fetch function to return the widget */ @@ -10,19 +16,75 @@ export type WidgetViewProps = { type: string; }; +/** + * Creates a component that chains middleware around a base component. + * Each middleware wraps the next, with the base component at the innermost layer. + */ +function createChainedComponent( + baseComponent: React.ComponentType>, + middleware: WidgetMiddlewarePlugin[] +): React.ComponentType> { + if (middleware.length === 0) { + return baseComponent; + } + + // Build the chain from inside out (base component is innermost) + // Middleware is ordered outermost to innermost, so we reverse to build from inside out + return [...middleware] + .reverse() + .reduce>>( + (WrappedComponent, middlewarePlugin) => { + const MiddlewareComponent = middlewarePlugin.component; + + function ChainedComponent(props: WidgetComponentProps) { + return ( + // eslint-disable-next-line react/jsx-props-no-spreading + + ); + } + ChainedComponent.displayName = `${middlewarePlugin.name}(${ + (WrappedComponent as React.ComponentType).displayName ?? + (WrappedComponent as React.ComponentType).name ?? + 'Component' + })`; + return ChainedComponent; + }, + baseComponent + ); +} + export function WidgetView({ fetch, type }: WidgetViewProps): JSX.Element { const plugins = usePlugins(); - const plugin = useMemo( - () => - [...plugins.values()] - .filter(isWidgetPlugin) - .find(p => [p.supportedTypes].flat().includes(type)), - [plugins, type] - ); - - if (plugin != null) { - const Component = plugin.component; - return ; + + const { basePlugin, middleware } = useMemo(() => { + let foundBasePlugin: WidgetPlugin | undefined; + const foundMiddleware: WidgetMiddlewarePlugin[] = []; + + [...plugins.values()].filter(isWidgetPlugin).forEach(p => { + const supportsType = [p.supportedTypes].flat().includes(type); + if (!supportsType) { + return; + } + + if (isWidgetMiddlewarePlugin(p)) { + foundMiddleware.push(p); + } else if (foundBasePlugin == null) { + foundBasePlugin = p; + } + }); + + return { basePlugin: foundBasePlugin, middleware: foundMiddleware }; + }, [plugins, type]); + + const ChainedComponent = useMemo(() => { + if (basePlugin == null) { + return null; + } + return createChainedComponent(basePlugin.component, middleware); + }, [basePlugin, middleware]); + + if (ChainedComponent != null) { + return ; } throw new Error(`Unknown widget type '${type}'`); From 4f91e0a1d1253b6b61dedb490f403ebe92961696 Mon Sep 17 00:00:00 2001 From: Vlad Babich Date: Mon, 13 Apr 2026 07:34:26 -0600 Subject: [PATCH 02/25] Cleanup --- .../src/WidgetLoaderPlugin.tsx | 39 +----------------- packages/plugin/src/PluginUtils.tsx | 41 ++++++++++++++++++- packages/plugin/src/WidgetView.tsx | 41 +------------------ 3 files changed, 43 insertions(+), 78 deletions(-) diff --git a/packages/dashboard-core-plugins/src/WidgetLoaderPlugin.tsx b/packages/dashboard-core-plugins/src/WidgetLoaderPlugin.tsx index 54cf308860..3ee7ed9d1f 100644 --- a/packages/dashboard-core-plugins/src/WidgetLoaderPlugin.tsx +++ b/packages/dashboard-core-plugins/src/WidgetLoaderPlugin.tsx @@ -15,10 +15,10 @@ import Log from '@deephaven/log'; import { isWidgetPlugin, isWidgetMiddlewarePlugin, + createChainedComponent, usePlugins, type WidgetPlugin, type WidgetMiddlewarePlugin, - type WidgetComponentProps, type WidgetPanelProps, type WidgetMiddlewarePanelProps, } from '@deephaven/plugin'; @@ -37,43 +37,6 @@ interface WidgetTypeInfo { middleware: WidgetMiddlewarePlugin[]; } -/** - * Creates a component that chains middleware around a base component. - * Each middleware wraps the next, with the base component at the innermost layer. - */ -function createChainedComponent( - baseComponent: React.ComponentType>, - middleware: WidgetMiddlewarePlugin[] -): React.ComponentType> { - if (middleware.length === 0) { - return baseComponent; - } - - // Build the chain from inside out (base component is innermost) - // Middleware is ordered outermost to innermost, so we reverse to build from inside out - return [...middleware] - .reverse() - .reduce>>( - (WrappedComponent, middlewarePlugin) => { - const MiddlewareComponent = middlewarePlugin.component; - - function ChainedComponent(props: WidgetComponentProps) { - return ( - // eslint-disable-next-line react/jsx-props-no-spreading - - ); - } - ChainedComponent.displayName = `${middlewarePlugin.name}(${ - (WrappedComponent as React.ComponentType).displayName ?? - (WrappedComponent as React.ComponentType).name ?? - 'Component' - })`; - return ChainedComponent; - }, - baseComponent - ); -} - /** * Creates a panel component that chains middleware around a base panel component. * Each middleware panel wraps the next, with the base panel at the innermost layer. diff --git a/packages/plugin/src/PluginUtils.tsx b/packages/plugin/src/PluginUtils.tsx index e4ea1fa617..ed8ef9a8e8 100644 --- a/packages/plugin/src/PluginUtils.tsx +++ b/packages/plugin/src/PluginUtils.tsx @@ -1,4 +1,4 @@ -import { isValidElement } from 'react'; +import React, { isValidElement } from 'react'; import { FontAwesomeIcon } from '@fortawesome/react-fontawesome'; import { getThemeKey, type ThemeData } from '@deephaven/components'; import { vsPreview } from '@deephaven/icons'; @@ -12,6 +12,8 @@ import { isElementPlugin, type ElementPlugin, type ElementMap, + type WidgetMiddlewarePlugin, + type WidgetComponentProps, } from './PluginTypes'; const log = Log.module('@deephaven/plugin.PluginUtils'); @@ -97,3 +99,40 @@ export function getPluginsElementMap(pluginMap: PluginModuleMap): ElementMap { elementPluginEntries.flatMap(([, plugin]) => Object.entries(plugin.mapping)) ); } + +/** + * Creates a component that chains middleware around a base component. + * Each middleware wraps the next, with the base component at the innermost layer. + */ +export function createChainedComponent( + baseComponent: React.ComponentType>, + middleware: WidgetMiddlewarePlugin[] +): React.ComponentType> { + if (middleware.length === 0) { + return baseComponent; + } + + // Build the chain from inside out (base component is innermost) + // Middleware is ordered outermost to innermost, so we reverse to build from inside out + return [...middleware] + .reverse() + .reduce>>( + (WrappedComponent, middlewarePlugin) => { + const MiddlewareComponent = middlewarePlugin.component; + + function ChainedComponent(props: WidgetComponentProps) { + return ( + // eslint-disable-next-line react/jsx-props-no-spreading + + ); + } + ChainedComponent.displayName = `${middlewarePlugin.name}(${ + (WrappedComponent as React.ComponentType).displayName ?? + (WrappedComponent as React.ComponentType).name ?? + 'Component' + })`; + return ChainedComponent; + }, + baseComponent + ); +} diff --git a/packages/plugin/src/WidgetView.tsx b/packages/plugin/src/WidgetView.tsx index c7376c4241..4e2e4bc4cc 100644 --- a/packages/plugin/src/WidgetView.tsx +++ b/packages/plugin/src/WidgetView.tsx @@ -1,12 +1,12 @@ -import React, { useMemo } from 'react'; +import { useMemo } from 'react'; import usePlugins from './usePlugins'; import { isWidgetPlugin, isWidgetMiddlewarePlugin, type WidgetPlugin, type WidgetMiddlewarePlugin, - type WidgetComponentProps, } from './PluginTypes'; +import { createChainedComponent } from './PluginUtils'; export type WidgetViewProps = { /** Fetch function to return the widget */ @@ -16,43 +16,6 @@ export type WidgetViewProps = { type: string; }; -/** - * Creates a component that chains middleware around a base component. - * Each middleware wraps the next, with the base component at the innermost layer. - */ -function createChainedComponent( - baseComponent: React.ComponentType>, - middleware: WidgetMiddlewarePlugin[] -): React.ComponentType> { - if (middleware.length === 0) { - return baseComponent; - } - - // Build the chain from inside out (base component is innermost) - // Middleware is ordered outermost to innermost, so we reverse to build from inside out - return [...middleware] - .reverse() - .reduce>>( - (WrappedComponent, middlewarePlugin) => { - const MiddlewareComponent = middlewarePlugin.component; - - function ChainedComponent(props: WidgetComponentProps) { - return ( - // eslint-disable-next-line react/jsx-props-no-spreading - - ); - } - ChainedComponent.displayName = `${middlewarePlugin.name}(${ - (WrappedComponent as React.ComponentType).displayName ?? - (WrappedComponent as React.ComponentType).name ?? - 'Component' - })`; - return ChainedComponent; - }, - baseComponent - ); -} - export function WidgetView({ fetch, type }: WidgetViewProps): JSX.Element { const plugins = usePlugins(); From a1720408e0b1bdb63e4fddb4725e21792f447cfd Mon Sep 17 00:00:00 2001 From: Vlad Babich Date: Mon, 13 Apr 2026 07:41:21 -0600 Subject: [PATCH 03/25] Cleanup --- .../src/WidgetLoaderPlugin.tsx | 27 ++++++++++++------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/packages/dashboard-core-plugins/src/WidgetLoaderPlugin.tsx b/packages/dashboard-core-plugins/src/WidgetLoaderPlugin.tsx index 3ee7ed9d1f..fd4f960c30 100644 --- a/packages/dashboard-core-plugins/src/WidgetLoaderPlugin.tsx +++ b/packages/dashboard-core-plugins/src/WidgetLoaderPlugin.tsx @@ -31,8 +31,8 @@ const log = Log.module('WidgetLoaderPlugin'); * Information about a widget type including its base plugin and any middleware. */ interface WidgetTypeInfo { - /** The base plugin that handles this widget type */ - basePlugin: WidgetPlugin; + /** The base plugin that handles this widget type, or null if only middleware registered so far */ + basePlugin: WidgetPlugin | null; /** Middleware plugins to apply, in order from outermost to innermost */ middleware: WidgetMiddlewarePlugin[]; } @@ -179,8 +179,7 @@ export function WidgetLoaderPlugin( // No base plugin yet, create entry with just middleware // The base plugin will be set when a non-middleware plugin is registered typeMap.set(supportedType, { - // Use a placeholder that will be replaced - basePlugin: plugin as unknown as WidgetPlugin, + basePlugin: null, middleware: [plugin], }); log.debug( @@ -190,7 +189,7 @@ export function WidgetLoaderPlugin( } else { // Non-middleware plugin: becomes the base plugin if (existing != null) { - if (!isWidgetMiddlewarePlugin(existing.basePlugin)) { + if (existing.basePlugin != null) { // Already have a base plugin, warn about replacement log.warn( `Multiple WidgetPlugins handling type ${supportedType}. ` + @@ -211,10 +210,16 @@ export function WidgetLoaderPlugin( }); // Filter out entries that only have middleware (no base plugin) - const validEntries = new Map(); + const validEntries = new Map< + string, + WidgetTypeInfo & { basePlugin: WidgetPlugin } + >(); typeMap.forEach((info, type) => { - if (!isWidgetMiddlewarePlugin(info.basePlugin)) { - validEntries.set(type, info); + if (info.basePlugin != null) { + validEntries.set( + type, + info as WidgetTypeInfo & { basePlugin: WidgetPlugin } + ); } else { log.warn( `No base plugin found for type ${type}, middleware will not be applied` @@ -266,8 +271,10 @@ export function WidgetLoaderPlugin( useEffect(() => { // Get unique base plugins (a plugin may handle multiple types) - const uniquePluginInfos = new Map(); - supportedTypes.forEach((info, type) => { + // supportedTypes is already filtered to entries with non-null basePlugin + type ValidWidgetTypeInfo = WidgetTypeInfo & { basePlugin: WidgetPlugin }; + const uniquePluginInfos = new Map(); + supportedTypes.forEach((info, _type) => { // Use the base plugin name as the key to get unique plugins if (!uniquePluginInfos.has(info.basePlugin.name)) { uniquePluginInfos.set(info.basePlugin.name, info); From 6ed5e6fa64fc92f48365ebf4c886864bd60d7c42 Mon Sep 17 00:00:00 2001 From: Vlad Babich Date: Mon, 13 Apr 2026 07:44:52 -0600 Subject: [PATCH 04/25] Cleanup --- .../src/WidgetLoaderPlugin.tsx | 47 +------------------ packages/plugin/src/PluginUtils.tsx | 46 ++++++++++++++++++ 2 files changed, 47 insertions(+), 46 deletions(-) diff --git a/packages/dashboard-core-plugins/src/WidgetLoaderPlugin.tsx b/packages/dashboard-core-plugins/src/WidgetLoaderPlugin.tsx index fd4f960c30..7eb37f9d82 100644 --- a/packages/dashboard-core-plugins/src/WidgetLoaderPlugin.tsx +++ b/packages/dashboard-core-plugins/src/WidgetLoaderPlugin.tsx @@ -16,11 +16,10 @@ import { isWidgetPlugin, isWidgetMiddlewarePlugin, createChainedComponent, + createChainedPanelComponent, usePlugins, type WidgetPlugin, type WidgetMiddlewarePlugin, - type WidgetPanelProps, - type WidgetMiddlewarePanelProps, } from '@deephaven/plugin'; import { WidgetPanel } from './panels'; import { type WidgetPanelDescriptor } from './panels/WidgetPanelTypes'; @@ -37,50 +36,6 @@ interface WidgetTypeInfo { middleware: WidgetMiddlewarePlugin[]; } -/** - * Creates a panel component that chains middleware around a base panel component. - * Each middleware panel wraps the next, with the base panel at the innermost layer. - */ -function createChainedPanelComponent( - basePanelComponent: React.ComponentType>, - middleware: WidgetMiddlewarePlugin[] -): React.ComponentType> { - // Filter to middleware that has a panelComponent and extract just the panel components - type MiddlewareWithPanel = WidgetMiddlewarePlugin & { - panelComponent: React.ComponentType>; - }; - const panelMiddleware = middleware.filter( - (m): m is MiddlewareWithPanel => m.panelComponent != null - ); - - if (panelMiddleware.length === 0) { - return basePanelComponent; - } - - // Build the chain from inside out (base panel is innermost) - return [...panelMiddleware] - .reverse() - .reduce>>( - (WrappedPanel, middlewarePlugin) => { - const { panelComponent: MiddlewarePanelComponent } = middlewarePlugin; - - function ChainedPanel(props: WidgetPanelProps) { - return ( - // eslint-disable-next-line react/jsx-props-no-spreading - - ); - } - ChainedPanel.displayName = `${middlewarePlugin.name}Panel(${ - (WrappedPanel as React.ComponentType).displayName ?? - (WrappedPanel as React.ComponentType).name ?? - 'Panel' - })`; - return ChainedPanel; - }, - basePanelComponent - ); -} - export function WrapWidgetPlugin( plugin: WidgetPlugin ): React.ForwardRefExoticComponent> { diff --git a/packages/plugin/src/PluginUtils.tsx b/packages/plugin/src/PluginUtils.tsx index ed8ef9a8e8..d9ff59605d 100644 --- a/packages/plugin/src/PluginUtils.tsx +++ b/packages/plugin/src/PluginUtils.tsx @@ -14,6 +14,8 @@ import { type ElementMap, type WidgetMiddlewarePlugin, type WidgetComponentProps, + type WidgetPanelProps, + type WidgetMiddlewarePanelProps, } from './PluginTypes'; const log = Log.module('@deephaven/plugin.PluginUtils'); @@ -136,3 +138,47 @@ export function createChainedComponent( baseComponent ); } + +/** + * Creates a panel component that chains middleware around a base panel component. + * Each middleware panel wraps the next, with the base panel at the innermost layer. + */ +export function createChainedPanelComponent( + basePanelComponent: React.ComponentType>, + middleware: WidgetMiddlewarePlugin[] +): React.ComponentType> { + // Filter to middleware that has a panelComponent and extract just the panel components + type MiddlewareWithPanel = WidgetMiddlewarePlugin & { + panelComponent: React.ComponentType>; + }; + const panelMiddleware = middleware.filter( + (m): m is MiddlewareWithPanel => m.panelComponent != null + ); + + if (panelMiddleware.length === 0) { + return basePanelComponent; + } + + // Build the chain from inside out (base panel is innermost) + return [...panelMiddleware] + .reverse() + .reduce>>( + (WrappedPanel, middlewarePlugin) => { + const { panelComponent: MiddlewarePanelComponent } = middlewarePlugin; + + function ChainedPanel(props: WidgetPanelProps) { + return ( + // eslint-disable-next-line react/jsx-props-no-spreading + + ); + } + ChainedPanel.displayName = `${middlewarePlugin.name}Panel(${ + (WrappedPanel as React.ComponentType).displayName ?? + (WrappedPanel as React.ComponentType).name ?? + 'Panel' + })`; + return ChainedPanel; + }, + basePanelComponent + ); +} From bf3716e88d4511179ebce3dc49fab62992e1b417 Mon Sep 17 00:00:00 2001 From: Vlad Babich Date: Mon, 13 Apr 2026 12:32:23 -0600 Subject: [PATCH 05/25] Add warning for duplicate base plugins --- packages/plugin/src/WidgetView.tsx | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/packages/plugin/src/WidgetView.tsx b/packages/plugin/src/WidgetView.tsx index 4e2e4bc4cc..e10278979f 100644 --- a/packages/plugin/src/WidgetView.tsx +++ b/packages/plugin/src/WidgetView.tsx @@ -1,4 +1,5 @@ import { useMemo } from 'react'; +import Log from '@deephaven/log'; import usePlugins from './usePlugins'; import { isWidgetPlugin, @@ -8,6 +9,8 @@ import { } from './PluginTypes'; import { createChainedComponent } from './PluginUtils'; +const log = Log.module('@deephaven/plugin.WidgetView'); + export type WidgetViewProps = { /** Fetch function to return the widget */ fetch: () => Promise; @@ -33,6 +36,8 @@ export function WidgetView({ fetch, type }: WidgetViewProps): JSX.Element { foundMiddleware.push(p); } else if (foundBasePlugin == null) { foundBasePlugin = p; + } else { + log.warn(`Multiple base plugins for type ${type}, ignoring ${p.name}`); } }); From 7b774ac78f4c99c33b56b8051cf81598ebcdc123 Mon Sep 17 00:00:00 2001 From: Vlad Babich Date: Tue, 14 Apr 2026 13:07:03 -0600 Subject: [PATCH 06/25] Fix review issues --- .../src/WidgetLoaderPlugin.tsx | 10 +- packages/plugin/MIDDLEWARE_ARCHITECTURE.md | 172 ++++++++++++++++++ packages/plugin/src/WidgetView.tsx | 9 +- 3 files changed, 186 insertions(+), 5 deletions(-) create mode 100644 packages/plugin/MIDDLEWARE_ARCHITECTURE.md diff --git a/packages/dashboard-core-plugins/src/WidgetLoaderPlugin.tsx b/packages/dashboard-core-plugins/src/WidgetLoaderPlugin.tsx index 7eb37f9d82..9a7b87a1bd 100644 --- a/packages/dashboard-core-plugins/src/WidgetLoaderPlugin.tsx +++ b/packages/dashboard-core-plugins/src/WidgetLoaderPlugin.tsx @@ -232,7 +232,11 @@ export function WidgetLoaderPlugin( supportedTypes.forEach((info, _type) => { // Use the base plugin name as the key to get unique plugins if (!uniquePluginInfos.has(info.basePlugin.name)) { - uniquePluginInfos.set(info.basePlugin.name, info); + // Clone to avoid mutating the useMemo result + uniquePluginInfos.set(info.basePlugin.name, { + basePlugin: info.basePlugin, + middleware: [...info.middleware], + }); } else { // Merge middleware from multiple type registrations for the same base plugin const existingInfo = uniquePluginInfos.get(info.basePlugin.name); @@ -266,7 +270,9 @@ export function WidgetLoaderPlugin( ); } - // Has panel component - chain both component and panel + // Has panel component - chain middleware around the panel. + // Middleware with panelComponent wraps at the panel level directly. + // Middleware with only component is auto-promoted to a panel wrapper. const chainedPanelComponent = createChainedPanelComponent( panelComponent, middleware diff --git a/packages/plugin/MIDDLEWARE_ARCHITECTURE.md b/packages/plugin/MIDDLEWARE_ARCHITECTURE.md new file mode 100644 index 0000000000..47aef1c4e2 --- /dev/null +++ b/packages/plugin/MIDDLEWARE_ARCHITECTURE.md @@ -0,0 +1,172 @@ +# Middleware Plugin Architecture + +## Overview + +Middleware plugins allow you to wrap and enhance existing widget plugins without modifying them. Multiple middleware plugins can target the same widget type and are chained in registration order, with the first registered middleware as the outermost wrapper. + +``` +┌─────────────────────────────┐ +│ Middleware A (outermost) │ +│ ┌────────────────────────┐ │ +│ │ Middleware B │ │ +│ │ ┌───────────────────┐ │ │ +│ │ │ Base Widget │ │ │ +│ │ └───────────────────┘ │ │ +│ └────────────────────────┘ │ +└─────────────────────────────┘ +``` + +## Key Concepts + +- **Base plugin**: A standard `WidgetPlugin` that renders a widget type. Exactly one base plugin handles each type (last registered wins if duplicates exist). +- **Middleware plugin**: A `WidgetMiddlewarePlugin` that wraps the next component in the chain. It receives a `Component` prop and must render it to continue the chain. +- **Chaining**: Middleware is applied in registration order. The first middleware registered becomes the outermost wrapper. Each middleware renders the next via its `Component` prop. + +## Types + +### `WidgetMiddlewarePlugin` + +```tsx +interface WidgetMiddlewarePlugin { + name: string; + type: PluginType.WIDGET_PLUGIN; + supportedTypes: string | string[]; + isMiddleware: true; + + // Required: wraps the widget component + component: React.ComponentType>; + + // Optional: wraps the panel component (only needed if the base plugin defines panelComponent) + panelComponent?: React.ComponentType>; +} +``` + +### `WidgetMiddlewareComponentProps` + +```tsx +interface WidgetMiddlewareComponentProps extends WidgetComponentProps { + // The next component in the chain — render this to continue + Component: React.ComponentType>; +} +``` + +## Rendering Paths + +The middleware is applied in two different contexts: + +| Context | File | When Used | +|---|---|---| +| Dashboard panels | `WidgetLoaderPlugin.tsx` | Widget opened as a panel via `PanelEvent.OPEN` | +| Inline/embedded | `WidgetView.tsx` | Widget rendered inline (e.g., inside a document) | + +Both paths collect middleware for the widget type and use `createChainedComponent` to build the wrapper chain. + +## Usage Examples + +### Basic Middleware — Add a Toolbar + +```tsx +import { PluginType, type WidgetMiddlewareComponentProps } from '@deephaven/plugin'; + +function MyToolbar({ Component, ...props }: WidgetMiddlewareComponentProps) { + return ( +
+
+ +
+ +
+ ); +} + +const myToolbarPlugin = { + name: 'my-toolbar-middleware', + type: PluginType.WIDGET_PLUGIN, + component: MyToolbar, + supportedTypes: 'deephaven.plot.express.DeephavenFigure', + isMiddleware: true, +} satisfies WidgetMiddlewarePlugin; +``` + +### Intercepting Props + +```tsx +function PropsInterceptor({ Component, ...props }: WidgetMiddlewareComponentProps) { + // Modify or augment props before passing them to the wrapped component + const enhancedFetch = useCallback(async () => { + const widget = await props.fetch(); + // Transform or cache the widget data + return widget; + }, [props.fetch]); + + return ; +} +``` + +### Adding Context + +```tsx +const MyFeatureContext = createContext(null); + +function FeatureProvider({ Component, ...props }: WidgetMiddlewareComponentProps) { + const [state, setState] = useState({ enabled: true }); + + return ( + + + + ); +} +``` + +### Conditional Wrapping + +```tsx +function ConditionalMiddleware({ Component, ...props }: WidgetMiddlewareComponentProps) { + const shouldWrap = useSomeCondition(); + + if (!shouldWrap) { + // Pass through without wrapping + return ; + } + + return ( +
+ +
+ ); +} +``` + +### Targeting Multiple Widget Types + +```tsx +const multiTypeMiddleware = { + name: 'multi-type-middleware', + type: PluginType.WIDGET_PLUGIN, + component: MyMiddlewareComponent, + supportedTypes: ['deephaven.plot.express.DeephavenFigure', 'deephaven.ui.Element'], + isMiddleware: true, +} satisfies WidgetMiddlewarePlugin; +``` + +## Registration + +Middleware plugins are registered the same way as regular plugins — they are included in the plugin map passed to `PluginsContext`. Registration order determines chaining order. + +```tsx +// In the plugin map, order matters: +const plugins = new Map([ + ['base-widget', baseWidgetPlugin], // Base plugin + ['middleware-a', middlewarePluginA], // Outermost wrapper + ['middleware-b', middlewarePluginB], // Inner wrapper (closer to base) +]); +``` + +## Rules + +1. **A base plugin is required.** Middleware registered for a type with no base plugin has no effect and produces a warning. +2. **Last base plugin wins.** If multiple non-middleware plugins register for the same type, the last one replaces earlier ones (with a warning). +3. **Middleware must render `Component`.** If a middleware doesn't render the `Component` prop, the rest of the chain (including the base widget) will not appear. +4. **Middleware must spread props.** Pass all received props to `Component` to ensure the base widget and other middleware receive them. +5. **`panelComponent` middleware is separate.** If the base plugin defines a `panelComponent`, middleware targeting the panel layer must also define `panelComponent`. diff --git a/packages/plugin/src/WidgetView.tsx b/packages/plugin/src/WidgetView.tsx index e10278979f..d15a36a1aa 100644 --- a/packages/plugin/src/WidgetView.tsx +++ b/packages/plugin/src/WidgetView.tsx @@ -34,10 +34,13 @@ export function WidgetView({ fetch, type }: WidgetViewProps): JSX.Element { if (isWidgetMiddlewarePlugin(p)) { foundMiddleware.push(p); - } else if (foundBasePlugin == null) { - foundBasePlugin = p; } else { - log.warn(`Multiple base plugins for type ${type}, ignoring ${p.name}`); + if (foundBasePlugin != null) { + log.warn( + `Multiple base plugins for type ${type}. Replacing ${foundBasePlugin.name} with ${p.name}` + ); + } + foundBasePlugin = p; } }); From 91565b03c4b411d1bfb3c8224e86d35d6470a39e Mon Sep 17 00:00:00 2001 From: Vlad Babich Date: Fri, 17 Apr 2026 12:17:43 -0600 Subject: [PATCH 07/25] Verbose debug logging --- .../src/WidgetLoaderPlugin.tsx | 15 ++++++++++++++ packages/plugin/src/PluginUtils.tsx | 20 +++++++++++++++++++ packages/plugin/src/WidgetView.tsx | 13 ++++++++++++ 3 files changed, 48 insertions(+) diff --git a/packages/dashboard-core-plugins/src/WidgetLoaderPlugin.tsx b/packages/dashboard-core-plugins/src/WidgetLoaderPlugin.tsx index 9a7b87a1bd..0640e35d65 100644 --- a/packages/dashboard-core-plugins/src/WidgetLoaderPlugin.tsx +++ b/packages/dashboard-core-plugins/src/WidgetLoaderPlugin.tsx @@ -250,12 +250,24 @@ export function WidgetLoaderPlugin( } }); + log.debug( + 'Registering widget components', + [...uniquePluginInfos.entries()].map(([name, info]) => ({ + plugin: name, + middleware: info.middleware.map(m => m.name), + hasPanel: info.basePlugin.panelComponent != null, + })) + ); + const deregisterFns = [...uniquePluginInfos.values()].map( ({ basePlugin, middleware }) => { const { panelComponent } = basePlugin; if (panelComponent == null) { // No panel component - chain the widget components and wrap in default panel + log.debug( + `Chaining widget components for ${basePlugin.name} (no panel component, using default wrapper)` + ); const chainedComponent = createChainedComponent( basePlugin.component, middleware @@ -273,6 +285,9 @@ export function WidgetLoaderPlugin( // Has panel component - chain middleware around the panel. // Middleware with panelComponent wraps at the panel level directly. // Middleware with only component is auto-promoted to a panel wrapper. + log.debug( + `Chaining panel components for ${basePlugin.name} (has custom panel component)` + ); const chainedPanelComponent = createChainedPanelComponent( panelComponent, middleware diff --git a/packages/plugin/src/PluginUtils.tsx b/packages/plugin/src/PluginUtils.tsx index d9ff59605d..ad24ef50ad 100644 --- a/packages/plugin/src/PluginUtils.tsx +++ b/packages/plugin/src/PluginUtils.tsx @@ -111,9 +111,19 @@ export function createChainedComponent( middleware: WidgetMiddlewarePlugin[] ): React.ComponentType> { if (middleware.length === 0) { + log.debug( + 'No middleware to chain for component', + baseComponent.displayName ?? baseComponent.name + ); return baseComponent; } + log.debug( + 'Chaining component middleware', + baseComponent.displayName ?? baseComponent.name, + middleware.map(m => m.name) + ); + // Build the chain from inside out (base component is innermost) // Middleware is ordered outermost to innermost, so we reverse to build from inside out return [...middleware] @@ -156,9 +166,19 @@ export function createChainedPanelComponent( ); if (panelMiddleware.length === 0) { + log.debug( + 'No panel middleware to chain for panel component', + basePanelComponent.displayName ?? basePanelComponent.name + ); return basePanelComponent; } + log.debug( + 'Chaining panel middleware', + basePanelComponent.displayName ?? basePanelComponent.name, + panelMiddleware.map(m => m.name) + ); + // Build the chain from inside out (base panel is innermost) return [...panelMiddleware] .reverse() diff --git a/packages/plugin/src/WidgetView.tsx b/packages/plugin/src/WidgetView.tsx index d15a36a1aa..56be1df0e9 100644 --- a/packages/plugin/src/WidgetView.tsx +++ b/packages/plugin/src/WidgetView.tsx @@ -44,17 +44,30 @@ export function WidgetView({ fetch, type }: WidgetViewProps): JSX.Element { } }); + log.debug( + `WidgetView resolved plugins for type ${type}:`, + 'base=', + foundBasePlugin?.name ?? 'none', + 'middleware=', + foundMiddleware.map(m => m.name) + ); + return { basePlugin: foundBasePlugin, middleware: foundMiddleware }; }, [plugins, type]); const ChainedComponent = useMemo(() => { if (basePlugin == null) { + log.debug(`No base plugin found for widget type ${type}`); return null; } return createChainedComponent(basePlugin.component, middleware); }, [basePlugin, middleware]); if (ChainedComponent != null) { + log.debug( + `Rendering chained component for type ${type}:`, + ChainedComponent.displayName ?? ChainedComponent.name + ); return ; } From 00e45e7afac2075985388996f1cca2c245d9619c Mon Sep 17 00:00:00 2001 From: Vlad Babich Date: Mon, 20 Apr 2026 07:18:56 -0600 Subject: [PATCH 08/25] Fix eslint --- packages/plugin/src/WidgetView.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/plugin/src/WidgetView.tsx b/packages/plugin/src/WidgetView.tsx index 56be1df0e9..cb0e322a11 100644 --- a/packages/plugin/src/WidgetView.tsx +++ b/packages/plugin/src/WidgetView.tsx @@ -61,7 +61,7 @@ export function WidgetView({ fetch, type }: WidgetViewProps): JSX.Element { return null; } return createChainedComponent(basePlugin.component, middleware); - }, [basePlugin, middleware]); + }, [basePlugin, middleware, type]); if (ChainedComponent != null) { log.debug( From c5bb6a219903a7bf9b7eec7b9e97ab3f2dab6f1b Mon Sep 17 00:00:00 2001 From: Vlad Babich Date: Tue, 21 Apr 2026 16:35:03 -0600 Subject: [PATCH 09/25] Plugin chaining - dynamic dependencies --- packages/app-utils/package.json | 1 + .../app-utils/src/plugins/PluginUtils.test.ts | 212 +++++++++++++++++- .../app-utils/src/plugins/PluginUtils.tsx | 35 +-- .../src/plugins/remote-component.config.ts | 6 +- 4 files changed, 238 insertions(+), 16 deletions(-) diff --git a/packages/app-utils/package.json b/packages/app-utils/package.json index 4754a3765f..701dd4d93e 100644 --- a/packages/app-utils/package.json +++ b/packages/app-utils/package.json @@ -34,6 +34,7 @@ "@deephaven/dashboard-core-plugins": "file:../dashboard-core-plugins", "@deephaven/file-explorer": "file:../file-explorer", "@deephaven/golden-layout": "file:../golden-layout", + "@deephaven/grid": "file:../grid", "@deephaven/icons": "file:../icons", "@deephaven/iris-grid": "file:../iris-grid", "@deephaven/jsapi-bootstrap": "file:../jsapi-bootstrap", diff --git a/packages/app-utils/src/plugins/PluginUtils.test.ts b/packages/app-utils/src/plugins/PluginUtils.test.ts index 4e060c6ca2..865c058f97 100644 --- a/packages/app-utils/src/plugins/PluginUtils.test.ts +++ b/packages/app-utils/src/plugins/PluginUtils.test.ts @@ -4,7 +4,22 @@ import { type Plugin, PluginType, } from '@deephaven/plugin'; -import { getPluginModuleValue } from './PluginUtils'; +import { getPluginModuleValue, loadModulePlugins } from './PluginUtils'; +import { resolve } from './remote-component.config'; + +jest.mock('./loadRemoteModule', () => { + const mockLoadRemoteModule = jest.fn(); + return { + __esModule: true, + default: mockLoadRemoteModule, + loadRemoteModule: mockLoadRemoteModule, + }; +}); + +// eslint-disable-next-line @typescript-eslint/no-var-requires, global-require +const { default: loadRemoteModule } = require('./loadRemoteModule') as { + default: jest.Mock; +}; describe('getPluginModuleValue', () => { const legacyPlugins: [type: string, moduleValue: LegacyPlugin][] = [ @@ -164,3 +179,198 @@ describe('getPluginModuleValue', () => { }); }); }); + +describe('loadModulePlugins', () => { + const BASE_URL = 'http://localhost:4100/plugins'; + + function makePlugin(name: string): Plugin { + return { name, type: PluginType.WIDGET_PLUGIN }; + } + + function mockManifest( + plugins: { name: string; main: string; version: string }[] + ) { + global.fetch = jest.fn().mockResolvedValue({ + ok: true, + json: jest.fn().mockResolvedValue({ plugins }), + }); + } + + beforeEach(() => { + jest.clearAllMocks(); + // Clean up any plugin entries added to resolve in previous tests + Object.keys(resolve).forEach(key => { + if (key.startsWith('test-plugin')) { + delete resolve[key]; + } + }); + }); + + it('loads plugins sequentially and registers them in the plugin map', async () => { + const pluginA = makePlugin('test-plugin-a'); + const pluginB = makePlugin('test-plugin-b'); + + mockManifest([ + { name: 'test-plugin-a', main: 'index.js', version: '1.0.0' }, + { name: 'test-plugin-b', main: 'index.js', version: '2.0.0' }, + ]); + + loadRemoteModule + .mockResolvedValueOnce(pluginA) + .mockResolvedValueOnce(pluginB); + + const pluginMap = await loadModulePlugins(BASE_URL); + + expect(pluginMap.size).toBe(2); + expect(pluginMap.get('test-plugin-a')).toEqual({ + ...pluginA, + version: '1.0.0', + }); + expect(pluginMap.get('test-plugin-b')).toEqual({ + ...pluginB, + version: '2.0.0', + }); + }); + + it('registers each plugin in the resolve map for plugin-to-plugin dependencies', async () => { + const pluginA = makePlugin('test-plugin-a'); + const moduleExports = { default: pluginA, SomeHelper: 'helper-value' }; + + mockManifest([ + { name: 'test-plugin-a', main: 'index.js', version: '1.0.0' }, + ]); + + loadRemoteModule.mockResolvedValueOnce(moduleExports); + + await loadModulePlugins(BASE_URL); + + // The raw module exports (not the extracted PluginModule) should be in the resolve map + expect(resolve['test-plugin-a']).toBe(moduleExports); + }); + + it('makes earlier plugins available to later plugins via resolve map', async () => { + const pluginA = makePlugin('test-plugin-a'); + const pluginB = makePlugin('test-plugin-b'); + + mockManifest([ + { name: 'test-plugin-a', main: 'index.js', version: '1.0.0' }, + { name: 'test-plugin-b', main: 'index.js', version: '1.0.0' }, + ]); + + const moduleA = { default: pluginA, ExportedClass: class MyClass {} }; + + // When plugin B loads, verify plugin A is already in the resolve map + loadRemoteModule.mockResolvedValueOnce(moduleA).mockImplementationOnce( + () => + new Promise(res => { + // At this point, plugin A should already be registered + expect(resolve['test-plugin-a']).toBe(moduleA); + res(pluginB); + }) + ); + + await loadModulePlugins(BASE_URL); + + expect(resolve['test-plugin-a']).toBe(moduleA); + expect(resolve['test-plugin-b']).toBe(pluginB); + }); + + it('continues loading remaining plugins when one fails', async () => { + const pluginB = makePlugin('test-plugin-b'); + + mockManifest([ + { name: 'test-plugin-a', main: 'index.js', version: '1.0.0' }, + { name: 'test-plugin-b', main: 'index.js', version: '1.0.0' }, + ]); + + loadRemoteModule + .mockRejectedValueOnce(new Error('Network error')) + .mockResolvedValueOnce(pluginB); + + const pluginMap = await loadModulePlugins(BASE_URL); + + expect(pluginMap.size).toBe(1); + expect(pluginMap.has('test-plugin-a')).toBe(false); + expect(pluginMap.get('test-plugin-b')).toEqual({ + ...pluginB, + version: '1.0.0', + }); + }); + + it('does not register a failed plugin in the resolve map', async () => { + mockManifest([ + { name: 'test-plugin-a', main: 'index.js', version: '1.0.0' }, + ]); + + loadRemoteModule.mockRejectedValueOnce(new Error('Load failed')); + + await loadModulePlugins(BASE_URL); + + expect(resolve['test-plugin-a']).toBeUndefined(); + }); + + it('returns empty map when manifest fetch fails', async () => { + global.fetch = jest.fn().mockResolvedValue({ + ok: false, + statusText: 'Not Found', + }); + + const pluginMap = await loadModulePlugins(BASE_URL); + + expect(pluginMap.size).toBe(0); + }); + + it('returns empty map when manifest has no plugins array', async () => { + global.fetch = jest.fn().mockResolvedValue({ + ok: true, + json: jest.fn().mockResolvedValue({}), + }); + + const pluginMap = await loadModulePlugins(BASE_URL); + + expect(pluginMap.size).toBe(0); + }); + + it('flattens MultiPlugin and registers inner plugins', async () => { + const multiPlugin: MultiPlugin = { + name: 'test-plugin-multi', + type: PluginType.MULTI_PLUGIN, + plugins: [ + makePlugin('test-plugin-inner-1'), + makePlugin('test-plugin-inner-2'), + ] as Plugin[], + }; + + mockManifest([ + { name: 'test-plugin-multi', main: 'index.js', version: '1.0.0' }, + ]); + + loadRemoteModule.mockResolvedValueOnce(multiPlugin); + + const pluginMap = await loadModulePlugins(BASE_URL); + + // Inner plugins should be registered by their own names + expect(pluginMap.size).toBe(2); + expect(pluginMap.has('test-plugin-inner-1')).toBe(true); + expect(pluginMap.has('test-plugin-inner-2')).toBe(true); + + // The raw module should still be in the resolve map under the package name + expect(resolve['test-plugin-multi']).toBe(multiPlugin); + }); + + it('loads plugins from correct URLs based on manifest', async () => { + const pluginA = makePlugin('test-plugin-a'); + + mockManifest([ + { name: 'test-plugin-a', main: 'bundle.js', version: '1.0.0' }, + ]); + + loadRemoteModule.mockResolvedValueOnce(pluginA); + + await loadModulePlugins(BASE_URL); + + expect(loadRemoteModule).toHaveBeenCalledWith( + `${BASE_URL}/test-plugin-a/bundle.js` + ); + }); +}); diff --git a/packages/app-utils/src/plugins/PluginUtils.tsx b/packages/app-utils/src/plugins/PluginUtils.tsx index 7db35f84ea..1d5d2d1995 100644 --- a/packages/app-utils/src/plugins/PluginUtils.tsx +++ b/packages/app-utils/src/plugins/PluginUtils.tsx @@ -15,6 +15,7 @@ import { isMultiPlugin, } from '@deephaven/plugin'; import loadRemoteModule from './loadRemoteModule'; +import { resolve } from './remote-component.config'; const log = Log.module('@deephaven/app-utils.PluginUtils'); @@ -104,7 +105,10 @@ function registerPlugin( } /** - * Load all plugin modules available based on the manifest file at the provided base URL + * Load all plugin modules available based on the manifest file at the provided base URL. + * Plugins are loaded sequentially so that each plugin's exports are registered + * in the module resolve map before subsequent plugins load. This allows plugins + * to depend on other plugins via standard require() calls. * @param modulePluginsUrl The base URL of the module plugins to load * @returns A map from the name of the plugin to the plugin module that was loaded */ @@ -120,21 +124,24 @@ export async function loadModulePlugins( } log.debug('Plugin manifest loaded:', manifest); - const pluginPromises: Promise[] = []; + + const pluginMap: PluginModuleMap = new Map(); + + // Load plugins sequentially so each plugin's exports are available + // to subsequently loaded plugins via require() for (let i = 0; i < manifest.plugins.length; i += 1) { - const { name, main } = manifest.plugins[i]; + const { name, main, version } = manifest.plugins[i]; const pluginMainUrl = `${modulePluginsUrl}/${name}/${main}`; - pluginPromises.push(loadModulePlugin(pluginMainUrl)); - } + try { + // eslint-disable-next-line no-await-in-loop + const myModule = await loadModulePlugin(pluginMainUrl); - const pluginModules = await Promise.allSettled(pluginPromises); + // Register the raw module exports in the resolve map so + // subsequent plugins can require() this plugin by name + resolve[name] = myModule; + log.debug(`Registered plugin '${name}' in module resolve map`); - const pluginMap: PluginModuleMap = new Map(); - for (let i = 0; i < pluginModules.length; i += 1) { - const module = pluginModules[i]; - const { name, version } = manifest.plugins[i]; - if (module.status === 'fulfilled') { - const moduleValue = getPluginModuleValue(module.value); + const moduleValue = getPluginModuleValue(myModule); if (moduleValue == null) { log.error(`Plugin '${name}' is missing an exported value.`); } else if (isMultiPlugin(moduleValue)) { @@ -148,8 +155,8 @@ export async function loadModulePlugins( } else { registerPlugin(pluginMap, name, moduleValue, version); } - } else { - log.error(`Unable to load plugin '${name}'`, module.reason); + } catch (e) { + log.error(`Unable to load plugin '${name}'`, e); } } log.info('Plugins loaded:', pluginMap); diff --git a/packages/app-utils/src/plugins/remote-component.config.ts b/packages/app-utils/src/plugins/remote-component.config.ts index e81e1da484..cb8edf96ee 100644 --- a/packages/app-utils/src/plugins/remote-component.config.ts +++ b/packages/app-utils/src/plugins/remote-component.config.ts @@ -14,6 +14,7 @@ import * as DeephavenChart from '@deephaven/chart'; import * as DeephavenComponents from '@deephaven/components'; import * as DeephavenDashboard from '@deephaven/dashboard'; import * as DeephavenDashboardCorePlugins from '@deephaven/dashboard-core-plugins'; +import * as DeephavenGrid from '@deephaven/grid'; import * as DeephavenIcons from '@deephaven/icons'; import * as DeephavenIrisGrid from '@deephaven/iris-grid'; import * as DeephavenJsapiBootstrap from '@deephaven/jsapi-bootstrap'; @@ -23,9 +24,10 @@ import * as DeephavenConsole from '@deephaven/console'; import DeephavenLog from '@deephaven/log'; import * as DeephavenReactHooks from '@deephaven/react-hooks'; import * as DeephavenPlugin from '@deephaven/plugin'; +import * as DeephavenUtils from '@deephaven/utils'; // eslint-disable-next-line import/prefer-default-export -export const resolve = { +export const resolve: Record = { react, 'react-dom': ReactDOM, redux, @@ -37,6 +39,7 @@ export const resolve = { '@deephaven/console': DeephavenConsole, '@deephaven/dashboard': DeephavenDashboard, '@deephaven/dashboard-core-plugins': DeephavenDashboardCorePlugins, + '@deephaven/grid': DeephavenGrid, '@deephaven/icons': DeephavenIcons, '@deephaven/iris-grid': DeephavenIrisGrid, '@deephaven/jsapi-bootstrap': DeephavenJsapiBootstrap, @@ -45,4 +48,5 @@ export const resolve = { '@deephaven/log': DeephavenLog, '@deephaven/plugin': DeephavenPlugin, '@deephaven/react-hooks': DeephavenReactHooks, + '@deephaven/utils': DeephavenUtils, }; From 0b773bc39bbb8b2868e3fc1b0429452ed6421630 Mon Sep 17 00:00:00 2001 From: Vlad Babich Date: Thu, 23 Apr 2026 16:55:34 -0600 Subject: [PATCH 10/25] Allow cross-plugin imports --- .../app-utils/src/plugins/PluginUtils.test.ts | 85 ++++++++++++++++--- .../app-utils/src/plugins/PluginUtils.tsx | 21 +++-- 2 files changed, 89 insertions(+), 17 deletions(-) diff --git a/packages/app-utils/src/plugins/PluginUtils.test.ts b/packages/app-utils/src/plugins/PluginUtils.test.ts index 865c058f97..9a4d56ec05 100644 --- a/packages/app-utils/src/plugins/PluginUtils.test.ts +++ b/packages/app-utils/src/plugins/PluginUtils.test.ts @@ -200,7 +200,11 @@ describe('loadModulePlugins', () => { jest.clearAllMocks(); // Clean up any plugin entries added to resolve in previous tests Object.keys(resolve).forEach(key => { - if (key.startsWith('test-plugin')) { + if ( + key.startsWith('test-plugin') || + key.startsWith('@deephaven/js-plugin-test-plugin') || + key.startsWith('@acme/') + ) { delete resolve[key]; } }); @@ -237,7 +241,12 @@ describe('loadModulePlugins', () => { const moduleExports = { default: pluginA, SomeHelper: 'helper-value' }; mockManifest([ - { name: 'test-plugin-a', main: 'index.js', version: '1.0.0' }, + { + name: 'test-plugin-a', + main: 'index.js', + version: '1.0.0', + package: '@deephaven/js-plugin-test-plugin-a', + }, ]); loadRemoteModule.mockResolvedValueOnce(moduleExports); @@ -245,7 +254,24 @@ describe('loadModulePlugins', () => { await loadModulePlugins(BASE_URL); // The raw module exports (not the extracted PluginModule) should be in the resolve map - expect(resolve['test-plugin-a']).toBe(moduleExports); + expect(resolve['@deephaven/js-plugin-test-plugin-a']).toBe(moduleExports); + }); + + it('does not register plugin in resolve map when package field is absent', async () => { + const pluginA = makePlugin('test-plugin-a'); + const moduleExports = { default: pluginA }; + + mockManifest([ + { name: 'test-plugin-a', main: 'index.js', version: '1.0.0' }, + ]); + + loadRemoteModule.mockResolvedValueOnce(moduleExports); + + await loadModulePlugins(BASE_URL); + + // No package field → not registered in resolve map + expect(resolve['@deephaven/js-plugin-test-plugin-a']).toBeUndefined(); + expect(resolve['test-plugin-a']).toBeUndefined(); }); it('makes earlier plugins available to later plugins via resolve map', async () => { @@ -253,8 +279,18 @@ describe('loadModulePlugins', () => { const pluginB = makePlugin('test-plugin-b'); mockManifest([ - { name: 'test-plugin-a', main: 'index.js', version: '1.0.0' }, - { name: 'test-plugin-b', main: 'index.js', version: '1.0.0' }, + { + name: 'test-plugin-a', + main: 'index.js', + version: '1.0.0', + package: '@deephaven/js-plugin-test-plugin-a', + }, + { + name: 'test-plugin-b', + main: 'index.js', + version: '1.0.0', + package: '@deephaven/js-plugin-test-plugin-b', + }, ]); const moduleA = { default: pluginA, ExportedClass: class MyClass {} }; @@ -264,15 +300,15 @@ describe('loadModulePlugins', () => { () => new Promise(res => { // At this point, plugin A should already be registered - expect(resolve['test-plugin-a']).toBe(moduleA); + expect(resolve['@deephaven/js-plugin-test-plugin-a']).toBe(moduleA); res(pluginB); }) ); await loadModulePlugins(BASE_URL); - expect(resolve['test-plugin-a']).toBe(moduleA); - expect(resolve['test-plugin-b']).toBe(pluginB); + expect(resolve['@deephaven/js-plugin-test-plugin-a']).toBe(moduleA); + expect(resolve['@deephaven/js-plugin-test-plugin-b']).toBe(pluginB); }); it('continues loading remaining plugins when one fails', async () => { @@ -299,14 +335,19 @@ describe('loadModulePlugins', () => { it('does not register a failed plugin in the resolve map', async () => { mockManifest([ - { name: 'test-plugin-a', main: 'index.js', version: '1.0.0' }, + { + name: 'test-plugin-a', + main: 'index.js', + version: '1.0.0', + package: '@deephaven/js-plugin-test-plugin-a', + }, ]); loadRemoteModule.mockRejectedValueOnce(new Error('Load failed')); await loadModulePlugins(BASE_URL); - expect(resolve['test-plugin-a']).toBeUndefined(); + expect(resolve['@deephaven/js-plugin-test-plugin-a']).toBeUndefined(); }); it('returns empty map when manifest fetch fails', async () => { @@ -355,7 +396,7 @@ describe('loadModulePlugins', () => { expect(pluginMap.has('test-plugin-inner-2')).toBe(true); // The raw module should still be in the resolve map under the package name - expect(resolve['test-plugin-multi']).toBe(multiPlugin); + expect(resolve['@deephaven/js-plugin-test-plugin-multi']).toBeUndefined(); }); it('loads plugins from correct URLs based on manifest', async () => { @@ -373,4 +414,26 @@ describe('loadModulePlugins', () => { `${BASE_URL}/test-plugin-a/bundle.js` ); }); + + it('uses manifest package field as resolve key when provided', async () => { + const pluginA = makePlugin('test-plugin-a'); + const moduleExports = { default: pluginA }; + + mockManifest([ + { + name: 'test-plugin-a', + main: 'index.js', + version: '1.0.0', + package: '@acme/grid-extras', + }, + ]); + + loadRemoteModule.mockResolvedValueOnce(moduleExports); + + await loadModulePlugins(BASE_URL); + + // Should use the explicit package name, not the fallback + expect(resolve['@acme/grid-extras']).toBe(moduleExports); + expect(resolve['@deephaven/js-plugin-test-plugin-a']).toBeUndefined(); + }); }); diff --git a/packages/app-utils/src/plugins/PluginUtils.tsx b/packages/app-utils/src/plugins/PluginUtils.tsx index 1d5d2d1995..40ca3beb6a 100644 --- a/packages/app-utils/src/plugins/PluginUtils.tsx +++ b/packages/app-utils/src/plugins/PluginUtils.tsx @@ -22,6 +22,13 @@ const log = Log.module('@deephaven/app-utils.PluginUtils'); export type PluginManifestPluginInfo = { name: string; main: string; + /** + * The npm package name for this plugin (e.g. `@deephaven/js-plugin-pivot`). + * When provided, the plugin's exports are registered in the module resolve + * map under this key, enabling other plugins to `require()` this plugin at + * runtime. If omitted, the plugin is not registered for cross-plugin imports. + */ + package?: string; version: string; }; @@ -130,18 +137,20 @@ export async function loadModulePlugins( // Load plugins sequentially so each plugin's exports are available // to subsequently loaded plugins via require() for (let i = 0; i < manifest.plugins.length; i += 1) { - const { name, main, version } = manifest.plugins[i]; + const { name, main, version, package: packageName } = manifest.plugins[i]; const pluginMainUrl = `${modulePluginsUrl}/${name}/${main}`; try { // eslint-disable-next-line no-await-in-loop - const myModule = await loadModulePlugin(pluginMainUrl); + const pluginExports = await loadModulePlugin(pluginMainUrl); // Register the raw module exports in the resolve map so - // subsequent plugins can require() this plugin by name - resolve[name] = myModule; - log.debug(`Registered plugin '${name}' in module resolve map`); + // subsequent plugins can require() this plugin by its package name + if (packageName != null) { + resolve[packageName] = pluginExports; + log.debug(`Registered plugin '${packageName}' in module resolve map`); + } - const moduleValue = getPluginModuleValue(myModule); + const moduleValue = getPluginModuleValue(pluginExports); if (moduleValue == null) { log.error(`Plugin '${name}' is missing an exported value.`); } else if (isMultiPlugin(moduleValue)) { From f21c0dd81162c10c27d4d4406661b518914b2143 Mon Sep 17 00:00:00 2001 From: Vlad Babich Date: Thu, 23 Apr 2026 17:11:06 -0600 Subject: [PATCH 11/25] Update .md --- packages/plugin/MIDDLEWARE_ARCHITECTURE.md | 58 ++++++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/packages/plugin/MIDDLEWARE_ARCHITECTURE.md b/packages/plugin/MIDDLEWARE_ARCHITECTURE.md index 47aef1c4e2..fc37e3ace8 100644 --- a/packages/plugin/MIDDLEWARE_ARCHITECTURE.md +++ b/packages/plugin/MIDDLEWARE_ARCHITECTURE.md @@ -170,3 +170,61 @@ const plugins = new Map([ 3. **Middleware must render `Component`.** If a middleware doesn't render the `Component` prop, the rest of the chain (including the base widget) will not appear. 4. **Middleware must spread props.** Pass all received props to `Component` to ensure the base widget and other middleware receive them. 5. **`panelComponent` middleware is separate.** If the base plugin defines a `panelComponent`, middleware targeting the panel layer must also define `panelComponent`. + +## Cross-Plugin Dependencies + +Plugins load sequentially in manifest order. A plugin can expose its exports for later plugins to import at runtime by declaring a `package` field in the manifest. + +``` +1. pivot loads → resolve["@deephaven/js-plugin-pivot"] = exports +2. grid-toolbar loads → import "@deephaven/js-plugin-pivot" ✓ +``` + +### Optional Manifest `package` Field + +When present, the plugin's exports are registered in the resolve map under this key. + +```json +{ + "plugins": [ + { "name": "pivot", "main": "src/js/dist/index.js", "package": "@deephaven/js-plugin-pivot" }, + { "name": "grid-toolbar", "main": "src/js/dist/bundle/index.js" } + ] +} +``` + +Here `pivot` is importable (has `package`); `grid-toolbar` only consumes. + +### Consuming Another Plugin + +1. **Externalize** in your build config: + ```ts + // vite.config.ts + rollupOptions: { external: ['@deephaven/js-plugin-pivot'] } + ``` + +2. **Import normally** in source: + ```ts + import { IrisGridPivotModel } from '@deephaven/js-plugin-pivot'; + ``` + +3. **Provide types.** In a monorepo, use `paths` in `tsconfig.json`: + ```json + { "compilerOptions": { "paths": { + "@deephaven/js-plugin-pivot": ["../pivot/src/js/src/index.ts"] + }}} + ``` + For cross-repo dependencies, use ambient declarations instead: + ```ts + // pivotPlugin.d.ts + declare module '@deephaven/js-plugin-pivot' { export class IrisGridPivotModel {} } + ``` + +4. **Ensure load order** — the dependency must appear before the consumer in `manifest.json`. + +### Rules + +- A plugin can only import plugins loaded earlier in the manifest. + **TODO**: add `dependencies` field to manifest and verify load order at runtime. +- Only plugins with a `package` field are registered in the resolve map. +- The `package` value must exactly match the `import` string. From 8442cd4e83ce00f6cd092799fb0bd5a4aa6538d4 Mon Sep 17 00:00:00 2001 From: Vlad Babich Date: Thu, 23 Apr 2026 17:29:04 -0600 Subject: [PATCH 12/25] Remove unnecessary changes --- packages/app-utils/src/plugins/remote-component.config.ts | 4 ---- 1 file changed, 4 deletions(-) diff --git a/packages/app-utils/src/plugins/remote-component.config.ts b/packages/app-utils/src/plugins/remote-component.config.ts index cb8edf96ee..98805705d8 100644 --- a/packages/app-utils/src/plugins/remote-component.config.ts +++ b/packages/app-utils/src/plugins/remote-component.config.ts @@ -14,7 +14,6 @@ import * as DeephavenChart from '@deephaven/chart'; import * as DeephavenComponents from '@deephaven/components'; import * as DeephavenDashboard from '@deephaven/dashboard'; import * as DeephavenDashboardCorePlugins from '@deephaven/dashboard-core-plugins'; -import * as DeephavenGrid from '@deephaven/grid'; import * as DeephavenIcons from '@deephaven/icons'; import * as DeephavenIrisGrid from '@deephaven/iris-grid'; import * as DeephavenJsapiBootstrap from '@deephaven/jsapi-bootstrap'; @@ -24,7 +23,6 @@ import * as DeephavenConsole from '@deephaven/console'; import DeephavenLog from '@deephaven/log'; import * as DeephavenReactHooks from '@deephaven/react-hooks'; import * as DeephavenPlugin from '@deephaven/plugin'; -import * as DeephavenUtils from '@deephaven/utils'; // eslint-disable-next-line import/prefer-default-export export const resolve: Record = { @@ -39,7 +37,6 @@ export const resolve: Record = { '@deephaven/console': DeephavenConsole, '@deephaven/dashboard': DeephavenDashboard, '@deephaven/dashboard-core-plugins': DeephavenDashboardCorePlugins, - '@deephaven/grid': DeephavenGrid, '@deephaven/icons': DeephavenIcons, '@deephaven/iris-grid': DeephavenIrisGrid, '@deephaven/jsapi-bootstrap': DeephavenJsapiBootstrap, @@ -48,5 +45,4 @@ export const resolve: Record = { '@deephaven/log': DeephavenLog, '@deephaven/plugin': DeephavenPlugin, '@deephaven/react-hooks': DeephavenReactHooks, - '@deephaven/utils': DeephavenUtils, }; From c1d74d67570260ac5dca05bb22b63b7768c29b19 Mon Sep 17 00:00:00 2001 From: Vlad Babich Date: Thu, 23 Apr 2026 18:06:23 -0600 Subject: [PATCH 13/25] PluginUtils deduplication phase 1 --- .../app-utils/src/plugins/PluginUtils.test.ts | 192 +++++++++++++++++- .../app-utils/src/plugins/PluginUtils.tsx | 105 +++++++--- 2 files changed, 270 insertions(+), 27 deletions(-) diff --git a/packages/app-utils/src/plugins/PluginUtils.test.ts b/packages/app-utils/src/plugins/PluginUtils.test.ts index 9a4d56ec05..3aa15dffe5 100644 --- a/packages/app-utils/src/plugins/PluginUtils.test.ts +++ b/packages/app-utils/src/plugins/PluginUtils.test.ts @@ -2,9 +2,15 @@ import { type LegacyPlugin, type MultiPlugin, type Plugin, + type PluginModuleMap, PluginType, } from '@deephaven/plugin'; -import { getPluginModuleValue, loadModulePlugins } from './PluginUtils'; +import { + getPluginModuleValue, + loadModulePlugins, + processLoadedModule, + registerPlugin, +} from './PluginUtils'; import { resolve } from './remote-component.config'; jest.mock('./loadRemoteModule', () => { @@ -180,6 +186,190 @@ describe('getPluginModuleValue', () => { }); }); +describe('registerPlugin', () => { + function makePlugin(name: string): Plugin { + return { name, type: PluginType.WIDGET_PLUGIN }; + } + + it('registers a plugin in the map', () => { + const pluginMap: PluginModuleMap = new Map(); + const plugin = makePlugin('my-plugin'); + + registerPlugin(pluginMap, 'my-plugin', plugin, '1.0.0'); + + expect(pluginMap.get('my-plugin')).toEqual({ ...plugin, version: '1.0.0' }); + }); + + it('skips duplicate and keeps the first registration', () => { + const pluginMap: PluginModuleMap = new Map(); + const first = makePlugin('my-plugin'); + const second = makePlugin('my-plugin'); + + registerPlugin(pluginMap, 'my-plugin', first, '1.0.0'); + registerPlugin(pluginMap, 'my-plugin', second, '2.0.0'); + + expect(pluginMap.size).toBe(1); + expect(pluginMap.get('my-plugin')).toEqual({ ...first, version: '1.0.0' }); + }); +}); + +describe('processLoadedModule', () => { + function makePlugin(name: string): Plugin { + return { name, type: PluginType.WIDGET_PLUGIN }; + } + + let pluginMap: PluginModuleMap; + let resolveMap: Record; + + beforeEach(() => { + pluginMap = new Map(); + resolveMap = {}; + }); + + it('registers a simple plugin', () => { + const plugin = makePlugin('my-plugin'); + processLoadedModule( + pluginMap, + resolveMap, + plugin, + 'my-plugin', + null, + '1.0.0' + ); + + expect(pluginMap.size).toBe(1); + expect(pluginMap.get('my-plugin')).toEqual({ ...plugin, version: '1.0.0' }); + }); + + it('registers a plugin with default export', () => { + const plugin = makePlugin('my-plugin'); + processLoadedModule( + pluginMap, + resolveMap, + { default: plugin }, + 'my-plugin' + ); + + expect(pluginMap.size).toBe(1); + expect(pluginMap.get('my-plugin')).toEqual({ + ...plugin, + version: undefined, + }); + }); + + it('registers in resolve map when packageName is provided', () => { + const plugin = makePlugin('my-plugin'); + const exports = { default: plugin }; + processLoadedModule( + pluginMap, + resolveMap, + exports, + 'my-plugin', + '@scope/my-plugin' + ); + + expect(resolveMap['@scope/my-plugin']).toBe(exports); + }); + + it('does not register in resolve map when packageName is null', () => { + const plugin = makePlugin('my-plugin'); + processLoadedModule(pluginMap, resolveMap, plugin, 'my-plugin', null); + + expect(Object.keys(resolveMap)).toHaveLength(0); + }); + + it('does not register in resolve map when packageName is undefined', () => { + const plugin = makePlugin('my-plugin'); + processLoadedModule(pluginMap, resolveMap, plugin, 'my-plugin'); + + expect(Object.keys(resolveMap)).toHaveLength(0); + }); + + it('flattens MultiPlugin and registers inner plugins', () => { + const multi: MultiPlugin = { + name: 'multi', + type: PluginType.MULTI_PLUGIN, + plugins: [makePlugin('inner-a'), makePlugin('inner-b')] as Plugin[], + }; + + processLoadedModule(pluginMap, resolveMap, multi, 'multi', null, '1.0.0'); + + expect(pluginMap.size).toBe(2); + expect(pluginMap.has('inner-a')).toBe(true); + expect(pluginMap.has('inner-b')).toBe(true); + expect(pluginMap.has('multi')).toBe(false); + }); + + it('skips invalid inner plugins in MultiPlugin', () => { + const multi: MultiPlugin = { + name: 'multi', + type: PluginType.MULTI_PLUGIN, + plugins: [ + makePlugin('valid'), + { notAPlugin: true } as unknown as Plugin, + ] as Plugin[], + }; + + processLoadedModule(pluginMap, resolveMap, multi, 'multi'); + + expect(pluginMap.size).toBe(1); + expect(pluginMap.has('valid')).toBe(true); + }); + + it('skips inner plugins with empty names in MultiPlugin', () => { + const multi: MultiPlugin = { + name: 'multi', + type: PluginType.MULTI_PLUGIN, + plugins: [ + makePlugin('valid'), + { name: '', type: PluginType.WIDGET_PLUGIN } as Plugin, + { name: ' ', type: PluginType.WIDGET_PLUGIN } as Plugin, + ] as Plugin[], + }; + + processLoadedModule(pluginMap, resolveMap, multi, 'multi'); + + expect(pluginMap.size).toBe(1); + expect(pluginMap.has('valid')).toBe(true); + }); + + it('skips duplicate inner plugins in MultiPlugin', () => { + const multi: MultiPlugin = { + name: 'multi', + type: PluginType.MULTI_PLUGIN, + plugins: [makePlugin('dupe'), makePlugin('dupe')] as Plugin[], + }; + + processLoadedModule(pluginMap, resolveMap, multi, 'multi'); + + expect(pluginMap.size).toBe(1); + }); + + it('does not register when module value is null', () => { + processLoadedModule(pluginMap, resolveMap, {} as Plugin, 'bad-plugin'); + + expect(pluginMap.size).toBe(0); + }); + + it('handles legacy plugin format', () => { + const legacy: LegacyPlugin = { TablePlugin: () => null }; + processLoadedModule( + pluginMap, + resolveMap, + legacy, + 'legacy-plugin', + null, + '1.0.0' + ); + + expect(pluginMap.size).toBe(1); + expect(pluginMap.get('legacy-plugin')).toEqual({ + ...legacy, + version: '1.0.0', + }); + }); +}); + describe('loadModulePlugins', () => { const BASE_URL = 'http://localhost:4100/plugins'; diff --git a/packages/app-utils/src/plugins/PluginUtils.tsx b/packages/app-utils/src/plugins/PluginUtils.tsx index 40ca3beb6a..5e9fb317c9 100644 --- a/packages/app-utils/src/plugins/PluginUtils.tsx +++ b/packages/app-utils/src/plugins/PluginUtils.tsx @@ -91,26 +91,92 @@ export function getPluginModuleValue( } /** - * Register a plugin in the plugin map, logging a warning if a plugin with the same name already exists. + * Register a plugin in the plugin map. If a plugin with the same name is + * already registered, the duplicate is skipped and a warning is logged. * @param pluginMap The plugin map to register the plugin in * @param name The name to register the plugin under * @param plugin The plugin to register * @param version Optional version to attach to the plugin */ -function registerPlugin( +export function registerPlugin( pluginMap: PluginModuleMap, name: string, plugin: PluginModule, version?: string ): void { if (pluginMap.has(name)) { - log.warn( - `Plugin '${name}' is already registered. The existing plugin will be replaced.` - ); + log.warn(`Plugin '${name}' is already registered. Skipping duplicate.`); + return; } pluginMap.set(name, { ...plugin, version }); } +/** + * Process a loaded plugin module: extract the plugin value, register in the + * resolve map for cross-plugin imports, flatten MultiPlugins, and register + * each resulting plugin in the plugin map. + * + * @param pluginMap The plugin map to register plugins in + * @param resolveMap The module resolve map for cross-plugin imports + * @param pluginExports The raw module exports from loading the plugin + * @param name The manifest name of the plugin + * @param packageName Optional package name for resolve map registration + * @param version Optional version to attach to registered plugins + */ +export function processLoadedModule( + pluginMap: PluginModuleMap, + resolveMap: Record, + pluginExports: unknown, + name: string, + packageName?: string | null, + version?: string +): void { + // Register raw module exports in the resolve map so subsequent + // plugins can import this plugin by its package name + if (packageName != null) { + resolveMap[packageName] = pluginExports; + log.debug(`Registered plugin '${packageName}' in module resolve map`); + } + + const moduleValue = getPluginModuleValue( + pluginExports as LegacyPlugin | Plugin | { default: Plugin } + ); + if (moduleValue == null) { + log.error(`Plugin '${name}' is missing an exported value.`); + return; + } + + if (isMultiPlugin(moduleValue)) { + log.debug( + `MultiPlugin '${name}' contains ${moduleValue.plugins.length} plugins` + ); + moduleValue.plugins.forEach(innerPlugin => { + if (!isPlugin(innerPlugin)) { + log.warn( + `Skipping invalid inner plugin from MultiPlugin '${name}'`, + innerPlugin + ); + return; + } + + const innerPluginName = + typeof innerPlugin.name === 'string' ? innerPlugin.name.trim() : ''; + + if (innerPluginName.length === 0) { + log.warn( + `Skipping unnamed inner plugin from MultiPlugin '${name}'`, + innerPlugin + ); + return; + } + + registerPlugin(pluginMap, innerPluginName, innerPlugin, version); + }); + } else { + registerPlugin(pluginMap, name, moduleValue, version); + } +} + /** * Load all plugin modules available based on the manifest file at the provided base URL. * Plugins are loaded sequentially so that each plugin's exports are registered @@ -143,27 +209,14 @@ export async function loadModulePlugins( // eslint-disable-next-line no-await-in-loop const pluginExports = await loadModulePlugin(pluginMainUrl); - // Register the raw module exports in the resolve map so - // subsequent plugins can require() this plugin by its package name - if (packageName != null) { - resolve[packageName] = pluginExports; - log.debug(`Registered plugin '${packageName}' in module resolve map`); - } - - const moduleValue = getPluginModuleValue(pluginExports); - if (moduleValue == null) { - log.error(`Plugin '${name}' is missing an exported value.`); - } else if (isMultiPlugin(moduleValue)) { - // Flatten MultiPlugin: register each inner plugin by its own name - log.debug( - `MultiPlugin '${name}' contains ${moduleValue.plugins.length} plugins` - ); - moduleValue.plugins.forEach(innerPlugin => { - registerPlugin(pluginMap, innerPlugin.name, innerPlugin, version); - }); - } else { - registerPlugin(pluginMap, name, moduleValue, version); - } + processLoadedModule( + pluginMap, + resolve, + pluginExports, + name, + packageName, + version + ); } catch (e) { log.error(`Unable to load plugin '${name}'`, e); } From 366cc204a752a5a7738c2f77e5ebea259e0c689a Mon Sep 17 00:00:00 2001 From: Vlad Babich Date: Thu, 23 Apr 2026 19:34:16 -0600 Subject: [PATCH 14/25] Implement plugin dependecies, topological sorting --- .../app-utils/src/plugins/PluginUtils.test.ts | 358 +----------- .../app-utils/src/plugins/PluginUtils.tsx | 142 +---- packages/plugin/MIDDLEWARE_ARCHITECTURE.md | 17 +- packages/plugin/src/PluginUtils.test.tsx | 530 +++++++++++++++++- packages/plugin/src/PluginUtils.tsx | 254 +++++++++ 5 files changed, 805 insertions(+), 496 deletions(-) diff --git a/packages/app-utils/src/plugins/PluginUtils.test.ts b/packages/app-utils/src/plugins/PluginUtils.test.ts index 3aa15dffe5..943c430ad8 100644 --- a/packages/app-utils/src/plugins/PluginUtils.test.ts +++ b/packages/app-utils/src/plugins/PluginUtils.test.ts @@ -1,16 +1,5 @@ -import { - type LegacyPlugin, - type MultiPlugin, - type Plugin, - type PluginModuleMap, - PluginType, -} from '@deephaven/plugin'; -import { - getPluginModuleValue, - loadModulePlugins, - processLoadedModule, - registerPlugin, -} from './PluginUtils'; +import { type MultiPlugin, type Plugin, PluginType } from '@deephaven/plugin'; +import { loadModulePlugins } from './PluginUtils'; import { resolve } from './remote-component.config'; jest.mock('./loadRemoteModule', () => { @@ -27,349 +16,6 @@ const { default: loadRemoteModule } = require('./loadRemoteModule') as { default: jest.Mock; }; -describe('getPluginModuleValue', () => { - const legacyPlugins: [type: string, moduleValue: LegacyPlugin][] = [ - [ - 'dashboard', - { - DashboardPlugin: () => null, - }, - ], - [ - 'auth', - { - AuthPlugin: { - Component: () => null, - isAvailable: () => true, - }, - }, - ], - [ - 'table', - { - TablePlugin: () => null, - }, - ], - ]; - - const newPlugins: [type: string, moduleValue: Plugin][] = Object.keys( - PluginType - ).map(type => [type, { name: `${type}`, type: PluginType[type] }]); - - const newPluginsWithNamedExports: [ - type: string, - moduleValue: { default: Plugin; [key: string]: unknown }, - ][] = Object.keys(PluginType).map(type => [ - type, - { - default: { name: `${type}Plugin`, type: PluginType[type] }, - NamedExport: 'NamedExportValue', - }, - ]); - - const combinedPlugins: [ - type: string, - moduleValue: { - default: Plugin; - } & LegacyPlugin, - ][] = [ - [ - 'dashboard', - { - default: { - name: 'combinedFormat1', - type: PluginType.DASHBOARD_PLUGIN, - }, - DashboardPlugin: () => null, - }, - ], - [ - 'auth', - { - default: { - name: 'combinedFormat2', - type: PluginType.AUTH_PLUGIN, - }, - AuthPlugin: { - Component: () => null, - isAvailable: () => true, - }, - }, - ], - [ - 'table', - { - default: { - name: 'combinedFormat3', - type: PluginType.TABLE_PLUGIN, - }, - TablePlugin: () => null, - }, - ], - [ - // Should be able to combine different plugin types - 'multiple', - { - default: { - name: 'widgetPlugin', - type: PluginType.WIDGET_PLUGIN, - }, - DashboardPlugin: () => null, - }, - ], - ]; - - it.each(legacyPlugins)( - 'supports legacy %s plugin format', - (type, legacyPlugin) => { - const moduleValue = getPluginModuleValue(legacyPlugin); - expect(moduleValue).toBe(legacyPlugin); - } - ); - - it.each(newPlugins)('supports new %s format', (type, plugin) => { - const moduleValue = getPluginModuleValue(plugin); - expect(moduleValue).toBe(plugin); - }); - - it.each(newPluginsWithNamedExports)( - 'supports new %s format with named exports', - (type, plugin) => { - const moduleValue = getPluginModuleValue(plugin); - expect(moduleValue).toBe(plugin.default); - } - ); - - it.each(combinedPlugins)( - 'prioritizes new %s plugin if the module contains both legacy and new format', - (type, plugin) => { - const moduleValue = getPluginModuleValue(plugin); - expect(moduleValue).toBe(plugin.default); - } - ); - - it('returns null if the module value is not a plugin', () => { - const moduleValue = getPluginModuleValue({} as Plugin); - expect(moduleValue).toBeNull(); - }); - - describe('MultiPlugin', () => { - const multiPlugin: MultiPlugin = { - name: 'test-multi-plugin', - type: PluginType.MULTI_PLUGIN, - plugins: [ - { name: 'widget-plugin', type: PluginType.WIDGET_PLUGIN }, - { name: 'dashboard-plugin', type: PluginType.DASHBOARD_PLUGIN }, - { name: 'theme-plugin', type: PluginType.THEME_PLUGIN }, - ] as Plugin[], - }; - - it('supports MultiPlugin format', () => { - const moduleValue = getPluginModuleValue(multiPlugin); - expect(moduleValue).toBe(multiPlugin); - }); - - it('supports MultiPlugin with default export', () => { - const moduleWithDefault = { default: multiPlugin }; - const moduleValue = getPluginModuleValue(moduleWithDefault); - expect(moduleValue).toBe(multiPlugin); - }); - - it('supports MultiPlugin with named exports', () => { - const moduleWithNamedExports = { - default: multiPlugin, - SomeNamedExport: 'value', - }; - const moduleValue = getPluginModuleValue(moduleWithNamedExports); - expect(moduleValue).toBe(multiPlugin); - }); - }); -}); - -describe('registerPlugin', () => { - function makePlugin(name: string): Plugin { - return { name, type: PluginType.WIDGET_PLUGIN }; - } - - it('registers a plugin in the map', () => { - const pluginMap: PluginModuleMap = new Map(); - const plugin = makePlugin('my-plugin'); - - registerPlugin(pluginMap, 'my-plugin', plugin, '1.0.0'); - - expect(pluginMap.get('my-plugin')).toEqual({ ...plugin, version: '1.0.0' }); - }); - - it('skips duplicate and keeps the first registration', () => { - const pluginMap: PluginModuleMap = new Map(); - const first = makePlugin('my-plugin'); - const second = makePlugin('my-plugin'); - - registerPlugin(pluginMap, 'my-plugin', first, '1.0.0'); - registerPlugin(pluginMap, 'my-plugin', second, '2.0.0'); - - expect(pluginMap.size).toBe(1); - expect(pluginMap.get('my-plugin')).toEqual({ ...first, version: '1.0.0' }); - }); -}); - -describe('processLoadedModule', () => { - function makePlugin(name: string): Plugin { - return { name, type: PluginType.WIDGET_PLUGIN }; - } - - let pluginMap: PluginModuleMap; - let resolveMap: Record; - - beforeEach(() => { - pluginMap = new Map(); - resolveMap = {}; - }); - - it('registers a simple plugin', () => { - const plugin = makePlugin('my-plugin'); - processLoadedModule( - pluginMap, - resolveMap, - plugin, - 'my-plugin', - null, - '1.0.0' - ); - - expect(pluginMap.size).toBe(1); - expect(pluginMap.get('my-plugin')).toEqual({ ...plugin, version: '1.0.0' }); - }); - - it('registers a plugin with default export', () => { - const plugin = makePlugin('my-plugin'); - processLoadedModule( - pluginMap, - resolveMap, - { default: plugin }, - 'my-plugin' - ); - - expect(pluginMap.size).toBe(1); - expect(pluginMap.get('my-plugin')).toEqual({ - ...plugin, - version: undefined, - }); - }); - - it('registers in resolve map when packageName is provided', () => { - const plugin = makePlugin('my-plugin'); - const exports = { default: plugin }; - processLoadedModule( - pluginMap, - resolveMap, - exports, - 'my-plugin', - '@scope/my-plugin' - ); - - expect(resolveMap['@scope/my-plugin']).toBe(exports); - }); - - it('does not register in resolve map when packageName is null', () => { - const plugin = makePlugin('my-plugin'); - processLoadedModule(pluginMap, resolveMap, plugin, 'my-plugin', null); - - expect(Object.keys(resolveMap)).toHaveLength(0); - }); - - it('does not register in resolve map when packageName is undefined', () => { - const plugin = makePlugin('my-plugin'); - processLoadedModule(pluginMap, resolveMap, plugin, 'my-plugin'); - - expect(Object.keys(resolveMap)).toHaveLength(0); - }); - - it('flattens MultiPlugin and registers inner plugins', () => { - const multi: MultiPlugin = { - name: 'multi', - type: PluginType.MULTI_PLUGIN, - plugins: [makePlugin('inner-a'), makePlugin('inner-b')] as Plugin[], - }; - - processLoadedModule(pluginMap, resolveMap, multi, 'multi', null, '1.0.0'); - - expect(pluginMap.size).toBe(2); - expect(pluginMap.has('inner-a')).toBe(true); - expect(pluginMap.has('inner-b')).toBe(true); - expect(pluginMap.has('multi')).toBe(false); - }); - - it('skips invalid inner plugins in MultiPlugin', () => { - const multi: MultiPlugin = { - name: 'multi', - type: PluginType.MULTI_PLUGIN, - plugins: [ - makePlugin('valid'), - { notAPlugin: true } as unknown as Plugin, - ] as Plugin[], - }; - - processLoadedModule(pluginMap, resolveMap, multi, 'multi'); - - expect(pluginMap.size).toBe(1); - expect(pluginMap.has('valid')).toBe(true); - }); - - it('skips inner plugins with empty names in MultiPlugin', () => { - const multi: MultiPlugin = { - name: 'multi', - type: PluginType.MULTI_PLUGIN, - plugins: [ - makePlugin('valid'), - { name: '', type: PluginType.WIDGET_PLUGIN } as Plugin, - { name: ' ', type: PluginType.WIDGET_PLUGIN } as Plugin, - ] as Plugin[], - }; - - processLoadedModule(pluginMap, resolveMap, multi, 'multi'); - - expect(pluginMap.size).toBe(1); - expect(pluginMap.has('valid')).toBe(true); - }); - - it('skips duplicate inner plugins in MultiPlugin', () => { - const multi: MultiPlugin = { - name: 'multi', - type: PluginType.MULTI_PLUGIN, - plugins: [makePlugin('dupe'), makePlugin('dupe')] as Plugin[], - }; - - processLoadedModule(pluginMap, resolveMap, multi, 'multi'); - - expect(pluginMap.size).toBe(1); - }); - - it('does not register when module value is null', () => { - processLoadedModule(pluginMap, resolveMap, {} as Plugin, 'bad-plugin'); - - expect(pluginMap.size).toBe(0); - }); - - it('handles legacy plugin format', () => { - const legacy: LegacyPlugin = { TablePlugin: () => null }; - processLoadedModule( - pluginMap, - resolveMap, - legacy, - 'legacy-plugin', - null, - '1.0.0' - ); - - expect(pluginMap.size).toBe(1); - expect(pluginMap.get('legacy-plugin')).toEqual({ - ...legacy, - version: '1.0.0', - }); - }); -}); - describe('loadModulePlugins', () => { const BASE_URL = 'http://localhost:4100/plugins'; diff --git a/packages/app-utils/src/plugins/PluginUtils.tsx b/packages/app-utils/src/plugins/PluginUtils.tsx index 5e9fb317c9..9eb2e328b6 100644 --- a/packages/app-utils/src/plugins/PluginUtils.tsx +++ b/packages/app-utils/src/plugins/PluginUtils.tsx @@ -9,31 +9,15 @@ import { type Plugin, PluginType, isLegacyAuthPlugin, - isLegacyPlugin, - type PluginModule, - isPlugin, - isMultiPlugin, + processLoadedModule, + sortPluginsByDependency, + type PluginManifest, } from '@deephaven/plugin'; import loadRemoteModule from './loadRemoteModule'; import { resolve } from './remote-component.config'; const log = Log.module('@deephaven/app-utils.PluginUtils'); -export type PluginManifestPluginInfo = { - name: string; - main: string; - /** - * The npm package name for this plugin (e.g. `@deephaven/js-plugin-pivot`). - * When provided, the plugin's exports are registered in the module resolve - * map under this key, enabling other plugins to `require()` this plugin at - * runtime. If omitted, the plugin is not registered for cross-plugin imports. - */ - package?: string; - version: string; -}; - -export type PluginManifest = { plugins: PluginManifestPluginInfo[] }; - /** * Imports a commonjs plugin module from the provided URL * @param pluginUrl The URL of the plugin to load @@ -63,120 +47,6 @@ export async function loadJson(jsonUrl: string): Promise { } } -function hasDefaultExport(value: unknown): value is { default: Plugin } { - return ( - typeof value === 'object' && - value != null && - typeof (value as { default?: unknown }).default === 'object' - ); -} - -export function getPluginModuleValue( - value: LegacyPlugin | Plugin | { default: Plugin } -): PluginModule | null { - // TypeScript builds CJS default exports differently depending on - // whether there are also named exports. If the default is the only - // export, it will be the value. If there are also named exports, - // it will be assigned to the `default` property on the value. - if (isPlugin(value)) { - return value; - } - if (hasDefaultExport(value) && isPlugin(value.default)) { - return value.default; - } - if (isLegacyPlugin(value)) { - return value; - } - return null; -} - -/** - * Register a plugin in the plugin map. If a plugin with the same name is - * already registered, the duplicate is skipped and a warning is logged. - * @param pluginMap The plugin map to register the plugin in - * @param name The name to register the plugin under - * @param plugin The plugin to register - * @param version Optional version to attach to the plugin - */ -export function registerPlugin( - pluginMap: PluginModuleMap, - name: string, - plugin: PluginModule, - version?: string -): void { - if (pluginMap.has(name)) { - log.warn(`Plugin '${name}' is already registered. Skipping duplicate.`); - return; - } - pluginMap.set(name, { ...plugin, version }); -} - -/** - * Process a loaded plugin module: extract the plugin value, register in the - * resolve map for cross-plugin imports, flatten MultiPlugins, and register - * each resulting plugin in the plugin map. - * - * @param pluginMap The plugin map to register plugins in - * @param resolveMap The module resolve map for cross-plugin imports - * @param pluginExports The raw module exports from loading the plugin - * @param name The manifest name of the plugin - * @param packageName Optional package name for resolve map registration - * @param version Optional version to attach to registered plugins - */ -export function processLoadedModule( - pluginMap: PluginModuleMap, - resolveMap: Record, - pluginExports: unknown, - name: string, - packageName?: string | null, - version?: string -): void { - // Register raw module exports in the resolve map so subsequent - // plugins can import this plugin by its package name - if (packageName != null) { - resolveMap[packageName] = pluginExports; - log.debug(`Registered plugin '${packageName}' in module resolve map`); - } - - const moduleValue = getPluginModuleValue( - pluginExports as LegacyPlugin | Plugin | { default: Plugin } - ); - if (moduleValue == null) { - log.error(`Plugin '${name}' is missing an exported value.`); - return; - } - - if (isMultiPlugin(moduleValue)) { - log.debug( - `MultiPlugin '${name}' contains ${moduleValue.plugins.length} plugins` - ); - moduleValue.plugins.forEach(innerPlugin => { - if (!isPlugin(innerPlugin)) { - log.warn( - `Skipping invalid inner plugin from MultiPlugin '${name}'`, - innerPlugin - ); - return; - } - - const innerPluginName = - typeof innerPlugin.name === 'string' ? innerPlugin.name.trim() : ''; - - if (innerPluginName.length === 0) { - log.warn( - `Skipping unnamed inner plugin from MultiPlugin '${name}'`, - innerPlugin - ); - return; - } - - registerPlugin(pluginMap, innerPluginName, innerPlugin, version); - }); - } else { - registerPlugin(pluginMap, name, moduleValue, version); - } -} - /** * Load all plugin modules available based on the manifest file at the provided base URL. * Plugins are loaded sequentially so that each plugin's exports are registered @@ -198,12 +68,14 @@ export async function loadModulePlugins( log.debug('Plugin manifest loaded:', manifest); + const sortedPlugins = sortPluginsByDependency(manifest.plugins); + const pluginMap: PluginModuleMap = new Map(); // Load plugins sequentially so each plugin's exports are available // to subsequently loaded plugins via require() - for (let i = 0; i < manifest.plugins.length; i += 1) { - const { name, main, version, package: packageName } = manifest.plugins[i]; + for (let i = 0; i < sortedPlugins.length; i += 1) { + const { name, main, version, package: packageName } = sortedPlugins[i]; const pluginMainUrl = `${modulePluginsUrl}/${name}/${main}`; try { // eslint-disable-next-line no-await-in-loop diff --git a/packages/plugin/MIDDLEWARE_ARCHITECTURE.md b/packages/plugin/MIDDLEWARE_ARCHITECTURE.md index fc37e3ace8..59259c51be 100644 --- a/packages/plugin/MIDDLEWARE_ARCHITECTURE.md +++ b/packages/plugin/MIDDLEWARE_ARCHITECTURE.md @@ -220,11 +220,22 @@ Here `pivot` is importable (has `package`); `grid-toolbar` only consumes. declare module '@deephaven/js-plugin-pivot' { export class IrisGridPivotModel {} } ``` -4. **Ensure load order** — the dependency must appear before the consumer in `manifest.json`. +4. **Declare dependencies** in `manifest.json`: + ```json + { + "name": "grid-toolbar", + "main": "src/js/dist/bundle/index.js", + "version": "0.0.0", + "dependencies": ["@deephaven/js-plugin-pivot"] + } + ``` + The loader topologically sorts plugins so dependencies load first. If + `dependencies` is omitted, manifest order is preserved. ### Rules -- A plugin can only import plugins loaded earlier in the manifest. - **TODO**: add `dependencies` field to manifest and verify load order at runtime. +- Plugins are topologically sorted by their `dependencies` before loading. + Circular dependencies throw an error at load time. - Only plugins with a `package` field are registered in the resolve map. - The `package` value must exactly match the `import` string. +- Dependency values in `dependencies` must match a `package` field of another plugin. diff --git a/packages/plugin/src/PluginUtils.test.tsx b/packages/plugin/src/PluginUtils.test.tsx index 0d9f6afef2..e8d03a9271 100644 --- a/packages/plugin/src/PluginUtils.test.tsx +++ b/packages/plugin/src/PluginUtils.test.tsx @@ -5,7 +5,11 @@ import { dhTruck, vsPreview } from '@deephaven/icons'; import { type DashboardPlugin, type ElementPlugin, + type LegacyPlugin, + type MultiPlugin, + type Plugin, type PluginModule, + type PluginModuleMap, PluginType, type ThemePlugin, type WidgetPlugin, @@ -15,6 +19,11 @@ import { getIconForPlugin, getThemeDataFromPlugins, getPluginsElementMap, + getPluginModuleValue, + processLoadedModule, + registerPlugin, + sortPluginsByDependency, + type PluginManifestPluginInfo, } from './PluginUtils'; function TestWidget() { @@ -208,13 +217,530 @@ describe('getElementPluginMap', () => { }); it('should return an empty map if no element plugins are present', () => { - const pluginMap = new Map([ + const pluginMap2 = new Map([ [widgetPlugin.name, widgetPlugin], [dashboardPlugin.name, dashboardPlugin], ]); - const elementMapping = getPluginsElementMap(pluginMap); + const elementMapping = getPluginsElementMap(pluginMap2); expect(elementMapping.size).toBe(0); }); }); + +describe('getPluginModuleValue', () => { + const legacyPlugins: [type: string, moduleValue: LegacyPlugin][] = [ + [ + 'dashboard', + { + DashboardPlugin: () => null, + }, + ], + [ + 'auth', + { + AuthPlugin: { + Component: () => null, + isAvailable: () => true, + }, + }, + ], + [ + 'table', + { + TablePlugin: () => null, + }, + ], + ]; + + const newPlugins: [type: string, moduleValue: Plugin][] = Object.keys( + PluginType + ).map(type => [type, { name: `${type}`, type: PluginType[type] }]); + + const newPluginsWithNamedExports: [ + type: string, + moduleValue: { default: Plugin; [key: string]: unknown }, + ][] = Object.keys(PluginType).map(type => [ + type, + { + default: { name: `${type}Plugin`, type: PluginType[type] }, + NamedExport: 'NamedExportValue', + }, + ]); + + const combinedPlugins: [ + type: string, + moduleValue: { + default: Plugin; + } & LegacyPlugin, + ][] = [ + [ + 'dashboard', + { + default: { + name: 'combinedFormat1', + type: PluginType.DASHBOARD_PLUGIN, + }, + DashboardPlugin: () => null, + }, + ], + [ + 'auth', + { + default: { + name: 'combinedFormat2', + type: PluginType.AUTH_PLUGIN, + }, + AuthPlugin: { + Component: () => null, + isAvailable: () => true, + }, + }, + ], + [ + 'table', + { + default: { + name: 'combinedFormat3', + type: PluginType.TABLE_PLUGIN, + }, + TablePlugin: () => null, + }, + ], + [ + // Should be able to combine different plugin types + 'multiple', + { + default: { + name: 'widgetPlugin2', + type: PluginType.WIDGET_PLUGIN, + }, + DashboardPlugin: () => null, + }, + ], + ]; + + it.each(legacyPlugins)( + 'supports legacy %s plugin format', + (type, legacyPlugin) => { + const moduleValue = getPluginModuleValue(legacyPlugin); + expect(moduleValue).toBe(legacyPlugin); + } + ); + + it.each(newPlugins)('supports new %s format', (type, plugin) => { + const moduleValue = getPluginModuleValue(plugin); + expect(moduleValue).toBe(plugin); + }); + + it.each(newPluginsWithNamedExports)( + 'supports new %s format with named exports', + (type, plugin) => { + const moduleValue = getPluginModuleValue(plugin); + expect(moduleValue).toBe(plugin.default); + } + ); + + it.each(combinedPlugins)( + 'prioritizes new %s plugin if the module contains both legacy and new format', + (type, plugin) => { + const moduleValue = getPluginModuleValue(plugin); + expect(moduleValue).toBe(plugin.default); + } + ); + + it('returns null if the module value is not a plugin', () => { + const moduleValue = getPluginModuleValue({} as Plugin); + expect(moduleValue).toBeNull(); + }); + + describe('MultiPlugin', () => { + const multiPlugin: MultiPlugin = { + name: 'test-multi-plugin', + type: PluginType.MULTI_PLUGIN, + plugins: [ + { name: 'widget-plugin', type: PluginType.WIDGET_PLUGIN }, + { name: 'dashboard-plugin2', type: PluginType.DASHBOARD_PLUGIN }, + { name: 'theme-plugin', type: PluginType.THEME_PLUGIN }, + ] as Plugin[], + }; + + it('supports MultiPlugin format', () => { + const moduleValue = getPluginModuleValue(multiPlugin); + expect(moduleValue).toBe(multiPlugin); + }); + + it('supports MultiPlugin with default export', () => { + const moduleWithDefault = { default: multiPlugin }; + const moduleValue = getPluginModuleValue(moduleWithDefault); + expect(moduleValue).toBe(multiPlugin); + }); + + it('supports MultiPlugin with named exports', () => { + const moduleWithNamedExports = { + default: multiPlugin, + SomeNamedExport: 'value', + }; + const moduleValue = getPluginModuleValue(moduleWithNamedExports); + expect(moduleValue).toBe(multiPlugin); + }); + }); +}); + +describe('registerPlugin', () => { + function makePlugin(name: string): Plugin { + return { name, type: PluginType.WIDGET_PLUGIN }; + } + + it('registers a plugin in the map', () => { + const regPluginMap: PluginModuleMap = new Map(); + const plugin = makePlugin('my-plugin'); + + registerPlugin(regPluginMap, 'my-plugin', plugin, '1.0.0'); + + expect(regPluginMap.get('my-plugin')).toEqual({ + ...plugin, + version: '1.0.0', + }); + }); + + it('skips duplicate and keeps the first registration', () => { + const regPluginMap: PluginModuleMap = new Map(); + const first = makePlugin('my-plugin'); + const second = makePlugin('my-plugin'); + + registerPlugin(regPluginMap, 'my-plugin', first, '1.0.0'); + registerPlugin(regPluginMap, 'my-plugin', second, '2.0.0'); + + expect(regPluginMap.size).toBe(1); + expect(regPluginMap.get('my-plugin')).toEqual({ + ...first, + version: '1.0.0', + }); + }); +}); + +describe('processLoadedModule', () => { + function makePlugin(name: string): Plugin { + return { name, type: PluginType.WIDGET_PLUGIN }; + } + + let procPluginMap: PluginModuleMap; + let resolveMap: Record; + + beforeEach(() => { + procPluginMap = new Map(); + resolveMap = {}; + }); + + it('registers a simple plugin', () => { + const plugin = makePlugin('my-plugin'); + processLoadedModule( + procPluginMap, + resolveMap, + plugin, + 'my-plugin', + null, + '1.0.0' + ); + + expect(procPluginMap.size).toBe(1); + expect(procPluginMap.get('my-plugin')).toEqual({ + ...plugin, + version: '1.0.0', + }); + }); + + it('registers a plugin with default export', () => { + const plugin = makePlugin('my-plugin'); + processLoadedModule( + procPluginMap, + resolveMap, + { default: plugin }, + 'my-plugin' + ); + + expect(procPluginMap.size).toBe(1); + expect(procPluginMap.get('my-plugin')).toEqual({ + ...plugin, + version: undefined, + }); + }); + + it('registers in resolve map when packageName is provided', () => { + const plugin = makePlugin('my-plugin'); + const pluginExports = { default: plugin }; + processLoadedModule( + procPluginMap, + resolveMap, + pluginExports, + 'my-plugin', + '@scope/my-plugin' + ); + + expect(resolveMap['@scope/my-plugin']).toBe(pluginExports); + }); + + it('does not register in resolve map when packageName is null', () => { + const plugin = makePlugin('my-plugin'); + processLoadedModule(procPluginMap, resolveMap, plugin, 'my-plugin', null); + + expect(Object.keys(resolveMap)).toHaveLength(0); + }); + + it('does not register in resolve map when packageName is undefined', () => { + const plugin = makePlugin('my-plugin'); + processLoadedModule(procPluginMap, resolveMap, plugin, 'my-plugin'); + + expect(Object.keys(resolveMap)).toHaveLength(0); + }); + + it('flattens MultiPlugin and registers inner plugins', () => { + const multi: MultiPlugin = { + name: 'multi', + type: PluginType.MULTI_PLUGIN, + plugins: [makePlugin('inner-a'), makePlugin('inner-b')] as Plugin[], + }; + + processLoadedModule( + procPluginMap, + resolveMap, + multi, + 'multi', + null, + '1.0.0' + ); + + expect(procPluginMap.size).toBe(2); + expect(procPluginMap.has('inner-a')).toBe(true); + expect(procPluginMap.has('inner-b')).toBe(true); + expect(procPluginMap.has('multi')).toBe(false); + }); + + it('skips invalid inner plugins in MultiPlugin', () => { + const multi: MultiPlugin = { + name: 'multi', + type: PluginType.MULTI_PLUGIN, + plugins: [ + makePlugin('valid'), + { notAPlugin: true } as unknown as Plugin, + ] as Plugin[], + }; + + processLoadedModule(procPluginMap, resolveMap, multi, 'multi'); + + expect(procPluginMap.size).toBe(1); + expect(procPluginMap.has('valid')).toBe(true); + }); + + it('skips inner plugins with empty names in MultiPlugin', () => { + const multi: MultiPlugin = { + name: 'multi', + type: PluginType.MULTI_PLUGIN, + plugins: [ + makePlugin('valid'), + { name: '', type: PluginType.WIDGET_PLUGIN } as Plugin, + { name: ' ', type: PluginType.WIDGET_PLUGIN } as Plugin, + ] as Plugin[], + }; + + processLoadedModule(procPluginMap, resolveMap, multi, 'multi'); + + expect(procPluginMap.size).toBe(1); + expect(procPluginMap.has('valid')).toBe(true); + }); + + it('skips duplicate inner plugins in MultiPlugin', () => { + const multi: MultiPlugin = { + name: 'multi', + type: PluginType.MULTI_PLUGIN, + plugins: [makePlugin('dupe'), makePlugin('dupe')] as Plugin[], + }; + + processLoadedModule(procPluginMap, resolveMap, multi, 'multi'); + + expect(procPluginMap.size).toBe(1); + }); + + it('does not register when module value is null', () => { + processLoadedModule(procPluginMap, resolveMap, {} as Plugin, 'bad-plugin'); + + expect(procPluginMap.size).toBe(0); + }); + + it('handles legacy plugin format', () => { + const legacy: LegacyPlugin = { TablePlugin: () => null }; + processLoadedModule( + procPluginMap, + resolveMap, + legacy, + 'legacy-plugin', + null, + '1.0.0' + ); + + expect(procPluginMap.size).toBe(1); + expect(procPluginMap.get('legacy-plugin')).toEqual({ + ...legacy, + version: '1.0.0', + }); + }); +}); + +describe('sortPluginsByDependency', () => { + function makeManifestPlugin( + name: string, + opts?: { + package?: string; + dependencies?: string[]; + } + ): PluginManifestPluginInfo { + return { + name, + main: 'index.js', + version: '1.0.0', + package: opts?.package, + dependencies: opts?.dependencies, + }; + } + + it('returns plugins in original order when no dependencies', () => { + const plugins = [ + makeManifestPlugin('a'), + makeManifestPlugin('b'), + makeManifestPlugin('c'), + ]; + + const sorted = sortPluginsByDependency(plugins); + + expect(sorted.map(p => p.name)).toEqual(['a', 'b', 'c']); + }); + + it('reorders so dependency loads before consumer', () => { + const plugins = [ + makeManifestPlugin('consumer', { + dependencies: ['@scope/dep'], + }), + makeManifestPlugin('dep', { + package: '@scope/dep', + }), + ]; + + const sorted = sortPluginsByDependency(plugins); + + expect(sorted.map(p => p.name)).toEqual(['dep', 'consumer']); + }); + + it('handles a chain of dependencies: a → b → c', () => { + const plugins = [ + makeManifestPlugin('c', { + package: '@scope/c', + dependencies: ['@scope/b'], + }), + makeManifestPlugin('b', { + package: '@scope/b', + dependencies: ['@scope/a'], + }), + makeManifestPlugin('a', { + package: '@scope/a', + }), + ]; + + const sorted = sortPluginsByDependency(plugins); + + const names = sorted.map(p => p.name); + expect(names.indexOf('a')).toBeLessThan(names.indexOf('b')); + expect(names.indexOf('b')).toBeLessThan(names.indexOf('c')); + }); + + it('preserves original order among independent plugins', () => { + const plugins = [ + makeManifestPlugin('x'), + makeManifestPlugin('dep', { package: '@scope/dep' }), + makeManifestPlugin('y'), + makeManifestPlugin('consumer', { dependencies: ['@scope/dep'] }), + makeManifestPlugin('z'), + ]; + + const sorted = sortPluginsByDependency(plugins); + + const names = sorted.map(p => p.name); + // dep must come before consumer + expect(names.indexOf('dep')).toBeLessThan(names.indexOf('consumer')); + // independent plugins keep their relative order + expect(names.indexOf('x')).toBeLessThan(names.indexOf('y')); + expect(names.indexOf('y')).toBeLessThan(names.indexOf('z')); + }); + + it('throws on circular dependencies', () => { + const plugins = [ + makeManifestPlugin('a', { + package: '@scope/a', + dependencies: ['@scope/b'], + }), + makeManifestPlugin('b', { + package: '@scope/b', + dependencies: ['@scope/a'], + }), + ]; + + expect(() => sortPluginsByDependency(plugins)).toThrow( + /Circular plugin dependency/ + ); + }); + + it('warns and ignores dependencies not in the manifest', () => { + const plugins = [ + makeManifestPlugin('consumer', { + dependencies: ['@scope/nonexistent'], + }), + ]; + + const sorted = sortPluginsByDependency(plugins); + + expect(sorted.map(p => p.name)).toEqual(['consumer']); + }); + + it('handles multiple dependencies', () => { + const plugins = [ + makeManifestPlugin('consumer', { + dependencies: ['@scope/dep-a', '@scope/dep-b'], + }), + makeManifestPlugin('dep-a', { package: '@scope/dep-a' }), + makeManifestPlugin('dep-b', { package: '@scope/dep-b' }), + ]; + + const sorted = sortPluginsByDependency(plugins); + + const names = sorted.map(p => p.name); + expect(names.indexOf('dep-a')).toBeLessThan(names.indexOf('consumer')); + expect(names.indexOf('dep-b')).toBeLessThan(names.indexOf('consumer')); + }); + + it('does not mutate the input array', () => { + const plugins = [ + makeManifestPlugin('consumer', { dependencies: ['@scope/dep'] }), + makeManifestPlugin('dep', { package: '@scope/dep' }), + ]; + const original = [...plugins]; + + sortPluginsByDependency(plugins); + + expect(plugins).toEqual(original); + }); + + it('handles empty plugin list', () => { + expect(sortPluginsByDependency([])).toEqual([]); + }); + + it('handles plugins with empty dependencies array', () => { + const plugins = [ + makeManifestPlugin('a', { dependencies: [] }), + makeManifestPlugin('b'), + ]; + + const sorted = sortPluginsByDependency(plugins); + + expect(sorted.map(p => p.name)).toEqual(['a', 'b']); + }); +}); diff --git a/packages/plugin/src/PluginUtils.tsx b/packages/plugin/src/PluginUtils.tsx index ad24ef50ad..186560d4f2 100644 --- a/packages/plugin/src/PluginUtils.tsx +++ b/packages/plugin/src/PluginUtils.tsx @@ -16,6 +16,11 @@ import { type WidgetComponentProps, type WidgetPanelProps, type WidgetMiddlewarePanelProps, + isLegacyPlugin, + isMultiPlugin, + isPlugin, + type LegacyPlugin, + type Plugin, } from './PluginTypes'; const log = Log.module('@deephaven/plugin.PluginUtils'); @@ -202,3 +207,252 @@ export function createChainedPanelComponent( basePanelComponent ); } + +export type PluginManifestPluginInfo = { + name: string; + main: string; + /** + * The npm package name for this plugin (e.g. `@deephaven/js-plugin-pivot`). + * When provided, the plugin's exports are registered in the module resolve + * map under this key, enabling other plugins to `require()` this plugin at + * runtime. If omitted, the plugin is not registered for cross-plugin imports. + */ + package?: string; + /** + * Package names this plugin depends on at runtime. These must match the + * `package` field of other plugins in the manifest. Plugins are + * topologically sorted so that dependencies are loaded first. If omitted, + * the plugin has no cross-plugin dependencies. + */ + dependencies?: string[]; + version: string; +}; + +export type PluginManifest = { plugins: PluginManifestPluginInfo[] }; + +/** + * Topologically sort plugins so that dependencies are loaded before the + * plugins that depend on them. Plugins without dependencies or whose + * dependencies are not in the manifest keep their original relative order + * (stable sort). Throws if a dependency cycle is detected. + * + * @param plugins The plugin list from the manifest + * @returns A new array with plugins sorted so dependencies come first + */ +export function sortPluginsByDependency< + T extends Pick, +>(plugins: readonly T[]): T[] { + // Build a lookup from package name → plugin index + const packageToIndex = new Map(); + plugins.forEach((p, i) => { + if (p.package != null) { + packageToIndex.set(p.package, i); + } + }); + + // Build adjacency list: index → indices it depends on + const depIndices = new Map(); + plugins.forEach((p, i) => { + if (p.dependencies != null && p.dependencies.length > 0) { + const resolved: number[] = []; + p.dependencies.forEach(dep => { + const idx = packageToIndex.get(dep); + if (idx != null) { + resolved.push(idx); + } else { + log.warn( + `Plugin '${p.name}' depends on '${dep}' which is not in the manifest` + ); + } + }); + if (resolved.length > 0) { + depIndices.set(i, resolved); + } + } + }); + + // If no plugin has in-manifest dependencies, return original order + if (depIndices.size === 0) { + return [...plugins]; + } + + // Kahn's algorithm for topological sort (stable — preserves original order + // among plugins at the same dependency depth) + const inDegree = new Array(plugins.length).fill(0); + + // Reverse adjacency: who depends on me? + const dependents = new Map(); + depIndices.forEach((deps, idx) => { + deps.forEach(dep => { + if (!dependents.has(dep)) { + dependents.set(dep, []); + } + const depList = dependents.get(dep); + if (depList != null) { + depList.push(idx); + } + inDegree[idx] += 1; + }); + }); + + // Seed queue with all nodes that have no in-manifest dependencies, + // in their original order + const queue: number[] = []; + for (let i = 0; i < plugins.length; i += 1) { + if (inDegree[i] === 0) { + queue.push(i); + } + } + + const sorted: T[] = []; + while (queue.length > 0) { + const idx = queue.shift(); + if (idx == null) { + break; + } + sorted.push(plugins[idx]); + const deps = dependents.get(idx); + if (deps != null) { + // Process dependents in original manifest order for stability + deps.sort((a, b) => a - b); + deps.forEach(depIdx => { + inDegree[depIdx] -= 1; + if (inDegree[depIdx] === 0) { + queue.push(depIdx); + } + }); + } + } + + if (sorted.length !== plugins.length) { + // Find the cycle participants for a useful error message + const inCycle = plugins.filter((_, i) => inDegree[i] > 0).map(p => p.name); + throw new Error( + `Circular plugin dependency detected among: ${inCycle.join(', ')}` + ); + } + + return sorted; +} + +function hasDefaultExport(value: unknown): value is { default: Plugin } { + return ( + typeof value === 'object' && + value != null && + typeof (value as { default?: unknown }).default === 'object' + ); +} + +/** + * Get the PluginModule value from a loaded module export. Handles: + * - Direct Plugin exports (`export default myPlugin`) + * - Named default exports (`{ default: myPlugin, ...namedExports }`) + * - Legacy plugin formats (`{ DashboardPlugin, AuthPlugin, TablePlugin }`) + * + * TypeScript builds CJS default exports differently depending on whether + * there are also named exports. If the default is the only export, it will + * be the value. If there are also named exports, it will be assigned to + * the `default` property on the value. + */ +export function getPluginModuleValue( + value: LegacyPlugin | Plugin | { default: Plugin } +): PluginModule | null { + if (isPlugin(value)) { + return value; + } + if (hasDefaultExport(value) && isPlugin(value.default)) { + return value.default; + } + if (isLegacyPlugin(value)) { + return value; + } + return null; +} + +/** + * Register a plugin in the plugin map. If a plugin with the same name is + * already registered, the duplicate is skipped and a warning is logged. + * @param pluginMap The plugin map to register the plugin in + * @param name The name to register the plugin under + * @param plugin The plugin to register + * @param version Optional version to attach to the plugin + */ +export function registerPlugin( + pluginMap: PluginModuleMap, + name: string, + plugin: PluginModule, + version?: string +): void { + if (pluginMap.has(name)) { + log.warn(`Plugin '${name}' is already registered. Skipping duplicate.`); + return; + } + pluginMap.set(name, { ...plugin, version }); +} + +/** + * Process a loaded plugin module: extract the plugin value, register in the + * resolve map for cross-plugin imports, flatten MultiPlugins, and register + * each resulting plugin in the plugin map. + * + * @param pluginMap The plugin map to register plugins in + * @param resolveMap The module resolve map for cross-plugin imports + * @param pluginExports The raw module exports from loading the plugin + * @param name The manifest name of the plugin + * @param packageName Optional package name for resolve map registration + * @param version Optional version to attach to registered plugins + */ +export function processLoadedModule( + pluginMap: PluginModuleMap, + resolveMap: Record, + pluginExports: unknown, + name: string, + packageName?: string | null, + version?: string +): void { + // Register raw module exports in the resolve map so subsequent + // plugins can import this plugin by its package name + if (packageName != null) { + // eslint-disable-next-line no-param-reassign + resolveMap[packageName] = pluginExports; + log.debug(`Registered plugin '${packageName}' in module resolve map`); + } + + const moduleValue = getPluginModuleValue( + pluginExports as LegacyPlugin | Plugin | { default: Plugin } + ); + if (moduleValue == null) { + log.error(`Plugin '${name}' is missing an exported value.`); + return; + } + + if (isMultiPlugin(moduleValue)) { + log.debug( + `MultiPlugin '${name}' contains ${moduleValue.plugins.length} plugins` + ); + moduleValue.plugins.forEach(innerPlugin => { + if (!isPlugin(innerPlugin)) { + log.warn( + `Skipping invalid inner plugin from MultiPlugin '${name}'`, + innerPlugin + ); + return; + } + + const innerPluginName = + typeof innerPlugin.name === 'string' ? innerPlugin.name.trim() : ''; + + if (innerPluginName.length === 0) { + log.warn( + `Skipping unnamed inner plugin from MultiPlugin '${name}'`, + innerPlugin + ); + return; + } + + registerPlugin(pluginMap, innerPluginName, innerPlugin, version); + }); + } else { + registerPlugin(pluginMap, name, moduleValue, version); + } +} From 4c735d8e02d9c0f03d91f2af87d0a2b7a03f59a5 Mon Sep 17 00:00:00 2001 From: Vlad Babich Date: Thu, 23 Apr 2026 19:55:24 -0600 Subject: [PATCH 15/25] Update doc, comments --- .../app-utils/src/plugins/PluginUtils.tsx | 6 ++-- .../middleware-architecture.md} | 33 ++++++++++++------- packages/plugin/src/PluginUtils.tsx | 4 +-- 3 files changed, 26 insertions(+), 17 deletions(-) rename packages/plugin/{MIDDLEWARE_ARCHITECTURE.md => docs/middleware-architecture.md} (82%) diff --git a/packages/app-utils/src/plugins/PluginUtils.tsx b/packages/app-utils/src/plugins/PluginUtils.tsx index 9eb2e328b6..7adeaefaf7 100644 --- a/packages/app-utils/src/plugins/PluginUtils.tsx +++ b/packages/app-utils/src/plugins/PluginUtils.tsx @@ -50,8 +50,8 @@ export async function loadJson(jsonUrl: string): Promise { /** * Load all plugin modules available based on the manifest file at the provided base URL. * Plugins are loaded sequentially so that each plugin's exports are registered - * in the module resolve map before subsequent plugins load. This allows plugins - * to depend on other plugins via standard require() calls. + * in the module resolve map before subsequent plugins load. This enables + * cross-plugin imports via standard import statements. * @param modulePluginsUrl The base URL of the module plugins to load * @returns A map from the name of the plugin to the plugin module that was loaded */ @@ -73,7 +73,7 @@ export async function loadModulePlugins( const pluginMap: PluginModuleMap = new Map(); // Load plugins sequentially so each plugin's exports are available - // to subsequently loaded plugins via require() + // to subsequently loaded plugins via import for (let i = 0; i < sortedPlugins.length; i += 1) { const { name, main, version, package: packageName } = sortedPlugins[i]; const pluginMainUrl = `${modulePluginsUrl}/${name}/${main}`; diff --git a/packages/plugin/MIDDLEWARE_ARCHITECTURE.md b/packages/plugin/docs/middleware-architecture.md similarity index 82% rename from packages/plugin/MIDDLEWARE_ARCHITECTURE.md rename to packages/plugin/docs/middleware-architecture.md index 59259c51be..043eb84b43 100644 --- a/packages/plugin/MIDDLEWARE_ARCHITECTURE.md +++ b/packages/plugin/docs/middleware-architecture.md @@ -50,6 +50,15 @@ interface WidgetMiddlewareComponentProps extends WidgetComponentPro } ``` +### `WidgetMiddlewarePanelProps` + +```tsx +interface WidgetMiddlewarePanelProps extends WidgetPanelProps { + // The next panel component in the chain — render this to continue + Component: React.ComponentType>; +} +``` + ## Rendering Paths The middleware is applied in two different contexts: @@ -91,15 +100,15 @@ const myToolbarPlugin = { ### Intercepting Props ```tsx -function PropsInterceptor({ Component, ...props }: WidgetMiddlewareComponentProps) { +function PropsInterceptor({ Component, fetch, ...rest }: WidgetMiddlewareComponentProps) { // Modify or augment props before passing them to the wrapped component const enhancedFetch = useCallback(async () => { - const widget = await props.fetch(); + const widget = await fetch(); // Transform or cache the widget data return widget; - }, [props.fetch]); + }, [fetch]); - return ; + return ; } ``` @@ -169,31 +178,31 @@ const plugins = new Map([ 2. **Last base plugin wins.** If multiple non-middleware plugins register for the same type, the last one replaces earlier ones (with a warning). 3. **Middleware must render `Component`.** If a middleware doesn't render the `Component` prop, the rest of the chain (including the base widget) will not appear. 4. **Middleware must spread props.** Pass all received props to `Component` to ensure the base widget and other middleware receive them. -5. **`panelComponent` middleware is separate.** If the base plugin defines a `panelComponent`, middleware targeting the panel layer must also define `panelComponent`. +5. **`panelComponent` middleware is separate.** When the base plugin defines a `panelComponent`, only middleware that also defines `panelComponent` is applied. Middleware with only `component` is silently skipped in the panel path — it will have no effect for that widget type. ## Cross-Plugin Dependencies -Plugins load sequentially in manifest order. A plugin can expose its exports for later plugins to import at runtime by declaring a `package` field in the manifest. +Plugins load sequentially in dependency order (topologically sorted by the `dependencies` field, with manifest order preserved among independent plugins). A plugin can expose its exports for later plugins to import at runtime by declaring a `package` field in the manifest. ``` -1. pivot loads → resolve["@deephaven/js-plugin-pivot"] = exports +1. pivot loads → exports registered under "@deephaven/js-plugin-pivot" 2. grid-toolbar loads → import "@deephaven/js-plugin-pivot" ✓ ``` ### Optional Manifest `package` Field -When present, the plugin's exports are registered in the resolve map under this key. +When present, the plugin's exports are made available to other plugins under this key, so they can be imported via standard `import` statements. ```json { "plugins": [ - { "name": "pivot", "main": "src/js/dist/index.js", "package": "@deephaven/js-plugin-pivot" }, - { "name": "grid-toolbar", "main": "src/js/dist/bundle/index.js" } + { "name": "pivot", "main": "src/js/dist/index.js", "version": "0.0.0", "package": "@deephaven/js-plugin-pivot" }, + { "name": "grid-toolbar", "main": "src/js/dist/bundle/index.js", "version": "0.0.0", "dependencies": ["@deephaven/js-plugin-pivot"] } ] } ``` -Here `pivot` is importable (has `package`); `grid-toolbar` only consumes. +Here `pivot` is importable (has `package`); `grid-toolbar` declares it as a dependency and only consumes. ### Consuming Another Plugin @@ -236,6 +245,6 @@ Here `pivot` is importable (has `package`); `grid-toolbar` only consumes. - Plugins are topologically sorted by their `dependencies` before loading. Circular dependencies throw an error at load time. -- Only plugins with a `package` field are registered in the resolve map. +- Only plugins with a `package` field are importable by other plugins. - The `package` value must exactly match the `import` string. - Dependency values in `dependencies` must match a `package` field of another plugin. diff --git a/packages/plugin/src/PluginUtils.tsx b/packages/plugin/src/PluginUtils.tsx index 186560d4f2..e3e389260f 100644 --- a/packages/plugin/src/PluginUtils.tsx +++ b/packages/plugin/src/PluginUtils.tsx @@ -214,8 +214,8 @@ export type PluginManifestPluginInfo = { /** * The npm package name for this plugin (e.g. `@deephaven/js-plugin-pivot`). * When provided, the plugin's exports are registered in the module resolve - * map under this key, enabling other plugins to `require()` this plugin at - * runtime. If omitted, the plugin is not registered for cross-plugin imports. + * map under this key, making this plugin importable by other plugins at + * runtime. If omitted, the plugin is not available for cross-plugin imports. */ package?: string; /** From 403d1663b22da3f1f46e6120b19ccaa42a95929e Mon Sep 17 00:00:00 2001 From: Vlad Babich Date: Thu, 23 Apr 2026 20:43:55 -0600 Subject: [PATCH 16/25] Self-review, cleanup --- .../src/WidgetLoaderPlugin.test.tsx | 97 +++++++ .../src/WidgetLoaderPlugin.tsx | 4 +- packages/plugin/src/PluginTypes.ts | 2 +- packages/plugin/src/PluginUtils.test.tsx | 255 ++++++++++++++++++ packages/plugin/src/PluginUtils.tsx | 36 ++- 5 files changed, 386 insertions(+), 8 deletions(-) diff --git a/packages/dashboard-core-plugins/src/WidgetLoaderPlugin.test.tsx b/packages/dashboard-core-plugins/src/WidgetLoaderPlugin.test.tsx index 63b5c4619f..e6d3c2b6cc 100644 --- a/packages/dashboard-core-plugins/src/WidgetLoaderPlugin.test.tsx +++ b/packages/dashboard-core-plugins/src/WidgetLoaderPlugin.test.tsx @@ -7,6 +7,7 @@ import { type WidgetMiddlewarePlugin, type WidgetComponentProps, type WidgetMiddlewareComponentProps, + type WidgetMiddlewarePanelProps, } from '@deephaven/plugin'; import { Provider } from 'react-redux'; import { Dashboard, PanelEvent } from '@deephaven/dashboard'; @@ -433,4 +434,100 @@ describe('middleware plugin chaining', () => { const wrapper = screen.getByTestId('middleware-wrapper'); expect(wrapper).toContainElement(screen.getByText('TestWidgetTwo')); }); + + it('middleware for one type does not affect a different type', async () => { + const otherPlugin: WidgetPlugin = { + name: 'other-widget', + type: PluginType.WIDGET_PLUGIN, + component: TestWidgetTwo, + supportedTypes: 'other-type', + }; + + const layoutManager = createAndMountDashboard([ + ['test-widget-plugin', testWidgetPlugin], + ['other-widget', otherPlugin], + ['test-middleware-plugin', testMiddlewarePlugin], // targets 'test-widget' only + ]); + + // Open the other type — middleware should NOT apply + act( + () => + layoutManager?.eventHub.emit(PanelEvent.OPEN, { + widget: { type: 'other-type' }, + }) + ); + + expect(screen.queryAllByText('TestWidgetTwo').length).toBe(1); + expect(screen.queryAllByText('MiddlewareWrapper').length).toBe(0); + }); + + it('chains panel middleware around base panelComponent', async () => { + function TestPanelMiddleware({ + Component, + ...props + }: WidgetMiddlewarePanelProps) { + return ( +
+ PanelMiddleware + {/* eslint-disable-next-line react/jsx-props-no-spreading */} + +
+ ); + } + + const panelMiddleware: WidgetMiddlewarePlugin = { + name: 'panel-middleware', + type: PluginType.WIDGET_PLUGIN, + component: TestMiddlewareWrapper, + panelComponent: TestPanelMiddleware, + supportedTypes: 'test-widget-panel', + isMiddleware: true, + }; + + const layoutManager = createAndMountDashboard([ + ['test-widget-plugin-with-panel', testWidgetPluginWithPanel], + ['panel-middleware', panelMiddleware], + ]); + + act( + () => + layoutManager?.eventHub.emit(PanelEvent.OPEN, { + widget: { type: 'test-widget-panel' }, + }) + ); + + // Panel middleware should wrap the base panel + expect(screen.queryAllByText('PanelMiddleware').length).toBe(1); + expect(screen.queryAllByText('TestPanel').length).toBe(1); + const wrapper = screen.getByTestId('panel-middleware'); + expect(wrapper).toContainElement(screen.getByText('TestPanel')); + }); + + it('skips component-only middleware when base has panelComponent', async () => { + // Middleware that only defines component (no panelComponent) + const componentOnlyMiddleware: WidgetMiddlewarePlugin = { + name: 'component-only-middleware', + type: PluginType.WIDGET_PLUGIN, + component: TestMiddlewareWrapper, + supportedTypes: 'test-widget-panel', + isMiddleware: true, + }; + + const layoutManager = createAndMountDashboard([ + ['test-widget-plugin-with-panel', testWidgetPluginWithPanel], + ['component-only-middleware', componentOnlyMiddleware], + ]); + + act( + () => + layoutManager?.eventHub.emit(PanelEvent.OPEN, { + widget: { type: 'test-widget-panel' }, + }) + ); + + // The base panel should render, but middleware wrapper should NOT appear + // because the middleware lacks panelComponent and the base has panelComponent + expect(screen.queryAllByText('TestPanel').length).toBe(1); + expect(screen.queryAllByText('MiddlewareWrapper').length).toBe(0); + }); }); diff --git a/packages/dashboard-core-plugins/src/WidgetLoaderPlugin.tsx b/packages/dashboard-core-plugins/src/WidgetLoaderPlugin.tsx index 0640e35d65..32c2e5a10a 100644 --- a/packages/dashboard-core-plugins/src/WidgetLoaderPlugin.tsx +++ b/packages/dashboard-core-plugins/src/WidgetLoaderPlugin.tsx @@ -283,8 +283,8 @@ export function WidgetLoaderPlugin( } // Has panel component - chain middleware around the panel. - // Middleware with panelComponent wraps at the panel level directly. - // Middleware with only component is auto-promoted to a panel wrapper. + // Only middleware that defines panelComponent is applied here. + // Middleware with only component is skipped in this path. log.debug( `Chaining panel components for ${basePlugin.name} (has custom panel component)` ); diff --git a/packages/plugin/src/PluginTypes.ts b/packages/plugin/src/PluginTypes.ts index 8c046e1c70..9676c5773b 100644 --- a/packages/plugin/src/PluginTypes.ts +++ b/packages/plugin/src/PluginTypes.ts @@ -206,7 +206,7 @@ export interface WidgetMiddlewarePlugin * Type guard to check if a plugin is a middleware plugin. */ export function isWidgetMiddlewarePlugin( - plugin: PluginModule + plugin: PluginModuleExport ): plugin is WidgetMiddlewarePlugin { return ( isWidgetPlugin(plugin) && diff --git a/packages/plugin/src/PluginUtils.test.tsx b/packages/plugin/src/PluginUtils.test.tsx index e8d03a9271..5aeca740ce 100644 --- a/packages/plugin/src/PluginUtils.test.tsx +++ b/packages/plugin/src/PluginUtils.test.tsx @@ -1,4 +1,5 @@ import React from 'react'; +import { render, screen } from '@testing-library/react'; import { FontAwesomeIcon } from '@fortawesome/react-fontawesome'; import { type ThemeData } from '@deephaven/components'; import { dhTruck, vsPreview } from '@deephaven/icons'; @@ -13,6 +14,11 @@ import { PluginType, type ThemePlugin, type WidgetPlugin, + type WidgetMiddlewarePlugin, + type WidgetComponentProps, + type WidgetMiddlewareComponentProps, + type WidgetPanelProps, + type WidgetMiddlewarePanelProps, } from './PluginTypes'; import { pluginSupportsType, @@ -23,6 +29,8 @@ import { processLoadedModule, registerPlugin, sortPluginsByDependency, + createChainedComponent, + createChainedPanelComponent, type PluginManifestPluginInfo, } from './PluginUtils'; @@ -354,6 +362,20 @@ describe('getPluginModuleValue', () => { expect(moduleValue).toBeNull(); }); + it('returns null for { default: null }', () => { + const moduleValue = getPluginModuleValue({ + default: null, + } as unknown as Plugin); + expect(moduleValue).toBeNull(); + }); + + it('returns null for { default: undefined }', () => { + const moduleValue = getPluginModuleValue({ + default: undefined, + } as unknown as Plugin); + expect(moduleValue).toBeNull(); + }); + describe('MultiPlugin', () => { const multiPlugin: MultiPlugin = { name: 'test-multi-plugin', @@ -744,3 +766,236 @@ describe('sortPluginsByDependency', () => { expect(sorted.map(p => p.name)).toEqual(['a', 'b']); }); }); + +describe('createChainedComponent', () => { + function BaseWidget({ fetch }: WidgetComponentProps) { + return
BaseWidget
; + } + + function makeMiddleware(name: string, label: string): WidgetMiddlewarePlugin { + function MiddlewareComp({ + Component, + ...props + }: WidgetMiddlewareComponentProps) { + return ( +
+ {label} + {/* eslint-disable-next-line react/jsx-props-no-spreading */} + +
+ ); + } + return { + name, + type: PluginType.WIDGET_PLUGIN, + supportedTypes: 'test-type', + isMiddleware: true, + component: MiddlewareComp, + }; + } + + it('returns base component unchanged when no middleware', () => { + const result = createChainedComponent(BaseWidget, []); + expect(result).toBe(BaseWidget); + }); + + it('wraps base component with single middleware', () => { + const mw = makeMiddleware('mw-a', 'A'); + const Chained = createChainedComponent(BaseWidget, [mw]); + + expect(Chained).not.toBe(BaseWidget); + expect(Chained.displayName).toBe('mw-a(BaseWidget)'); + }); + + it('chains multiple middleware in correct order', () => { + const mwA = makeMiddleware('mw-a', 'A'); + const mwB = makeMiddleware('mw-b', 'B'); + const Chained = createChainedComponent(BaseWidget, [mwA, mwB]); + + // First middleware is outermost + expect(Chained.displayName).toBe('mw-a(mw-b(BaseWidget))'); + }); + + it('sets displayName for each layer', () => { + const mwA = makeMiddleware('outer', 'Outer'); + const mwB = makeMiddleware('inner', 'Inner'); + const Chained = createChainedComponent(BaseWidget, [mwA, mwB]); + + expect(Chained.displayName).toContain('outer'); + expect(Chained.displayName).toContain('inner'); + expect(Chained.displayName).toContain('BaseWidget'); + }); + + it('skips middleware when widget type does not match supportedTypes', () => { + const mw = makeMiddleware('table-mw', 'TableOnly'); + // Override supportedTypes to only target 'table-type' + mw.supportedTypes = 'table-type'; + const Chained = createChainedComponent(BaseWidget, [mw]); + + const { container } = render( + + ); + + // Middleware should be skipped — base widget rendered directly + expect(screen.getByTestId('base')).toBeInTheDocument(); + expect(container.querySelector('[data-testid="table-mw"]')).toBeNull(); + }); + + it('applies middleware when widget type matches supportedTypes', () => { + const mw = makeMiddleware('table-mw', 'TableOnly'); + mw.supportedTypes = 'table-type'; + const Chained = createChainedComponent(BaseWidget, [mw]); + + render(); + + // Middleware should be applied + expect(screen.getByTestId('table-mw')).toBeInTheDocument(); + expect(screen.getByTestId('base')).toBeInTheDocument(); + }); + + it('applies middleware when metadata is undefined', () => { + const mw = makeMiddleware('mw-a', 'A'); + const Chained = createChainedComponent(BaseWidget, [mw]); + + render(); + + // No metadata means type is unknown — middleware should apply + expect(screen.getByTestId('mw-a')).toBeInTheDocument(); + expect(screen.getByTestId('base')).toBeInTheDocument(); + }); +}); + +describe('createChainedPanelComponent', () => { + function BasePanel({ fetch }: WidgetPanelProps) { + return
BasePanel
; + } + + function makePanelMiddleware( + name: string, + opts?: { hasPanelComponent?: boolean } + ): WidgetMiddlewarePlugin { + const hasPanelComp = opts?.hasPanelComponent ?? true; + + function MiddlewareComp({ + Component, + ...props + }: WidgetMiddlewareComponentProps) { + return ( +
+ {/* eslint-disable-next-line react/jsx-props-no-spreading */} + +
+ ); + } + + function PanelComp({ Component, ...props }: WidgetMiddlewarePanelProps) { + return ( +
+ {name} + {/* eslint-disable-next-line react/jsx-props-no-spreading */} + +
+ ); + } + + return { + name, + type: PluginType.WIDGET_PLUGIN, + supportedTypes: 'test-type', + isMiddleware: true, + component: MiddlewareComp, + panelComponent: hasPanelComp ? PanelComp : undefined, + }; + } + + it('returns base panel unchanged when no middleware', () => { + const result = createChainedPanelComponent(BasePanel, []); + expect(result).toBe(BasePanel); + }); + + it('returns base panel unchanged when middleware lacks panelComponent', () => { + const mw = makePanelMiddleware('mw-no-panel', { + hasPanelComponent: false, + }); + const result = createChainedPanelComponent(BasePanel, [mw]); + expect(result).toBe(BasePanel); + }); + + it('wraps base panel with single panel middleware', () => { + const mw = makePanelMiddleware('mw-panel'); + const Chained = createChainedPanelComponent(BasePanel, [mw]); + + expect(Chained).not.toBe(BasePanel); + expect(Chained.displayName).toBe('mw-panelPanel(BasePanel)'); + }); + + it('chains multiple panel middleware in correct order', () => { + const mwA = makePanelMiddleware('outer'); + const mwB = makePanelMiddleware('inner'); + const Chained = createChainedPanelComponent(BasePanel, [mwA, mwB]); + + expect(Chained.displayName).toBe('outerPanel(innerPanel(BasePanel))'); + }); + + it('filters out middleware without panelComponent', () => { + const mwWithPanel = makePanelMiddleware('with-panel', { + hasPanelComponent: true, + }); + const mwWithout = makePanelMiddleware('without-panel', { + hasPanelComponent: false, + }); + const Chained = createChainedPanelComponent(BasePanel, [ + mwWithPanel, + mwWithout, + ]); + + // Only the middleware with panelComponent should be applied + expect(Chained.displayName).toBe('with-panelPanel(BasePanel)'); + }); + + it('skips panel middleware when widget type does not match supportedTypes', () => { + const mw = makePanelMiddleware('table-panel-mw'); + mw.supportedTypes = 'table-type'; + const Chained = createChainedPanelComponent(BasePanel, [mw]); + + const panelProps = { + fetch: jest.fn(), + metadata: { type: 'other-type' }, + localDashboardId: 'test', + glContainer: {} as WidgetPanelProps['glContainer'], + glEventHub: {} as WidgetPanelProps['glEventHub'], + }; + + const { container } = render( + // eslint-disable-next-line react/jsx-props-no-spreading + + ); + + // Middleware should be skipped + expect(screen.getByTestId('base-panel')).toBeInTheDocument(); + expect( + container.querySelector('[data-testid="table-panel-mw-panel"]') + ).toBeNull(); + }); + + it('applies panel middleware when widget type matches supportedTypes', () => { + const mw = makePanelMiddleware('table-panel-mw'); + mw.supportedTypes = 'table-type'; + const Chained = createChainedPanelComponent(BasePanel, [mw]); + + const panelProps = { + fetch: jest.fn(), + metadata: { type: 'table-type' }, + localDashboardId: 'test', + glContainer: {} as WidgetPanelProps['glContainer'], + glEventHub: {} as WidgetPanelProps['glEventHub'], + }; + + // eslint-disable-next-line react/jsx-props-no-spreading + render(); + + // Middleware should be applied + expect(screen.getByTestId('table-panel-mw-panel')).toBeInTheDocument(); + expect(screen.getByTestId('base-panel')).toBeInTheDocument(); + }); +}); diff --git a/packages/plugin/src/PluginUtils.tsx b/packages/plugin/src/PluginUtils.tsx index e3e389260f..0681714ffe 100644 --- a/packages/plugin/src/PluginUtils.tsx +++ b/packages/plugin/src/PluginUtils.tsx @@ -136,11 +136,25 @@ export function createChainedComponent( .reduce>>( (WrappedComponent, middlewarePlugin) => { const MiddlewareComponent = middlewarePlugin.component; - - function ChainedComponent(props: WidgetComponentProps) { + const supported = [middlewarePlugin.supportedTypes].flat(); + + function ChainedComponent({ + metadata, + ...rest + }: WidgetComponentProps) { + // Skip middleware if the widget type doesn't match its supportedTypes + if (metadata?.type != null && !supported.includes(metadata.type)) { + // eslint-disable-next-line react/jsx-props-no-spreading + return ; + } return ( // eslint-disable-next-line react/jsx-props-no-spreading - + ); } ChainedComponent.displayName = `${middlewarePlugin.name}(${ @@ -190,11 +204,22 @@ export function createChainedPanelComponent( .reduce>>( (WrappedPanel, middlewarePlugin) => { const { panelComponent: MiddlewarePanelComponent } = middlewarePlugin; + const supported = [middlewarePlugin.supportedTypes].flat(); - function ChainedPanel(props: WidgetPanelProps) { + function ChainedPanel({ metadata, ...rest }: WidgetPanelProps) { + // Skip middleware if the widget type doesn't match its supportedTypes + if (metadata?.type != null && !supported.includes(metadata.type)) { + // eslint-disable-next-line react/jsx-props-no-spreading + return ; + } return ( // eslint-disable-next-line react/jsx-props-no-spreading - + ); } ChainedPanel.displayName = `${middlewarePlugin.name}Panel(${ @@ -339,6 +364,7 @@ function hasDefaultExport(value: unknown): value is { default: Plugin } { return ( typeof value === 'object' && value != null && + (value as { default?: unknown }).default != null && typeof (value as { default?: unknown }).default === 'object' ); } From e610a4b791149e021c26be3bd8407be79537fad8 Mon Sep 17 00:00:00 2001 From: Vlad Babich Date: Thu, 23 Apr 2026 20:49:33 -0600 Subject: [PATCH 17/25] Address review comment --- packages/plugin/src/PluginUtils.tsx | 36 ++++++++++++++++++----------- 1 file changed, 22 insertions(+), 14 deletions(-) diff --git a/packages/plugin/src/PluginUtils.tsx b/packages/plugin/src/PluginUtils.tsx index 0681714ffe..01764a7e43 100644 --- a/packages/plugin/src/PluginUtils.tsx +++ b/packages/plugin/src/PluginUtils.tsx @@ -25,6 +25,18 @@ import { const log = Log.module('@deephaven/plugin.PluginUtils'); +/** + * Get a display-friendly name for a React component. + * Prefers displayName, falls back to name, then the provided fallback. + */ +function getComponentName( + // eslint-disable-next-line @typescript-eslint/no-explicit-any + component: React.ComponentType, + fallback = 'Component' +): string { + return component.displayName ?? component.name ?? fallback; +} + export function pluginSupportsType( plugin: PluginModule | undefined, type: string @@ -118,14 +130,14 @@ export function createChainedComponent( if (middleware.length === 0) { log.debug( 'No middleware to chain for component', - baseComponent.displayName ?? baseComponent.name + getComponentName(baseComponent) ); return baseComponent; } log.debug( 'Chaining component middleware', - baseComponent.displayName ?? baseComponent.name, + getComponentName(baseComponent), middleware.map(m => m.name) ); @@ -157,11 +169,9 @@ export function createChainedComponent( /> ); } - ChainedComponent.displayName = `${middlewarePlugin.name}(${ - (WrappedComponent as React.ComponentType).displayName ?? - (WrappedComponent as React.ComponentType).name ?? - 'Component' - })`; + ChainedComponent.displayName = `${ + middlewarePlugin.name + }(${getComponentName(WrappedComponent)})`; return ChainedComponent; }, baseComponent @@ -187,14 +197,14 @@ export function createChainedPanelComponent( if (panelMiddleware.length === 0) { log.debug( 'No panel middleware to chain for panel component', - basePanelComponent.displayName ?? basePanelComponent.name + getComponentName(basePanelComponent) ); return basePanelComponent; } log.debug( 'Chaining panel middleware', - basePanelComponent.displayName ?? basePanelComponent.name, + getComponentName(basePanelComponent), panelMiddleware.map(m => m.name) ); @@ -222,11 +232,9 @@ export function createChainedPanelComponent( /> ); } - ChainedPanel.displayName = `${middlewarePlugin.name}Panel(${ - (WrappedPanel as React.ComponentType).displayName ?? - (WrappedPanel as React.ComponentType).name ?? - 'Panel' - })`; + ChainedPanel.displayName = `${ + middlewarePlugin.name + }Panel(${getComponentName(WrappedPanel, 'Panel')})`; return ChainedPanel; }, basePanelComponent From 44503a1264486288053e73a617311bce5f962a70 Mon Sep 17 00:00:00 2001 From: Vlad Babich Date: Fri, 24 Apr 2026 11:49:28 -0600 Subject: [PATCH 18/25] Change middleware flag to new plugin type --- .../src/WidgetLoaderPlugin.test.tsx | 12 ++---- .../src/WidgetLoaderPlugin.tsx | 5 +-- packages/plugin/FEATURE.md | 41 +++++++++++++++++++ .../plugin/docs/middleware-architecture.md | 9 ++-- packages/plugin/src/PluginTypes.test.ts | 12 +----- packages/plugin/src/PluginTypes.ts | 29 +++++++------ packages/plugin/src/PluginUtils.test.tsx | 6 +-- packages/plugin/src/WidgetView.tsx | 6 ++- 8 files changed, 75 insertions(+), 45 deletions(-) create mode 100644 packages/plugin/FEATURE.md diff --git a/packages/dashboard-core-plugins/src/WidgetLoaderPlugin.test.tsx b/packages/dashboard-core-plugins/src/WidgetLoaderPlugin.test.tsx index e6d3c2b6cc..125ca346f6 100644 --- a/packages/dashboard-core-plugins/src/WidgetLoaderPlugin.test.tsx +++ b/packages/dashboard-core-plugins/src/WidgetLoaderPlugin.test.tsx @@ -300,18 +300,16 @@ describe('middleware plugin chaining', () => { const testMiddlewarePlugin: WidgetMiddlewarePlugin = { name: 'test-middleware', - type: PluginType.WIDGET_PLUGIN, + type: PluginType.MIDDLEWARE_PLUGIN, component: TestMiddlewareWrapper, supportedTypes: 'test-widget', - isMiddleware: true, }; const testMiddlewarePluginTwo: WidgetMiddlewarePlugin = { name: 'test-middleware-two', - type: PluginType.WIDGET_PLUGIN, + type: PluginType.MIDDLEWARE_PLUGIN, component: TestMiddlewareWrapperTwo, supportedTypes: 'test-widget', - isMiddleware: true, }; it('chains middleware plugin around base widget', async () => { @@ -477,11 +475,10 @@ describe('middleware plugin chaining', () => { const panelMiddleware: WidgetMiddlewarePlugin = { name: 'panel-middleware', - type: PluginType.WIDGET_PLUGIN, + type: PluginType.MIDDLEWARE_PLUGIN, component: TestMiddlewareWrapper, panelComponent: TestPanelMiddleware, supportedTypes: 'test-widget-panel', - isMiddleware: true, }; const layoutManager = createAndMountDashboard([ @@ -507,10 +504,9 @@ describe('middleware plugin chaining', () => { // Middleware that only defines component (no panelComponent) const componentOnlyMiddleware: WidgetMiddlewarePlugin = { name: 'component-only-middleware', - type: PluginType.WIDGET_PLUGIN, + type: PluginType.MIDDLEWARE_PLUGIN, component: TestMiddlewareWrapper, supportedTypes: 'test-widget-panel', - isMiddleware: true, }; const layoutManager = createAndMountDashboard([ diff --git a/packages/dashboard-core-plugins/src/WidgetLoaderPlugin.tsx b/packages/dashboard-core-plugins/src/WidgetLoaderPlugin.tsx index 32c2e5a10a..a3cafe059d 100644 --- a/packages/dashboard-core-plugins/src/WidgetLoaderPlugin.tsx +++ b/packages/dashboard-core-plugins/src/WidgetLoaderPlugin.tsx @@ -110,12 +110,11 @@ export function WidgetLoaderPlugin( const typeMap = new Map(); plugins.forEach(plugin => { - if (!isWidgetPlugin(plugin)) { + const isMiddleware = isWidgetMiddlewarePlugin(plugin); + if (!isWidgetPlugin(plugin) && !isMiddleware) { return; } - const isMiddleware = isWidgetMiddlewarePlugin(plugin); - [plugin.supportedTypes].flat().forEach(supportedType => { if (supportedType == null || supportedType === '') { return; diff --git a/packages/plugin/FEATURE.md b/packages/plugin/FEATURE.md new file mode 100644 index 0000000000..8f1be1b6f5 --- /dev/null +++ b/packages/plugin/FEATURE.md @@ -0,0 +1,41 @@ +# Middleware Plugin Infrastructure for Widget Chaining + +**PR:** [#2660](https://github.com/deephaven/web-client-ui/pull/2660) | **Ticket:** DH-21476 + +## Summary + +Adds middleware plugin support so plugins can wrap existing widgets without replacing them, cross-plugin dependency loading via manifest `package`/`dependencies` fields, and shared plugin-loading utilities in `@deephaven/plugin`. + +## What Changed + +**Middleware chaining** — New `WidgetMiddlewarePlugin` type with its own `PluginType.MIDDLEWARE_PLUGIN` discriminator. Middleware receives a `Component` prop and wraps the next layer. Multiple middleware compose in registration order. Applied in both `WidgetLoaderPlugin` (dashboard panels) and `WidgetView` (inline widgets) via `createChainedComponent`/`createChainedPanelComponent`. The chaining functions automatically filter middleware by `supportedTypes` at render time, so middleware only activates for matching widget types. + +**Cross-plugin dependencies** — Manifest entries can declare `package` (makes the plugin's exports available to other plugins) and `dependencies` (topological sort so deps load first). Plugins load sequentially so each plugin's exports are available to subsequent plugins via standard `import` statements. + +**Shared loading utilities** — `getPluginModuleValue`, `registerPlugin`, `processLoadedModule`, `sortPluginsByDependency`, and manifest types moved from `@deephaven/app-utils` into `@deephaven/plugin`. Both web-client-ui and gplus consume these instead of duplicating the logic. ~80 lines removed from app-utils. + +## Key Files + +| File | Change | +|---|---| +| `packages/plugin/src/PluginTypes.ts` | Middleware types, type guard | +| `packages/plugin/src/PluginUtils.tsx` | Chaining functions, loader utils, manifest types | +| `packages/plugin/src/WidgetView.tsx` | Middleware-aware inline rendering | +| `packages/dashboard-core-plugins/src/WidgetLoaderPlugin.tsx` | Middleware-aware panel loading | +| `packages/app-utils/src/plugins/PluginUtils.tsx` | Simplified to use shared utils | +| `packages/plugin/docs/middleware-architecture.md` | Architecture reference | + +## Breaking Changes + +`PluginManifestPluginInfo`, `PluginManifest`, and `getPluginModuleValue` are no longer exported from `@deephaven/app-utils`. Import them from `@deephaven/plugin` instead. + +## PR Discussion + +**Why is `WidgetMiddlewarePlugin` its own `PluginType` rather than a `WidgetPlugin` subtype with a flag?** + +- **Clean type narrowing** — the discriminated union splits cleanly on `type`. `isWidgetPlugin` matches base widget plugins only; `isWidgetMiddlewarePlugin` matches middleware only. There is no overlap and no "is it a flagged variant of the other?" check. +- **No hidden field-shape coupling** — with the previous `Omit` approach, any change to `WidgetPlugin` (e.g. a new required field) silently propagated into the middleware contract. A standalone interface declares exactly what middleware needs (`name`, `supportedTypes`, `component`, optional `panelComponent`). +- **Clearer intent at the call site** — `type: PluginType.MIDDLEWARE_PLUGIN` is self-documenting. `type: PluginType.WIDGET_PLUGIN, isMiddleware: true` requires the reader to know that the boolean flag fundamentally changes the runtime role of the plugin. +- **Migration cost is small** — only the two iteration sites (`WidgetLoaderPlugin`, `WidgetView`) needed to add `|| isWidgetMiddlewarePlugin(p)` to their filter. No external consumers in this repo, gplus, or deephaven-plugins iterate plugins and care about the distinction. + +**Trade-off accepted:** `supportedTypes` is duplicated as a concept across `WidgetPlugin` and `WidgetMiddlewarePlugin`. This is intentional — the field has the same semantics but the two interfaces have independent lifecycles, and shared inheritance proved more brittle than helpful. diff --git a/packages/plugin/docs/middleware-architecture.md b/packages/plugin/docs/middleware-architecture.md index 043eb84b43..b2037cf3e5 100644 --- a/packages/plugin/docs/middleware-architecture.md +++ b/packages/plugin/docs/middleware-architecture.md @@ -29,9 +29,8 @@ Middleware plugins allow you to wrap and enhance existing widget plugins without ```tsx interface WidgetMiddlewarePlugin { name: string; - type: PluginType.WIDGET_PLUGIN; + type: PluginType.MIDDLEWARE_PLUGIN; supportedTypes: string | string[]; - isMiddleware: true; // Required: wraps the widget component component: React.ComponentType>; @@ -90,10 +89,9 @@ function MyToolbar({ Component, ...props }: WidgetMiddlewareComponentProps) { const myToolbarPlugin = { name: 'my-toolbar-middleware', - type: PluginType.WIDGET_PLUGIN, + type: PluginType.MIDDLEWARE_PLUGIN, component: MyToolbar, supportedTypes: 'deephaven.plot.express.DeephavenFigure', - isMiddleware: true, } satisfies WidgetMiddlewarePlugin; ``` @@ -152,10 +150,9 @@ function ConditionalMiddleware({ Component, ...props }: WidgetMiddlewareComponen ```tsx const multiTypeMiddleware = { name: 'multi-type-middleware', - type: PluginType.WIDGET_PLUGIN, + type: PluginType.MIDDLEWARE_PLUGIN, component: MyMiddlewareComponent, supportedTypes: ['deephaven.plot.express.DeephavenFigure', 'deephaven.ui.Element'], - isMiddleware: true, } satisfies WidgetMiddlewarePlugin; ``` diff --git a/packages/plugin/src/PluginTypes.test.ts b/packages/plugin/src/PluginTypes.test.ts index d76a11afe8..1de2a2fd69 100644 --- a/packages/plugin/src/PluginTypes.test.ts +++ b/packages/plugin/src/PluginTypes.test.ts @@ -18,6 +18,7 @@ const pluginTypeToTypeGuardMap = [ [PluginType.AUTH_PLUGIN, isAuthPlugin], [PluginType.DASHBOARD_PLUGIN, isDashboardPlugin], [PluginType.ELEMENT_PLUGIN, isElementPlugin], + [PluginType.MIDDLEWARE_PLUGIN, isWidgetMiddlewarePlugin], [PluginType.MULTI_PLUGIN, isMultiPlugin], [PluginType.TABLE_PLUGIN, isTablePlugin], [PluginType.THEME_PLUGIN, isThemePlugin], @@ -65,10 +66,9 @@ describe('isWidgetMiddlewarePlugin', () => { const middlewarePlugin: WidgetMiddlewarePlugin = { name: 'test-middleware', - type: PluginType.WIDGET_PLUGIN, + type: PluginType.MIDDLEWARE_PLUGIN, component: () => null, supportedTypes: 'test-type', - isMiddleware: true, }; it('returns true for middleware plugins', () => { @@ -79,14 +79,6 @@ describe('isWidgetMiddlewarePlugin', () => { expect(isWidgetMiddlewarePlugin(baseWidgetPlugin)).toBe(false); }); - it('returns false for widget plugins with isMiddleware set to false', () => { - const notMiddleware = { - ...baseWidgetPlugin, - isMiddleware: false, - }; - expect(isWidgetMiddlewarePlugin(notMiddleware)).toBe(false); - }); - it('returns false for non-widget plugins', () => { expect( isWidgetMiddlewarePlugin({ diff --git a/packages/plugin/src/PluginTypes.ts b/packages/plugin/src/PluginTypes.ts index 9676c5773b..bc1972422f 100644 --- a/packages/plugin/src/PluginTypes.ts +++ b/packages/plugin/src/PluginTypes.ts @@ -12,6 +12,7 @@ export const PluginType = Object.freeze({ AUTH_PLUGIN: 'AuthPlugin', DASHBOARD_PLUGIN: 'DashboardPlugin', ELEMENT_PLUGIN: 'ElementPlugin', + MIDDLEWARE_PLUGIN: 'MiddlewarePlugin', MULTI_PLUGIN: 'MultiPlugin', TABLE_PLUGIN: 'TablePlugin', THEME_PLUGIN: 'ThemePlugin', @@ -180,14 +181,13 @@ export interface WidgetMiddlewarePanelProps * - Intercept and modify props before they reach the wrapped component * - Provide additional context or state to the wrapped component * - Add menu items or other extensions to the widget + * + * A middleware plugin uses its own `PluginType.MIDDLEWARE_PLUGIN` discriminator + * (rather than being a flagged `WidgetPlugin`) so consumers can cleanly + * distinguish base widget plugins from middleware via type narrowing. */ -export interface WidgetMiddlewarePlugin - extends Omit, 'component' | 'panelComponent'> { - /** - * Marks this plugin as middleware that should be chained - * with other plugins of the same supportedTypes. - */ - isMiddleware: true; +export interface WidgetMiddlewarePlugin extends Plugin { + type: typeof PluginType.MIDDLEWARE_PLUGIN; /** * The middleware component that wraps the base widget component. @@ -195,6 +195,12 @@ export interface WidgetMiddlewarePlugin */ component: React.ComponentType>; + /** + * The server widget types that this middleware applies to. + * Matches the same `supportedTypes` semantics as `WidgetPlugin`. + */ + supportedTypes: string | string[]; + /** * The middleware panel component that wraps the base panel component. * If omitted, only the component middleware will be applied. @@ -208,11 +214,7 @@ export interface WidgetMiddlewarePlugin export function isWidgetMiddlewarePlugin( plugin: PluginModuleExport ): plugin is WidgetMiddlewarePlugin { - return ( - isWidgetPlugin(plugin) && - 'isMiddleware' in plugin && - plugin.isMiddleware === true - ); + return 'type' in plugin && plugin.type === PluginType.MIDDLEWARE_PLUGIN; } export interface WidgetPlugin extends Plugin { @@ -389,6 +391,7 @@ export function isPlugin(plugin: unknown): plugin is Plugin { isMultiPlugin(plugin as PluginModuleExport) || isTablePlugin(plugin as PluginModuleExport) || isThemePlugin(plugin as PluginModuleExport) || - isWidgetPlugin(plugin as PluginModuleExport) + isWidgetPlugin(plugin as PluginModuleExport) || + isWidgetMiddlewarePlugin(plugin as PluginModuleExport) ); } diff --git a/packages/plugin/src/PluginUtils.test.tsx b/packages/plugin/src/PluginUtils.test.tsx index 5aeca740ce..c93e9a8eae 100644 --- a/packages/plugin/src/PluginUtils.test.tsx +++ b/packages/plugin/src/PluginUtils.test.tsx @@ -787,9 +787,8 @@ describe('createChainedComponent', () => { } return { name, - type: PluginType.WIDGET_PLUGIN, + type: PluginType.MIDDLEWARE_PLUGIN, supportedTypes: 'test-type', - isMiddleware: true, component: MiddlewareComp, }; } @@ -900,9 +899,8 @@ describe('createChainedPanelComponent', () => { return { name, - type: PluginType.WIDGET_PLUGIN, + type: PluginType.MIDDLEWARE_PLUGIN, supportedTypes: 'test-type', - isMiddleware: true, component: MiddlewareComp, panelComponent: hasPanelComp ? PanelComp : undefined, }; diff --git a/packages/plugin/src/WidgetView.tsx b/packages/plugin/src/WidgetView.tsx index cb0e322a11..9636cc7b91 100644 --- a/packages/plugin/src/WidgetView.tsx +++ b/packages/plugin/src/WidgetView.tsx @@ -26,7 +26,11 @@ export function WidgetView({ fetch, type }: WidgetViewProps): JSX.Element { let foundBasePlugin: WidgetPlugin | undefined; const foundMiddleware: WidgetMiddlewarePlugin[] = []; - [...plugins.values()].filter(isWidgetPlugin).forEach(p => { + plugins.forEach(p => { + if (!isWidgetPlugin(p) && !isWidgetMiddlewarePlugin(p)) { + return; + } + const supportsType = [p.supportedTypes].flat().includes(type); if (!supportsType) { return; From 4b2f7352851e1919c9d2dbeb856539c93df3bbbe Mon Sep 17 00:00:00 2001 From: Vlad Babich Date: Fri, 24 Apr 2026 11:50:03 -0600 Subject: [PATCH 19/25] Delete feature doc --- packages/plugin/FEATURE.md | 41 -------------------------------------- 1 file changed, 41 deletions(-) delete mode 100644 packages/plugin/FEATURE.md diff --git a/packages/plugin/FEATURE.md b/packages/plugin/FEATURE.md deleted file mode 100644 index 8f1be1b6f5..0000000000 --- a/packages/plugin/FEATURE.md +++ /dev/null @@ -1,41 +0,0 @@ -# Middleware Plugin Infrastructure for Widget Chaining - -**PR:** [#2660](https://github.com/deephaven/web-client-ui/pull/2660) | **Ticket:** DH-21476 - -## Summary - -Adds middleware plugin support so plugins can wrap existing widgets without replacing them, cross-plugin dependency loading via manifest `package`/`dependencies` fields, and shared plugin-loading utilities in `@deephaven/plugin`. - -## What Changed - -**Middleware chaining** — New `WidgetMiddlewarePlugin` type with its own `PluginType.MIDDLEWARE_PLUGIN` discriminator. Middleware receives a `Component` prop and wraps the next layer. Multiple middleware compose in registration order. Applied in both `WidgetLoaderPlugin` (dashboard panels) and `WidgetView` (inline widgets) via `createChainedComponent`/`createChainedPanelComponent`. The chaining functions automatically filter middleware by `supportedTypes` at render time, so middleware only activates for matching widget types. - -**Cross-plugin dependencies** — Manifest entries can declare `package` (makes the plugin's exports available to other plugins) and `dependencies` (topological sort so deps load first). Plugins load sequentially so each plugin's exports are available to subsequent plugins via standard `import` statements. - -**Shared loading utilities** — `getPluginModuleValue`, `registerPlugin`, `processLoadedModule`, `sortPluginsByDependency`, and manifest types moved from `@deephaven/app-utils` into `@deephaven/plugin`. Both web-client-ui and gplus consume these instead of duplicating the logic. ~80 lines removed from app-utils. - -## Key Files - -| File | Change | -|---|---| -| `packages/plugin/src/PluginTypes.ts` | Middleware types, type guard | -| `packages/plugin/src/PluginUtils.tsx` | Chaining functions, loader utils, manifest types | -| `packages/plugin/src/WidgetView.tsx` | Middleware-aware inline rendering | -| `packages/dashboard-core-plugins/src/WidgetLoaderPlugin.tsx` | Middleware-aware panel loading | -| `packages/app-utils/src/plugins/PluginUtils.tsx` | Simplified to use shared utils | -| `packages/plugin/docs/middleware-architecture.md` | Architecture reference | - -## Breaking Changes - -`PluginManifestPluginInfo`, `PluginManifest`, and `getPluginModuleValue` are no longer exported from `@deephaven/app-utils`. Import them from `@deephaven/plugin` instead. - -## PR Discussion - -**Why is `WidgetMiddlewarePlugin` its own `PluginType` rather than a `WidgetPlugin` subtype with a flag?** - -- **Clean type narrowing** — the discriminated union splits cleanly on `type`. `isWidgetPlugin` matches base widget plugins only; `isWidgetMiddlewarePlugin` matches middleware only. There is no overlap and no "is it a flagged variant of the other?" check. -- **No hidden field-shape coupling** — with the previous `Omit` approach, any change to `WidgetPlugin` (e.g. a new required field) silently propagated into the middleware contract. A standalone interface declares exactly what middleware needs (`name`, `supportedTypes`, `component`, optional `panelComponent`). -- **Clearer intent at the call site** — `type: PluginType.MIDDLEWARE_PLUGIN` is self-documenting. `type: PluginType.WIDGET_PLUGIN, isMiddleware: true` requires the reader to know that the boolean flag fundamentally changes the runtime role of the plugin. -- **Migration cost is small** — only the two iteration sites (`WidgetLoaderPlugin`, `WidgetView`) needed to add `|| isWidgetMiddlewarePlugin(p)` to their filter. No external consumers in this repo, gplus, or deephaven-plugins iterate plugins and care about the distinction. - -**Trade-off accepted:** `supportedTypes` is duplicated as a concept across `WidgetPlugin` and `WidgetMiddlewarePlugin`. This is intentional — the field has the same semantics but the two interfaces have independent lifecycles, and shared inheritance proved more brittle than helpful. From 9a4c44202de5b3b1bcc4da0aeddf5223d6537720 Mon Sep 17 00:00:00 2001 From: Vlad Babich Date: Mon, 27 Apr 2026 12:17:38 -0600 Subject: [PATCH 20/25] Fix types, package-lock --- package-lock.json | 1 + packages/redux/src/store.ts | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/package-lock.json b/package-lock.json index d90177048e..aca94891bf 100644 --- a/package-lock.json +++ b/package-lock.json @@ -30003,6 +30003,7 @@ "@deephaven/dashboard-core-plugins": "file:../dashboard-core-plugins", "@deephaven/file-explorer": "file:../file-explorer", "@deephaven/golden-layout": "file:../golden-layout", + "@deephaven/grid": "file:../grid", "@deephaven/icons": "file:../icons", "@deephaven/iris-grid": "file:../iris-grid", "@deephaven/jsapi-bootstrap": "file:../jsapi-bootstrap", diff --git a/packages/redux/src/store.ts b/packages/redux/src/store.ts index fe023d988c..ac696f4b16 100644 --- a/packages/redux/src/store.ts +++ b/packages/redux/src/store.ts @@ -108,7 +108,8 @@ export type PluginModule = | 'TablePlugin' | 'ThemePlugin' | 'ElementPlugin' - | 'MultiPlugin'; + | 'MultiPlugin' + | 'MiddlewarePlugin'; } | { // eslint-disable-next-line @typescript-eslint/no-explicit-any From 28a771e396d9fdf2066ae5064d38e618002bfad2 Mon Sep 17 00:00:00 2001 From: Vlad Babich Date: Tue, 28 Apr 2026 14:28:30 -0600 Subject: [PATCH 21/25] Extract topological sort into a separate util --- packages/plugin/src/PluginUtils.test.tsx | 160 ------------------ packages/plugin/src/PluginUtils.tsx | 105 ------------ packages/plugin/src/index.ts | 1 + packages/plugin/src/pluginSortUtil/index.ts | 1 + .../sortPluginsByDependency.test.ts | 160 ++++++++++++++++++ .../pluginSortUtil/sortPluginsByDependency.ts | 109 ++++++++++++ 6 files changed, 271 insertions(+), 265 deletions(-) create mode 100644 packages/plugin/src/pluginSortUtil/index.ts create mode 100644 packages/plugin/src/pluginSortUtil/sortPluginsByDependency.test.ts create mode 100644 packages/plugin/src/pluginSortUtil/sortPluginsByDependency.ts diff --git a/packages/plugin/src/PluginUtils.test.tsx b/packages/plugin/src/PluginUtils.test.tsx index c93e9a8eae..14fc57ccc3 100644 --- a/packages/plugin/src/PluginUtils.test.tsx +++ b/packages/plugin/src/PluginUtils.test.tsx @@ -28,10 +28,8 @@ import { getPluginModuleValue, processLoadedModule, registerPlugin, - sortPluginsByDependency, createChainedComponent, createChainedPanelComponent, - type PluginManifestPluginInfo, } from './PluginUtils'; function TestWidget() { @@ -609,164 +607,6 @@ describe('processLoadedModule', () => { }); }); -describe('sortPluginsByDependency', () => { - function makeManifestPlugin( - name: string, - opts?: { - package?: string; - dependencies?: string[]; - } - ): PluginManifestPluginInfo { - return { - name, - main: 'index.js', - version: '1.0.0', - package: opts?.package, - dependencies: opts?.dependencies, - }; - } - - it('returns plugins in original order when no dependencies', () => { - const plugins = [ - makeManifestPlugin('a'), - makeManifestPlugin('b'), - makeManifestPlugin('c'), - ]; - - const sorted = sortPluginsByDependency(plugins); - - expect(sorted.map(p => p.name)).toEqual(['a', 'b', 'c']); - }); - - it('reorders so dependency loads before consumer', () => { - const plugins = [ - makeManifestPlugin('consumer', { - dependencies: ['@scope/dep'], - }), - makeManifestPlugin('dep', { - package: '@scope/dep', - }), - ]; - - const sorted = sortPluginsByDependency(plugins); - - expect(sorted.map(p => p.name)).toEqual(['dep', 'consumer']); - }); - - it('handles a chain of dependencies: a → b → c', () => { - const plugins = [ - makeManifestPlugin('c', { - package: '@scope/c', - dependencies: ['@scope/b'], - }), - makeManifestPlugin('b', { - package: '@scope/b', - dependencies: ['@scope/a'], - }), - makeManifestPlugin('a', { - package: '@scope/a', - }), - ]; - - const sorted = sortPluginsByDependency(plugins); - - const names = sorted.map(p => p.name); - expect(names.indexOf('a')).toBeLessThan(names.indexOf('b')); - expect(names.indexOf('b')).toBeLessThan(names.indexOf('c')); - }); - - it('preserves original order among independent plugins', () => { - const plugins = [ - makeManifestPlugin('x'), - makeManifestPlugin('dep', { package: '@scope/dep' }), - makeManifestPlugin('y'), - makeManifestPlugin('consumer', { dependencies: ['@scope/dep'] }), - makeManifestPlugin('z'), - ]; - - const sorted = sortPluginsByDependency(plugins); - - const names = sorted.map(p => p.name); - // dep must come before consumer - expect(names.indexOf('dep')).toBeLessThan(names.indexOf('consumer')); - // independent plugins keep their relative order - expect(names.indexOf('x')).toBeLessThan(names.indexOf('y')); - expect(names.indexOf('y')).toBeLessThan(names.indexOf('z')); - }); - - it('throws on circular dependencies', () => { - const plugins = [ - makeManifestPlugin('a', { - package: '@scope/a', - dependencies: ['@scope/b'], - }), - makeManifestPlugin('b', { - package: '@scope/b', - dependencies: ['@scope/a'], - }), - ]; - - expect(() => sortPluginsByDependency(plugins)).toThrow( - /Circular plugin dependency/ - ); - }); - - it('warns and ignores dependencies not in the manifest', () => { - const plugins = [ - makeManifestPlugin('consumer', { - dependencies: ['@scope/nonexistent'], - }), - ]; - - const sorted = sortPluginsByDependency(plugins); - - expect(sorted.map(p => p.name)).toEqual(['consumer']); - }); - - it('handles multiple dependencies', () => { - const plugins = [ - makeManifestPlugin('consumer', { - dependencies: ['@scope/dep-a', '@scope/dep-b'], - }), - makeManifestPlugin('dep-a', { package: '@scope/dep-a' }), - makeManifestPlugin('dep-b', { package: '@scope/dep-b' }), - ]; - - const sorted = sortPluginsByDependency(plugins); - - const names = sorted.map(p => p.name); - expect(names.indexOf('dep-a')).toBeLessThan(names.indexOf('consumer')); - expect(names.indexOf('dep-b')).toBeLessThan(names.indexOf('consumer')); - }); - - it('does not mutate the input array', () => { - const plugins = [ - makeManifestPlugin('consumer', { dependencies: ['@scope/dep'] }), - makeManifestPlugin('dep', { package: '@scope/dep' }), - ]; - const original = [...plugins]; - - sortPluginsByDependency(plugins); - - expect(plugins).toEqual(original); - }); - - it('handles empty plugin list', () => { - expect(sortPluginsByDependency([])).toEqual([]); - }); - - it('handles plugins with empty dependencies array', () => { - const plugins = [ - makeManifestPlugin('a', { dependencies: [] }), - makeManifestPlugin('b'), - ]; - - const sorted = sortPluginsByDependency(plugins); - - expect(sorted.map(p => p.name)).toEqual(['a', 'b']); - }); -}); - describe('createChainedComponent', () => { function BaseWidget({ fetch }: WidgetComponentProps) { return
BaseWidget
; diff --git a/packages/plugin/src/PluginUtils.tsx b/packages/plugin/src/PluginUtils.tsx index 01764a7e43..c652537c9d 100644 --- a/packages/plugin/src/PluginUtils.tsx +++ b/packages/plugin/src/PluginUtils.tsx @@ -263,111 +263,6 @@ export type PluginManifestPluginInfo = { export type PluginManifest = { plugins: PluginManifestPluginInfo[] }; -/** - * Topologically sort plugins so that dependencies are loaded before the - * plugins that depend on them. Plugins without dependencies or whose - * dependencies are not in the manifest keep their original relative order - * (stable sort). Throws if a dependency cycle is detected. - * - * @param plugins The plugin list from the manifest - * @returns A new array with plugins sorted so dependencies come first - */ -export function sortPluginsByDependency< - T extends Pick, ->(plugins: readonly T[]): T[] { - // Build a lookup from package name → plugin index - const packageToIndex = new Map(); - plugins.forEach((p, i) => { - if (p.package != null) { - packageToIndex.set(p.package, i); - } - }); - - // Build adjacency list: index → indices it depends on - const depIndices = new Map(); - plugins.forEach((p, i) => { - if (p.dependencies != null && p.dependencies.length > 0) { - const resolved: number[] = []; - p.dependencies.forEach(dep => { - const idx = packageToIndex.get(dep); - if (idx != null) { - resolved.push(idx); - } else { - log.warn( - `Plugin '${p.name}' depends on '${dep}' which is not in the manifest` - ); - } - }); - if (resolved.length > 0) { - depIndices.set(i, resolved); - } - } - }); - - // If no plugin has in-manifest dependencies, return original order - if (depIndices.size === 0) { - return [...plugins]; - } - - // Kahn's algorithm for topological sort (stable — preserves original order - // among plugins at the same dependency depth) - const inDegree = new Array(plugins.length).fill(0); - - // Reverse adjacency: who depends on me? - const dependents = new Map(); - depIndices.forEach((deps, idx) => { - deps.forEach(dep => { - if (!dependents.has(dep)) { - dependents.set(dep, []); - } - const depList = dependents.get(dep); - if (depList != null) { - depList.push(idx); - } - inDegree[idx] += 1; - }); - }); - - // Seed queue with all nodes that have no in-manifest dependencies, - // in their original order - const queue: number[] = []; - for (let i = 0; i < plugins.length; i += 1) { - if (inDegree[i] === 0) { - queue.push(i); - } - } - - const sorted: T[] = []; - while (queue.length > 0) { - const idx = queue.shift(); - if (idx == null) { - break; - } - sorted.push(plugins[idx]); - const deps = dependents.get(idx); - if (deps != null) { - // Process dependents in original manifest order for stability - deps.sort((a, b) => a - b); - deps.forEach(depIdx => { - inDegree[depIdx] -= 1; - if (inDegree[depIdx] === 0) { - queue.push(depIdx); - } - }); - } - } - - if (sorted.length !== plugins.length) { - // Find the cycle participants for a useful error message - const inCycle = plugins.filter((_, i) => inDegree[i] > 0).map(p => p.name); - throw new Error( - `Circular plugin dependency detected among: ${inCycle.join(', ')}` - ); - } - - return sorted; -} - function hasDefaultExport(value: unknown): value is { default: Plugin } { return ( typeof value === 'object' && diff --git a/packages/plugin/src/index.ts b/packages/plugin/src/index.ts index 19a07c399a..31664602c1 100644 --- a/packages/plugin/src/index.ts +++ b/packages/plugin/src/index.ts @@ -1,6 +1,7 @@ export * from './PluginsContext'; export * from './PluginTypes'; export * from './PluginUtils'; +export * from './pluginSortUtil'; export * from './TablePlugin'; export * from './useCustomThemes'; export * from './useDashboardPlugins'; diff --git a/packages/plugin/src/pluginSortUtil/index.ts b/packages/plugin/src/pluginSortUtil/index.ts new file mode 100644 index 0000000000..0498914684 --- /dev/null +++ b/packages/plugin/src/pluginSortUtil/index.ts @@ -0,0 +1 @@ +export * from './sortPluginsByDependency'; diff --git a/packages/plugin/src/pluginSortUtil/sortPluginsByDependency.test.ts b/packages/plugin/src/pluginSortUtil/sortPluginsByDependency.test.ts new file mode 100644 index 0000000000..0fb5427e7c --- /dev/null +++ b/packages/plugin/src/pluginSortUtil/sortPluginsByDependency.test.ts @@ -0,0 +1,160 @@ +import { sortPluginsByDependency } from './sortPluginsByDependency'; +import { type PluginManifestPluginInfo } from '../PluginUtils'; + +function makeManifestPlugin( + name: string, + opts?: { + package?: string; + dependencies?: string[]; + } +): PluginManifestPluginInfo { + return { + name, + main: 'index.js', + version: '1.0.0', + package: opts?.package, + dependencies: opts?.dependencies, + }; +} + +describe('sortPluginsByDependency', () => { + it('returns plugins in original order when no dependencies', () => { + const plugins = [ + makeManifestPlugin('a'), + makeManifestPlugin('b'), + makeManifestPlugin('c'), + ]; + + const sorted = sortPluginsByDependency(plugins); + + expect(sorted.map(p => p.name)).toEqual(['a', 'b', 'c']); + }); + + it('reorders so dependency loads before consumer', () => { + const plugins = [ + makeManifestPlugin('consumer', { + dependencies: ['@scope/dep'], + }), + makeManifestPlugin('dep', { + package: '@scope/dep', + }), + ]; + + const sorted = sortPluginsByDependency(plugins); + + expect(sorted.map(p => p.name)).toEqual(['dep', 'consumer']); + }); + + it('handles a chain of dependencies: a → b → c', () => { + const plugins = [ + makeManifestPlugin('c', { + package: '@scope/c', + dependencies: ['@scope/b'], + }), + makeManifestPlugin('b', { + package: '@scope/b', + dependencies: ['@scope/a'], + }), + makeManifestPlugin('a', { + package: '@scope/a', + }), + ]; + + const sorted = sortPluginsByDependency(plugins); + + const names = sorted.map(p => p.name); + expect(names.indexOf('a')).toBeLessThan(names.indexOf('b')); + expect(names.indexOf('b')).toBeLessThan(names.indexOf('c')); + }); + + it('preserves original order among independent plugins', () => { + const plugins = [ + makeManifestPlugin('x'), + makeManifestPlugin('dep', { package: '@scope/dep' }), + makeManifestPlugin('y'), + makeManifestPlugin('consumer', { dependencies: ['@scope/dep'] }), + makeManifestPlugin('z'), + ]; + + const sorted = sortPluginsByDependency(plugins); + + const names = sorted.map(p => p.name); + // dep must come before consumer + expect(names.indexOf('dep')).toBeLessThan(names.indexOf('consumer')); + // independent plugins keep their relative order + expect(names.indexOf('x')).toBeLessThan(names.indexOf('y')); + expect(names.indexOf('y')).toBeLessThan(names.indexOf('z')); + }); + + it('throws on circular dependencies', () => { + const plugins = [ + makeManifestPlugin('a', { + package: '@scope/a', + dependencies: ['@scope/b'], + }), + makeManifestPlugin('b', { + package: '@scope/b', + dependencies: ['@scope/a'], + }), + ]; + + expect(() => sortPluginsByDependency(plugins)).toThrow( + /Circular plugin dependency/ + ); + }); + + it('warns and ignores dependencies not in the manifest', () => { + const plugins = [ + makeManifestPlugin('consumer', { + dependencies: ['@scope/nonexistent'], + }), + ]; + + const sorted = sortPluginsByDependency(plugins); + + expect(sorted.map(p => p.name)).toEqual(['consumer']); + }); + + it('handles multiple dependencies', () => { + const plugins = [ + makeManifestPlugin('consumer', { + dependencies: ['@scope/dep-a', '@scope/dep-b'], + }), + makeManifestPlugin('dep-a', { package: '@scope/dep-a' }), + makeManifestPlugin('dep-b', { package: '@scope/dep-b' }), + ]; + + const sorted = sortPluginsByDependency(plugins); + + const names = sorted.map(p => p.name); + expect(names.indexOf('dep-a')).toBeLessThan(names.indexOf('consumer')); + expect(names.indexOf('dep-b')).toBeLessThan(names.indexOf('consumer')); + }); + + it('does not mutate the input array', () => { + const plugins = [ + makeManifestPlugin('consumer', { dependencies: ['@scope/dep'] }), + makeManifestPlugin('dep', { package: '@scope/dep' }), + ]; + const original = [...plugins]; + + sortPluginsByDependency(plugins); + + expect(plugins).toEqual(original); + }); + + it('handles empty plugin list', () => { + expect(sortPluginsByDependency([])).toEqual([]); + }); + + it('handles plugins with empty dependencies array', () => { + const plugins = [ + makeManifestPlugin('a', { dependencies: [] }), + makeManifestPlugin('b'), + ]; + + const sorted = sortPluginsByDependency(plugins); + + expect(sorted.map(p => p.name)).toEqual(['a', 'b']); + }); +}); diff --git a/packages/plugin/src/pluginSortUtil/sortPluginsByDependency.ts b/packages/plugin/src/pluginSortUtil/sortPluginsByDependency.ts new file mode 100644 index 0000000000..8414d3bcbe --- /dev/null +++ b/packages/plugin/src/pluginSortUtil/sortPluginsByDependency.ts @@ -0,0 +1,109 @@ +import Log from '@deephaven/log'; +import { type PluginManifestPluginInfo } from '../PluginUtils'; + +const log = Log.module('@deephaven/plugin.sortPluginsByDependency'); + +/** + * Topologically sort plugins so that dependencies are loaded before the + * plugins that depend on them. Plugins without dependencies or whose + * dependencies are not in the manifest keep their original relative order + * (stable sort). Throws if a dependency cycle is detected. + * + * @param plugins The plugin list from the manifest + * @returns A new array with plugins sorted so dependencies come first + */ +export function sortPluginsByDependency< + T extends Pick, +>(plugins: readonly T[]): T[] { + // Build a lookup from package name → plugin index + const packageToIndex = new Map(); + plugins.forEach((p, i) => { + if (p.package != null) { + packageToIndex.set(p.package, i); + } + }); + + // Build adjacency list: index → indices it depends on + const depIndices = new Map(); + plugins.forEach((p, i) => { + if (p.dependencies != null && p.dependencies.length > 0) { + const resolved: number[] = []; + p.dependencies.forEach(dep => { + const idx = packageToIndex.get(dep); + if (idx != null) { + resolved.push(idx); + } else { + log.warn( + `Plugin '${p.name}' depends on '${dep}' which is not in the manifest` + ); + } + }); + if (resolved.length > 0) { + depIndices.set(i, resolved); + } + } + }); + + // If no plugin has in-manifest dependencies, return original order + if (depIndices.size === 0) { + return [...plugins]; + } + + // Kahn's algorithm for topological sort (stable — preserves original order + // among plugins at the same dependency depth) + const inDegree = new Array(plugins.length).fill(0); + + // Reverse adjacency: who depends on me? + const dependents = new Map(); + depIndices.forEach((deps, idx) => { + deps.forEach(dep => { + if (!dependents.has(dep)) { + dependents.set(dep, []); + } + const depList = dependents.get(dep); + if (depList != null) { + depList.push(idx); + } + inDegree[idx] += 1; + }); + }); + + // Seed queue with all nodes that have no in-manifest dependencies, + // in their original order + const queue: number[] = []; + for (let i = 0; i < plugins.length; i += 1) { + if (inDegree[i] === 0) { + queue.push(i); + } + } + + const sorted: T[] = []; + while (queue.length > 0) { + const idx = queue.shift(); + if (idx == null) { + break; + } + sorted.push(plugins[idx]); + const deps = dependents.get(idx); + if (deps != null) { + // Process dependents in original manifest order for stability + deps.sort((a, b) => a - b); + deps.forEach(depIdx => { + inDegree[depIdx] -= 1; + if (inDegree[depIdx] === 0) { + queue.push(depIdx); + } + }); + } + } + + if (sorted.length !== plugins.length) { + // Find the cycle participants for a useful error message + const inCycle = plugins.filter((_, i) => inDegree[i] > 0).map(p => p.name); + throw new Error( + `Circular plugin dependency detected among: ${inCycle.join(', ')}` + ); + } + + return sorted; +} From aae7c6b07416d418f2859cf362f35ecb79e34670 Mon Sep 17 00:00:00 2001 From: Vlad Babich Date: Tue, 28 Apr 2026 14:39:08 -0600 Subject: [PATCH 22/25] Eslint --- packages/plugin/src/pluginSortUtil/sortPluginsByDependency.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/plugin/src/pluginSortUtil/sortPluginsByDependency.ts b/packages/plugin/src/pluginSortUtil/sortPluginsByDependency.ts index 8414d3bcbe..90208643c6 100644 --- a/packages/plugin/src/pluginSortUtil/sortPluginsByDependency.ts +++ b/packages/plugin/src/pluginSortUtil/sortPluginsByDependency.ts @@ -12,6 +12,7 @@ const log = Log.module('@deephaven/plugin.sortPluginsByDependency'); * @param plugins The plugin list from the manifest * @returns A new array with plugins sorted so dependencies come first */ +// eslint-disable-next-line import/prefer-default-export export function sortPluginsByDependency< T extends Pick, >(plugins: readonly T[]): T[] { From b8d4bbadf7cc4bda515eb5ab82993191f28e62be Mon Sep 17 00:00:00 2001 From: Vlad Babich Date: Thu, 30 Apr 2026 11:16:09 -0600 Subject: [PATCH 23/25] Move sort plugins to parent --- packages/plugin/src/index.ts | 2 +- packages/plugin/src/pluginSortUtil/index.ts | 1 - .../src/{pluginSortUtil => }/sortPluginsByDependency.test.ts | 2 +- .../plugin/src/{pluginSortUtil => }/sortPluginsByDependency.ts | 2 +- 4 files changed, 3 insertions(+), 4 deletions(-) delete mode 100644 packages/plugin/src/pluginSortUtil/index.ts rename packages/plugin/src/{pluginSortUtil => }/sortPluginsByDependency.test.ts (98%) rename packages/plugin/src/{pluginSortUtil => }/sortPluginsByDependency.ts (98%) diff --git a/packages/plugin/src/index.ts b/packages/plugin/src/index.ts index 31664602c1..1b4d797cca 100644 --- a/packages/plugin/src/index.ts +++ b/packages/plugin/src/index.ts @@ -1,7 +1,7 @@ export * from './PluginsContext'; export * from './PluginTypes'; export * from './PluginUtils'; -export * from './pluginSortUtil'; +export * from './sortPluginsByDependency'; export * from './TablePlugin'; export * from './useCustomThemes'; export * from './useDashboardPlugins'; diff --git a/packages/plugin/src/pluginSortUtil/index.ts b/packages/plugin/src/pluginSortUtil/index.ts deleted file mode 100644 index 0498914684..0000000000 --- a/packages/plugin/src/pluginSortUtil/index.ts +++ /dev/null @@ -1 +0,0 @@ -export * from './sortPluginsByDependency'; diff --git a/packages/plugin/src/pluginSortUtil/sortPluginsByDependency.test.ts b/packages/plugin/src/sortPluginsByDependency.test.ts similarity index 98% rename from packages/plugin/src/pluginSortUtil/sortPluginsByDependency.test.ts rename to packages/plugin/src/sortPluginsByDependency.test.ts index 0fb5427e7c..de76b2fbd3 100644 --- a/packages/plugin/src/pluginSortUtil/sortPluginsByDependency.test.ts +++ b/packages/plugin/src/sortPluginsByDependency.test.ts @@ -1,5 +1,5 @@ import { sortPluginsByDependency } from './sortPluginsByDependency'; -import { type PluginManifestPluginInfo } from '../PluginUtils'; +import { type PluginManifestPluginInfo } from './PluginUtils'; function makeManifestPlugin( name: string, diff --git a/packages/plugin/src/pluginSortUtil/sortPluginsByDependency.ts b/packages/plugin/src/sortPluginsByDependency.ts similarity index 98% rename from packages/plugin/src/pluginSortUtil/sortPluginsByDependency.ts rename to packages/plugin/src/sortPluginsByDependency.ts index 90208643c6..d0e601b7d7 100644 --- a/packages/plugin/src/pluginSortUtil/sortPluginsByDependency.ts +++ b/packages/plugin/src/sortPluginsByDependency.ts @@ -1,5 +1,5 @@ import Log from '@deephaven/log'; -import { type PluginManifestPluginInfo } from '../PluginUtils'; +import { type PluginManifestPluginInfo } from './PluginUtils'; const log = Log.module('@deephaven/plugin.sortPluginsByDependency'); From 54d0b9292dea7bcf26f4088a160a50cb82b51905 Mon Sep 17 00:00:00 2001 From: Vlad Babich Date: Thu, 30 Apr 2026 11:38:38 -0600 Subject: [PATCH 24/25] Address self-review --- package-lock.json | 1 - packages/app-utils/package.json | 1 - .../app-utils/src/plugins/PluginUtils.test.ts | 17 +++++++++++------ packages/plugin/docs/middleware-architecture.md | 9 +++++---- packages/plugin/src/WidgetView.tsx | 12 ------------ 5 files changed, 16 insertions(+), 24 deletions(-) diff --git a/package-lock.json b/package-lock.json index 9dc55022ab..2a3514b2a5 100644 --- a/package-lock.json +++ b/package-lock.json @@ -30003,7 +30003,6 @@ "@deephaven/dashboard-core-plugins": "file:../dashboard-core-plugins", "@deephaven/file-explorer": "file:../file-explorer", "@deephaven/golden-layout": "file:../golden-layout", - "@deephaven/grid": "file:../grid", "@deephaven/icons": "file:../icons", "@deephaven/iris-grid": "file:../iris-grid", "@deephaven/jsapi-bootstrap": "file:../jsapi-bootstrap", diff --git a/packages/app-utils/package.json b/packages/app-utils/package.json index 68ed3584c9..438e1fd81f 100644 --- a/packages/app-utils/package.json +++ b/packages/app-utils/package.json @@ -34,7 +34,6 @@ "@deephaven/dashboard-core-plugins": "file:../dashboard-core-plugins", "@deephaven/file-explorer": "file:../file-explorer", "@deephaven/golden-layout": "file:../golden-layout", - "@deephaven/grid": "file:../grid", "@deephaven/icons": "file:../icons", "@deephaven/iris-grid": "file:../iris-grid", "@deephaven/jsapi-bootstrap": "file:../jsapi-bootstrap", diff --git a/packages/app-utils/src/plugins/PluginUtils.test.ts b/packages/app-utils/src/plugins/PluginUtils.test.ts index 943c430ad8..0b36e797f2 100644 --- a/packages/app-utils/src/plugins/PluginUtils.test.ts +++ b/packages/app-utils/src/plugins/PluginUtils.test.ts @@ -19,6 +19,11 @@ const { default: loadRemoteModule } = require('./loadRemoteModule') as { describe('loadModulePlugins', () => { const BASE_URL = 'http://localhost:4100/plugins'; + // Snapshot the resolve map and global.fetch once so each test starts + // from a known baseline regardless of what previous tests added. + const originalResolveKeys = new Set(Object.keys(resolve)); + const originalFetch = global.fetch; + function makePlugin(name: string): Plugin { return { name, type: PluginType.WIDGET_PLUGIN }; } @@ -34,18 +39,18 @@ describe('loadModulePlugins', () => { beforeEach(() => { jest.clearAllMocks(); - // Clean up any plugin entries added to resolve in previous tests + // Remove any keys added to the shared resolve map by previous tests Object.keys(resolve).forEach(key => { - if ( - key.startsWith('test-plugin') || - key.startsWith('@deephaven/js-plugin-test-plugin') || - key.startsWith('@acme/') - ) { + if (!originalResolveKeys.has(key)) { delete resolve[key]; } }); }); + afterEach(() => { + global.fetch = originalFetch; + }); + it('loads plugins sequentially and registers them in the plugin map', async () => { const pluginA = makePlugin('test-plugin-a'); const pluginB = makePlugin('test-plugin-b'); diff --git a/packages/plugin/docs/middleware-architecture.md b/packages/plugin/docs/middleware-architecture.md index b2037cf3e5..910dc3af8f 100644 --- a/packages/plugin/docs/middleware-architecture.md +++ b/packages/plugin/docs/middleware-architecture.md @@ -172,10 +172,11 @@ const plugins = new Map([ ## Rules 1. **A base plugin is required.** Middleware registered for a type with no base plugin has no effect and produces a warning. -2. **Last base plugin wins.** If multiple non-middleware plugins register for the same type, the last one replaces earlier ones (with a warning). -3. **Middleware must render `Component`.** If a middleware doesn't render the `Component` prop, the rest of the chain (including the base widget) will not appear. -4. **Middleware must spread props.** Pass all received props to `Component` to ensure the base widget and other middleware receive them. -5. **`panelComponent` middleware is separate.** When the base plugin defines a `panelComponent`, only middleware that also defines `panelComponent` is applied. Middleware with only `component` is silently skipped in the panel path — it will have no effect for that widget type. +2. **Last base plugin wins (per widget type).** If multiple non-middleware plugins register for the same widget type, the last one replaces earlier ones (with a warning). This is widget-type resolution in `WidgetLoaderPlugin`, distinct from plugin-name collisions in the plugin map (see below). +3. **First plugin name wins (in the plugin map).** When `registerPlugin` encounters a plugin name that is already in the plugin map, the duplicate is **skipped** and a warning is logged. The first registration is kept. +4. **Middleware must render `Component`.** If a middleware doesn't render the `Component` prop, the rest of the chain (including the base widget) will not appear. +5. **Middleware must spread props.** Pass all received props to `Component` to ensure the base widget and other middleware receive them. +6. **`panelComponent` middleware is separate.** When the base plugin defines a `panelComponent`, only middleware that also defines `panelComponent` is applied. Middleware with only `component` is silently skipped in the panel path — it will have no effect for that widget type. ## Cross-Plugin Dependencies diff --git a/packages/plugin/src/WidgetView.tsx b/packages/plugin/src/WidgetView.tsx index 9636cc7b91..aec878cfd6 100644 --- a/packages/plugin/src/WidgetView.tsx +++ b/packages/plugin/src/WidgetView.tsx @@ -48,14 +48,6 @@ export function WidgetView({ fetch, type }: WidgetViewProps): JSX.Element { } }); - log.debug( - `WidgetView resolved plugins for type ${type}:`, - 'base=', - foundBasePlugin?.name ?? 'none', - 'middleware=', - foundMiddleware.map(m => m.name) - ); - return { basePlugin: foundBasePlugin, middleware: foundMiddleware }; }, [plugins, type]); @@ -68,10 +60,6 @@ export function WidgetView({ fetch, type }: WidgetViewProps): JSX.Element { }, [basePlugin, middleware, type]); if (ChainedComponent != null) { - log.debug( - `Rendering chained component for type ${type}:`, - ChainedComponent.displayName ?? ChainedComponent.name - ); return ; } From 5b3e45f1bd667ae7e646c6a89bb4634434398265 Mon Sep 17 00:00:00 2001 From: Vlad Babich Date: Mon, 4 May 2026 07:49:55 -0600 Subject: [PATCH 25/25] Address review comment --- packages/plugin/src/PluginUtils.tsx | 105 +++++++++++++++++ packages/plugin/src/index.ts | 1 - .../src/sortPluginsByDependency.test.ts | 6 +- .../plugin/src/sortPluginsByDependency.ts | 110 ------------------ 4 files changed, 109 insertions(+), 113 deletions(-) delete mode 100644 packages/plugin/src/sortPluginsByDependency.ts diff --git a/packages/plugin/src/PluginUtils.tsx b/packages/plugin/src/PluginUtils.tsx index c652537c9d..abaa906778 100644 --- a/packages/plugin/src/PluginUtils.tsx +++ b/packages/plugin/src/PluginUtils.tsx @@ -385,3 +385,108 @@ export function processLoadedModule( registerPlugin(pluginMap, name, moduleValue, version); } } + +/** + * Topologically sort plugins so that dependencies are loaded before the + * plugins that depend on them. Plugins without dependencies or whose + * dependencies are not in the manifest keep their original relative order + * (stable sort). Throws if a dependency cycle is detected. + * + * @param plugins The plugin list from the manifest + * @returns A new array with plugins sorted so dependencies come first + */ +export function sortPluginsByDependency< + T extends Pick, +>(plugins: readonly T[]): T[] { + // Build a lookup from package name → plugin index + const packageToIndex = new Map(); + plugins.forEach((p, i) => { + if (p.package != null) { + packageToIndex.set(p.package, i); + } + }); + + // Build adjacency list: index → indices it depends on + const depIndices = new Map(); + plugins.forEach((p, i) => { + if (p.dependencies != null && p.dependencies.length > 0) { + const resolved: number[] = []; + p.dependencies.forEach(dep => { + const idx = packageToIndex.get(dep); + if (idx != null) { + resolved.push(idx); + } else { + log.warn( + `Plugin '${p.name}' depends on '${dep}' which is not in the manifest` + ); + } + }); + if (resolved.length > 0) { + depIndices.set(i, resolved); + } + } + }); + + // If no plugin has in-manifest dependencies, return original order + if (depIndices.size === 0) { + return [...plugins]; + } + + // Kahn's algorithm for topological sort (stable — preserves original order + // among plugins at the same dependency depth) + const inDegree = new Array(plugins.length).fill(0); + + // Reverse adjacency: who depends on me? + const dependents = new Map(); + depIndices.forEach((deps, idx) => { + deps.forEach(dep => { + if (!dependents.has(dep)) { + dependents.set(dep, []); + } + const depList = dependents.get(dep); + if (depList != null) { + depList.push(idx); + } + inDegree[idx] += 1; + }); + }); + + // Seed queue with all nodes that have no in-manifest dependencies, + // in their original order + const queue: number[] = []; + for (let i = 0; i < plugins.length; i += 1) { + if (inDegree[i] === 0) { + queue.push(i); + } + } + + const sorted: T[] = []; + while (queue.length > 0) { + const idx = queue.shift(); + if (idx == null) { + break; + } + sorted.push(plugins[idx]); + const deps = dependents.get(idx); + if (deps != null) { + // Process dependents in original manifest order for stability + deps.sort((a, b) => a - b); + deps.forEach(depIdx => { + inDegree[depIdx] -= 1; + if (inDegree[depIdx] === 0) { + queue.push(depIdx); + } + }); + } + } + + if (sorted.length !== plugins.length) { + // Find the cycle participants for a useful error message + const inCycle = plugins.filter((_, i) => inDegree[i] > 0).map(p => p.name); + throw new Error( + `Circular plugin dependency detected among: ${inCycle.join(', ')}` + ); + } + + return sorted; +} diff --git a/packages/plugin/src/index.ts b/packages/plugin/src/index.ts index 1b4d797cca..19a07c399a 100644 --- a/packages/plugin/src/index.ts +++ b/packages/plugin/src/index.ts @@ -1,7 +1,6 @@ export * from './PluginsContext'; export * from './PluginTypes'; export * from './PluginUtils'; -export * from './sortPluginsByDependency'; export * from './TablePlugin'; export * from './useCustomThemes'; export * from './useDashboardPlugins'; diff --git a/packages/plugin/src/sortPluginsByDependency.test.ts b/packages/plugin/src/sortPluginsByDependency.test.ts index de76b2fbd3..3ea4835395 100644 --- a/packages/plugin/src/sortPluginsByDependency.test.ts +++ b/packages/plugin/src/sortPluginsByDependency.test.ts @@ -1,5 +1,7 @@ -import { sortPluginsByDependency } from './sortPluginsByDependency'; -import { type PluginManifestPluginInfo } from './PluginUtils'; +import { + sortPluginsByDependency, + type PluginManifestPluginInfo, +} from './PluginUtils'; function makeManifestPlugin( name: string, diff --git a/packages/plugin/src/sortPluginsByDependency.ts b/packages/plugin/src/sortPluginsByDependency.ts deleted file mode 100644 index d0e601b7d7..0000000000 --- a/packages/plugin/src/sortPluginsByDependency.ts +++ /dev/null @@ -1,110 +0,0 @@ -import Log from '@deephaven/log'; -import { type PluginManifestPluginInfo } from './PluginUtils'; - -const log = Log.module('@deephaven/plugin.sortPluginsByDependency'); - -/** - * Topologically sort plugins so that dependencies are loaded before the - * plugins that depend on them. Plugins without dependencies or whose - * dependencies are not in the manifest keep their original relative order - * (stable sort). Throws if a dependency cycle is detected. - * - * @param plugins The plugin list from the manifest - * @returns A new array with plugins sorted so dependencies come first - */ -// eslint-disable-next-line import/prefer-default-export -export function sortPluginsByDependency< - T extends Pick, ->(plugins: readonly T[]): T[] { - // Build a lookup from package name → plugin index - const packageToIndex = new Map(); - plugins.forEach((p, i) => { - if (p.package != null) { - packageToIndex.set(p.package, i); - } - }); - - // Build adjacency list: index → indices it depends on - const depIndices = new Map(); - plugins.forEach((p, i) => { - if (p.dependencies != null && p.dependencies.length > 0) { - const resolved: number[] = []; - p.dependencies.forEach(dep => { - const idx = packageToIndex.get(dep); - if (idx != null) { - resolved.push(idx); - } else { - log.warn( - `Plugin '${p.name}' depends on '${dep}' which is not in the manifest` - ); - } - }); - if (resolved.length > 0) { - depIndices.set(i, resolved); - } - } - }); - - // If no plugin has in-manifest dependencies, return original order - if (depIndices.size === 0) { - return [...plugins]; - } - - // Kahn's algorithm for topological sort (stable — preserves original order - // among plugins at the same dependency depth) - const inDegree = new Array(plugins.length).fill(0); - - // Reverse adjacency: who depends on me? - const dependents = new Map(); - depIndices.forEach((deps, idx) => { - deps.forEach(dep => { - if (!dependents.has(dep)) { - dependents.set(dep, []); - } - const depList = dependents.get(dep); - if (depList != null) { - depList.push(idx); - } - inDegree[idx] += 1; - }); - }); - - // Seed queue with all nodes that have no in-manifest dependencies, - // in their original order - const queue: number[] = []; - for (let i = 0; i < plugins.length; i += 1) { - if (inDegree[i] === 0) { - queue.push(i); - } - } - - const sorted: T[] = []; - while (queue.length > 0) { - const idx = queue.shift(); - if (idx == null) { - break; - } - sorted.push(plugins[idx]); - const deps = dependents.get(idx); - if (deps != null) { - // Process dependents in original manifest order for stability - deps.sort((a, b) => a - b); - deps.forEach(depIdx => { - inDegree[depIdx] -= 1; - if (inDegree[depIdx] === 0) { - queue.push(depIdx); - } - }); - } - } - - if (sorted.length !== plugins.length) { - // Find the cycle participants for a useful error message - const inCycle = plugins.filter((_, i) => inDegree[i] > 0).map(p => p.name); - throw new Error( - `Circular plugin dependency detected among: ${inCycle.join(', ')}` - ); - } - - return sorted; -}