Skip to content

Commit 54d0b92

Browse files
committed
Address self-review
1 parent b8d4bba commit 54d0b92

5 files changed

Lines changed: 16 additions & 24 deletions

File tree

package-lock.json

Lines changed: 0 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/app-utils/package.json

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,6 @@
3434
"@deephaven/dashboard-core-plugins": "file:../dashboard-core-plugins",
3535
"@deephaven/file-explorer": "file:../file-explorer",
3636
"@deephaven/golden-layout": "file:../golden-layout",
37-
"@deephaven/grid": "file:../grid",
3837
"@deephaven/icons": "file:../icons",
3938
"@deephaven/iris-grid": "file:../iris-grid",
4039
"@deephaven/jsapi-bootstrap": "file:../jsapi-bootstrap",

packages/app-utils/src/plugins/PluginUtils.test.ts

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,11 @@ const { default: loadRemoteModule } = require('./loadRemoteModule') as {
1919
describe('loadModulePlugins', () => {
2020
const BASE_URL = 'http://localhost:4100/plugins';
2121

22+
// Snapshot the resolve map and global.fetch once so each test starts
23+
// from a known baseline regardless of what previous tests added.
24+
const originalResolveKeys = new Set(Object.keys(resolve));
25+
const originalFetch = global.fetch;
26+
2227
function makePlugin(name: string): Plugin {
2328
return { name, type: PluginType.WIDGET_PLUGIN };
2429
}
@@ -34,18 +39,18 @@ describe('loadModulePlugins', () => {
3439

3540
beforeEach(() => {
3641
jest.clearAllMocks();
37-
// Clean up any plugin entries added to resolve in previous tests
42+
// Remove any keys added to the shared resolve map by previous tests
3843
Object.keys(resolve).forEach(key => {
39-
if (
40-
key.startsWith('test-plugin') ||
41-
key.startsWith('@deephaven/js-plugin-test-plugin') ||
42-
key.startsWith('@acme/')
43-
) {
44+
if (!originalResolveKeys.has(key)) {
4445
delete resolve[key];
4546
}
4647
});
4748
});
4849

50+
afterEach(() => {
51+
global.fetch = originalFetch;
52+
});
53+
4954
it('loads plugins sequentially and registers them in the plugin map', async () => {
5055
const pluginA = makePlugin('test-plugin-a');
5156
const pluginB = makePlugin('test-plugin-b');

packages/plugin/docs/middleware-architecture.md

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -172,10 +172,11 @@ const plugins = new Map([
172172
## Rules
173173

174174
1. **A base plugin is required.** Middleware registered for a type with no base plugin has no effect and produces a warning.
175-
2. **Last base plugin wins.** If multiple non-middleware plugins register for the same type, the last one replaces earlier ones (with a warning).
176-
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.
177-
4. **Middleware must spread props.** Pass all received props to `Component` to ensure the base widget and other middleware receive them.
178-
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.
175+
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).
176+
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.
177+
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.
178+
5. **Middleware must spread props.** Pass all received props to `Component` to ensure the base widget and other middleware receive them.
179+
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.
179180

180181
## Cross-Plugin Dependencies
181182

packages/plugin/src/WidgetView.tsx

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -48,14 +48,6 @@ export function WidgetView({ fetch, type }: WidgetViewProps): JSX.Element {
4848
}
4949
});
5050

51-
log.debug(
52-
`WidgetView resolved plugins for type ${type}:`,
53-
'base=',
54-
foundBasePlugin?.name ?? 'none',
55-
'middleware=',
56-
foundMiddleware.map(m => m.name)
57-
);
58-
5951
return { basePlugin: foundBasePlugin, middleware: foundMiddleware };
6052
}, [plugins, type]);
6153

@@ -68,10 +60,6 @@ export function WidgetView({ fetch, type }: WidgetViewProps): JSX.Element {
6860
}, [basePlugin, middleware, type]);
6961

7062
if (ChainedComponent != null) {
71-
log.debug(
72-
`Rendering chained component for type ${type}:`,
73-
ChainedComponent.displayName ?? ChainedComponent.name
74-
);
7563
return <ChainedComponent fetch={fetch} />;
7664
}
7765

0 commit comments

Comments
 (0)