From a6e830a33144c00fd1aff9f9942cba3941afd84e Mon Sep 17 00:00:00 2001 From: Thomas Bouffard <27200110+tbouffard@users.noreply.github.com> Date: Thu, 13 Aug 2026 10:32:14 +0200 Subject: [PATCH 1/2] fix!: declare that getPlugin can return undefined `BpmnVisualization.getPlugin` casts the result of a `Map.get` to `T` and declares a non-nullable return type. The map returns `undefined` for an unknown identifier, so the declared type is a lie: a typo in the id, or a plugin that was never registered, produces `undefined` at runtime while the compiler guarantees an instance. The failure then surfaces later as a `TypeError` on the first method call, far from its cause. Declare `T | undefined` so the compiler forces the call site to deal with the missing plugin. The runtime behavior is unchanged. The demo, the README example and the tests assert non-null at the retrieval sites, since they all register the plugin they retrieve a few lines above. Also document the returned value on the method and in the ADR, and cover the lookup of an unregistered identifier on an instance that does have plugins. `check-ts-support` now registers and retrieves a plugin before its two existing API calls, and imports `BpmnVisualization` from the addons rather than from the core package, which is what the README asks consumers to do. It therefore validates the `GlobalOptions` module augmentation, the new return type and both documented retrieval forms against the lowest supported TypeScript version. None of this was checked there before. BREAKING CHANGE: - `BpmnVisualization.getPlugin` now returns `T | undefined` instead of `T`. Code compiled with `strictNullChecks` that chains directly on the result, such as `getPlugin('my-plugin').aMethod()`, no longer compiles. Add a non-null assertion when the plugin is known to be registered, or handle `undefined`. --- docs/adr/0001-plugin-support.md | 2 +- packages/addons/README.md | 6 ++++-- packages/addons/src/plugins-support.ts | 10 +++++++-- .../addons/test/spec/plugins-support.test.ts | 21 ++++++++++++------- .../test/spec/plugins/css-classes.test.ts | 2 +- .../addons/test/spec/plugins/elements.test.ts | 2 +- .../addons/test/spec/plugins/overlays.test.ts | 18 ++++++++-------- .../addons/test/spec/plugins/style.test.ts | 4 ++-- packages/check-ts-support/src/index.ts | 14 +++++++++---- packages/demo/src/overlays.ts | 2 +- packages/demo/src/path-resolver.ts | 4 ++-- packages/demo/src/plugins-by-name.ts | 2 +- 12 files changed, 53 insertions(+), 34 deletions(-) diff --git a/docs/adr/0001-plugin-support.md b/docs/adr/0001-plugin-support.md index ef71fb57..9db3d267 100644 --- a/docs/adr/0001-plugin-support.md +++ b/docs/adr/0001-plugin-support.md @@ -48,7 +48,7 @@ The plugin system is implemented in `packages/addons/src/plugins-support.ts`: - `onLoadError(error)` runs when a `load` call fails, before the error is rethrown to the caller. Implement it to roll back partial work; it does not swallow the error. - `onDispose()` runs when the instance is disposed, before the underlying core resources are released, so the instance and the BPMN model are still accessible. Implement it to release everything the plugin acquired. - Plugins are passed to the constructor through `options.plugins`. They are instantiated with `(bpmnVisualization, options)` and stored in a per-instance registry. -- Consumers retrieve a plugin with `getPlugin(pluginId)` and then call its methods. +- Consumers retrieve a plugin with `getPlugin(pluginId)` and then call its methods. The lookup returns `undefined` when no plugin is registered with this id. The plugin lifecycle is therefore: diff --git a/packages/addons/README.md b/packages/addons/README.md index fc703d2c..67c5dc65 100644 --- a/packages/addons/README.md +++ b/packages/addons/README.md @@ -53,8 +53,10 @@ const bpmnVisualization = new BpmnVisualization({ container: 'bpmn-container', plugins: [MyPlugin] }); -// Retrieve the plugin by id. The id is defined in the plugin implementation -const myPlugin = bpmnVisualization.getPlugin('my-plugin'); +// Retrieve the plugin by id. The id is defined in the plugin implementation. +// `getPlugin` returns `undefined` when no plugin is registered with this id, hence the non-null assertion here: the +// plugin has just been registered above. +const myPlugin = bpmnVisualization.getPlugin('my-plugin')!; myPlugin.aMethod(); ``` diff --git a/packages/addons/src/plugins-support.ts b/packages/addons/src/plugins-support.ts index dedf4f36..1a7fa743 100644 --- a/packages/addons/src/plugins-support.ts +++ b/packages/addons/src/plugins-support.ts @@ -143,8 +143,14 @@ export class BpmnVisualization extends BaseBpmnVisualization { this.forEachPlugin(plugin => plugin.onLoadSuccess?.()); } - getPlugin(id: PluginIds): T { - return this.plugins.get(id) as T; + /** + * Retrieve a plugin registered on this instance. + * + * @param id The identifier of the plugin, as returned by its {@link Plugin.getPluginId} implementation. + * @returns The plugin registered with this identifier, or `undefined` when no plugin has been registered with it. + */ + getPlugin(id: PluginIds): T | undefined { + return this.plugins.get(id) as T | undefined; } private readonly registerPlugins = (options: GlobalOptions): void => { diff --git a/packages/addons/test/spec/plugins-support.test.ts b/packages/addons/test/spec/plugins-support.test.ts index 972e65cd..0e96bc9b 100644 --- a/packages/addons/test/spec/plugins-support.test.ts +++ b/packages/addons/test/spec/plugins-support.test.ts @@ -40,14 +40,19 @@ class MyCustomPlugin1 implements Plugin { test('Load a typed plugin and use it', () => { const bpmnVisualization = new BpmnVisualization({ container: undefined!, plugins: [MyCustomPlugin1] }); - const plugin = bpmnVisualization.getPlugin('custom-plugin-1'); + const plugin = bpmnVisualization.getPlugin('custom-plugin-1')!; expect(plugin).toBeInstanceOf(MyCustomPlugin1); expect(plugin.doSomethingSpecial()).toBe(5); }); +test('Retrieve a plugin with an identifier that is not registered', () => { + const bpmnVisualization = new BpmnVisualization({ container: undefined!, plugins: [MyCustomPlugin1] }); + expect(bpmnVisualization.getPlugin('unknown')).toBeUndefined(); +}); + test('Load a untyped plugin and use it', () => { const bpmnVisualization = new BpmnVisualization({ container: undefined!, plugins: [MyCustomPlugin1] }); - const plugin = bpmnVisualization.getPlugin('custom-plugin-1'); + const plugin = bpmnVisualization.getPlugin('custom-plugin-1')!; expect(plugin).toBeInstanceOf(MyCustomPlugin1); expect(plugin.getPluginId()).toBe('custom-plugin-1'); expect((plugin as MyCustomPlugin1).doSomethingSpecial()).toBe(5); @@ -67,7 +72,7 @@ test('Load several plugins and use them', () => { const plugin1 = bpmnVisualization.getPlugin('custom-plugin-1'); expect(plugin1).toBeInstanceOf(MyCustomPlugin1); - const plugin2 = bpmnVisualization.getPlugin('custom-plugin-2'); + const plugin2 = bpmnVisualization.getPlugin('custom-plugin-2')!; expect(plugin2).toBeInstanceOf(MyCustomPlugin2); expect(plugin2.doSomethingSpecial()).toBe('I am awesome'); }); @@ -141,7 +146,7 @@ describe('Ensure that plugins are configured', () => { test('Ensure that the configurable plugin is configured after BpmnVisualization initialization', () => { const bpmnVisualization = new BpmnVisualization({ container: undefined!, customValue: 'custom in options', plugins: [ConfigurablePlugin] } as CustomGlobalOptions); - const configurablePlugin = bpmnVisualization.getPlugin('custom-configurable-plugin'); + const configurablePlugin = bpmnVisualization.getPlugin('custom-configurable-plugin')!; expect(configurablePlugin.isConfigured).toBeTruthy(); expect(configurablePlugin.customValue).toBe('custom in options'); // ensure that the options are passed to the plugin configuration }); @@ -176,8 +181,8 @@ describe('Ensure that plugins are disposed', () => { test('Call onDispose on plugins that implement it and ignore the others when disposing BpmnVisualization', () => { const bpmnVisualization = new BpmnVisualization({ container: undefined!, plugins: [DisposablePlugin1, PluginWithoutOptionalMethods, DisposablePlugin2] }); - const disposablePlugin1 = bpmnVisualization.getPlugin('custom-disposable-plugin-1'); - const disposablePlugin2 = bpmnVisualization.getPlugin('custom-disposable-plugin-2'); + const disposablePlugin1 = bpmnVisualization.getPlugin('custom-disposable-plugin-1')!; + const disposablePlugin2 = bpmnVisualization.getPlugin('custom-disposable-plugin-2')!; expect(() => bpmnVisualization.dispose()).not.toThrow(); expect(disposablePlugin1.onDispose).toHaveBeenCalledTimes(1); @@ -212,8 +217,8 @@ const setupLoadAwareVisualization = (): { bpmnVisualization: BpmnVisualization; const bpmnVisualization = new BpmnVisualization({ container: insertBpmnContainerWithoutId(), plugins: [LoadAwarePlugin1, PluginWithoutOptionalMethods, LoadAwarePlugin2] }); return { bpmnVisualization, - loadAwarePlugin1: bpmnVisualization.getPlugin('custom-load-aware-plugin-1'), - loadAwarePlugin2: bpmnVisualization.getPlugin('custom-load-aware-plugin-2'), + loadAwarePlugin1: bpmnVisualization.getPlugin('custom-load-aware-plugin-1')!, + loadAwarePlugin2: bpmnVisualization.getPlugin('custom-load-aware-plugin-2')!, }; }; diff --git a/packages/addons/test/spec/plugins/css-classes.test.ts b/packages/addons/test/spec/plugins/css-classes.test.ts index 3a3d9aef..0704da4e 100644 --- a/packages/addons/test/spec/plugins/css-classes.test.ts +++ b/packages/addons/test/spec/plugins/css-classes.test.ts @@ -36,7 +36,7 @@ beforeEach(() => { // The actual implementation is in `bpmn-visualization`. Here, we only validate that the `bpmn-visualization` code is called. describe('CssClassesPlugin', () => { const bpmnVisualization = new BpmnVisualization({ container: insertBpmnContainerWithoutId(), plugins: [CssClassesPlugin] }); - const cssClassesPlugin = bpmnVisualization.getPlugin('css'); + const cssClassesPlugin = bpmnVisualization.getPlugin('css')!; describe('addCssClasses', () => { test('Pass a single id', () => { diff --git a/packages/addons/test/spec/plugins/elements.test.ts b/packages/addons/test/spec/plugins/elements.test.ts index 1002b4eb..b1d49619 100644 --- a/packages/addons/test/spec/plugins/elements.test.ts +++ b/packages/addons/test/spec/plugins/elements.test.ts @@ -25,7 +25,7 @@ import { readFileSync } from '../../shared/io-utilities.js'; describe('Check ElementsPlugin methods', () => { const bpmnVisualization = new BpmnVisualization({ container: insertBpmnContainerWithoutId(), plugins: [ElementsPlugin] }); bpmnVisualization.load(readFileSync('./fixtures/bpmn/search-elements.bpmn')); - const elementsPlugin = bpmnVisualization.getPlugin('elements'); + const elementsPlugin = bpmnVisualization.getPlugin('elements')!; test('getElementsByIds', () => { const bpmnElements = elementsPlugin.getElementsByIds('Gateway_0t7d2lu'); diff --git a/packages/addons/test/spec/plugins/overlays.test.ts b/packages/addons/test/spec/plugins/overlays.test.ts index 5e7cc837..ddfa612a 100644 --- a/packages/addons/test/spec/plugins/overlays.test.ts +++ b/packages/addons/test/spec/plugins/overlays.test.ts @@ -89,7 +89,7 @@ describe('setVisible', () => { const bpmnVisualization = new BpmnVisualization({ container: insertBpmnContainerWithoutId(), plugins: [OverlaysPlugin] }); bpmnVisualization.load(readFileSync('./fixtures/bpmn/1_pool_custom_colors_with_1_text-annotation.bpmn')); - const plugin = bpmnVisualization.getPlugin('overlays'); + const plugin = bpmnVisualization.getPlugin('overlays')!; plugin.setVisible(false); expect(new ContainersRetriever(bpmnVisualization).getOverlaysContainer()).not.toBeVisible(); }); @@ -98,7 +98,7 @@ describe('setVisible', () => { const bpmnVisualization = new BpmnVisualization({ container: insertBpmnContainerWithoutId(), plugins: [OverlaysPlugin] }); bpmnVisualization.load(readFileSync('./fixtures/bpmn/1_pool_custom_colors_with_1_text-annotation.bpmn')); - const plugin = bpmnVisualization.getPlugin('overlays'); + const plugin = bpmnVisualization.getPlugin('overlays')!; plugin.setVisible(false); plugin.setVisible(); const overlaysContainer = new ContainersRetriever(bpmnVisualization).getOverlaysContainer(); @@ -112,7 +112,7 @@ describe('setVisible', () => { overlaysContainer.style.display = 'inherit'; expect(overlaysContainer).toHaveStyle('display: inherit'); - const plugin = bpmnVisualization.getPlugin('overlays'); + const plugin = bpmnVisualization.getPlugin('overlays')!; plugin.setVisible(false); plugin.setVisible(); expect(overlaysContainer).toHaveStyle('display: inherit'); @@ -124,7 +124,7 @@ describe('setVisible', () => { overlaysContainer.style.display = 'inherit'; expect(overlaysContainer).toHaveStyle('display: inherit'); - const plugin = bpmnVisualization.getPlugin('overlays'); + const plugin = bpmnVisualization.getPlugin('overlays')!; plugin.setVisible(false); plugin.setVisible(false); plugin.setVisible(); @@ -137,7 +137,7 @@ describe('setVisible', () => { overlaysContainer.style.display = 'inherit'; expect(overlaysContainer).toHaveStyle('display: inherit'); - const plugin = bpmnVisualization.getPlugin('overlays'); + const plugin = bpmnVisualization.getPlugin('overlays')!; plugin.setVisible(); expect(overlaysContainer).toHaveStyle('display: inherit'); }); @@ -146,7 +146,7 @@ describe('setVisible', () => { const bpmnVisualization = new BpmnVisualization({ container: insertBpmnContainerWithoutId(), plugins: [OverlaysPlugin] }); bpmnVisualization.load(readFileSync('./fixtures/bpmn/1_pool_custom_colors_with_1_text-annotation.bpmn')); - const plugin = bpmnVisualization.getPlugin('overlays'); + const plugin = bpmnVisualization.getPlugin('overlays')!; plugin.setVisible(false); plugin.setVisible(false); expect(new ContainersRetriever(bpmnVisualization).getOverlaysContainer()).not.toBeVisible(); @@ -156,7 +156,7 @@ describe('setVisible', () => { const bpmnVisualization = new BpmnVisualization({ container: insertBpmnContainerWithoutId(), plugins: [OverlaysPlugin] }); bpmnVisualization.load(readFileSync('./fixtures/bpmn/1_pool_custom_colors_with_1_text-annotation.bpmn')); - const plugin = bpmnVisualization.getPlugin('overlays'); + const plugin = bpmnVisualization.getPlugin('overlays')!; plugin.setVisible(); plugin.setVisible(true); plugin.setVisible(); @@ -167,7 +167,7 @@ describe('setVisible', () => { const bpmnVisualization = new BpmnVisualization({ container: insertBpmnContainerWithoutId(), plugins: [OverlaysPlugin] }); bpmnVisualization.load(readFileSync('./fixtures/bpmn/1_pool_custom_colors_with_1_text-annotation.bpmn')); - const plugin = bpmnVisualization.getPlugin('overlays'); + const plugin = bpmnVisualization.getPlugin('overlays')!; plugin.setVisible(false); plugin.setVisible(); plugin.setVisible(false); @@ -216,7 +216,7 @@ function createOverlay(label: string): Overlay { describe('Add and remove Overlays', () => { const bpmnVisualization = new BpmnVisualization({ container: insertBpmnContainerWithoutId(), plugins: [OverlaysPlugin] }); - const overlaysPlugin = bpmnVisualization.getPlugin('overlays'); + const overlaysPlugin = bpmnVisualization.getPlugin('overlays')!; const overlaysExpectation = new OverlaysExpectation(bpmnVisualization); beforeEach(() => { diff --git a/packages/addons/test/spec/plugins/style.test.ts b/packages/addons/test/spec/plugins/style.test.ts index 3fb798ee..863c62b8 100644 --- a/packages/addons/test/spec/plugins/style.test.ts +++ b/packages/addons/test/spec/plugins/style.test.ts @@ -33,7 +33,7 @@ beforeEach(() => { // The actual implementation is in `bpmn-visualization`. Here, we only validate that the `bpmn-visualization` code is called. describe('StylePlugin', () => { const bpmnVisualization = new BpmnVisualization({ container: insertBpmnContainerWithoutId(), plugins: [StylePlugin] }); - const stylePlugin = bpmnVisualization.getPlugin('style'); + const stylePlugin = bpmnVisualization.getPlugin('style')!; describe('updateStyle', () => { test('Pass a single id', () => { @@ -79,7 +79,7 @@ describe('StylePlugin', () => { describe('StyleByNamePlugin', () => { const bpmnVisualization = new BpmnVisualization({ container: insertBpmnContainerWithoutId(), plugins: [StyleByNamePlugin] }); bpmnVisualization.load(readFileSync('./fixtures/bpmn/search-elements.bpmn')); - const styleByNamePlugin = bpmnVisualization.getPlugin('style-by-name'); + const styleByNamePlugin = bpmnVisualization.getPlugin('style-by-name')!; describe('updateStyle', () => { test('Pass a single name related to an existing element', () => { diff --git a/packages/check-ts-support/src/index.ts b/packages/check-ts-support/src/index.ts index 90f16ab3..17b8dbce 100644 --- a/packages/check-ts-support/src/index.ts +++ b/packages/check-ts-support/src/index.ts @@ -14,14 +14,20 @@ See the License for the specific language governing permissions and limitations under the License. */ -import { BpmnElementsIdentifier, PathResolver } from '@process-analytics/bpmn-visualization-addons'; -import { BpmnVisualization } from 'bpmn-visualization'; +import { BpmnElementsIdentifier, BpmnVisualization, CssClassesPlugin, PathResolver } from '@process-analytics/bpmn-visualization-addons'; -// bpmn-visualization -const bpmnVisualization = new BpmnVisualization({ container: 'bpmn-container' }); +// bpmn-visualization, through the BpmnVisualization subclass provided by the addons. Importing it from +// `bpmn-visualization` would also compile, but would not provide plugin support. +// The `plugins` property comes from the module augmentation of `GlobalOptions`, which is only checked here. +const bpmnVisualization = new BpmnVisualization({ container: 'bpmn-container', plugins: [CssClassesPlugin] }); bpmnVisualization.load(`fake BPMN content`); const bpmnElementsRegistry = bpmnVisualization.bpmnElementsRegistry; +// addons: plugin retrieval, in both forms documented in the README +const cssClassesPlugin = bpmnVisualization.getPlugin('css'); +cssClassesPlugin?.addCssClasses('id_1', 'class_1'); +bpmnVisualization.getPlugin('css')!.addCssClasses('id_2', 'class_2'); + // addons const bpmnElementsIdentifier = new BpmnElementsIdentifier(bpmnElementsRegistry); bpmnElementsIdentifier.isActivity('id_1'); diff --git a/packages/demo/src/overlays.ts b/packages/demo/src/overlays.ts index 1ba1901d..691f8385 100644 --- a/packages/demo/src/overlays.ts +++ b/packages/demo/src/overlays.ts @@ -35,7 +35,7 @@ const fitOptions: FitOptions = { type: FitType.Center, margin: 20 }; bpmnVisualization.load(diagram, { fit: fitOptions }); // Add overlays -const overlaysPlugin = bpmnVisualization.getPlugin('overlays'); +const overlaysPlugin = bpmnVisualization.getPlugin('overlays')!; const overlayStyle = { stroke: { color: 'chartreuse' }, fill: { color: 'chartreuse' }, font: { color: 'white', size: 18 } }; // SRM subprocess overlaysPlugin.addOverlays('Activity_0ec8azh', { label: '123', position: 'top-center', style: overlayStyle }); diff --git a/packages/demo/src/path-resolver.ts b/packages/demo/src/path-resolver.ts index ef4563a0..38dea271 100644 --- a/packages/demo/src/path-resolver.ts +++ b/packages/demo/src/path-resolver.ts @@ -30,8 +30,8 @@ const bpmnVisualization = new BpmnVisualization({ // Load the BPMN diagram defined above const diagram = await fetchDiagram(); bpmnVisualization.load(diagram, { fit: { type: FitType.Center, margin: 20 } }); -const elementsPlugin = bpmnVisualization.getPlugin('elements'); -const stylePlugin = bpmnVisualization.getPlugin('style'); +const elementsPlugin = bpmnVisualization.getPlugin('elements')!; +const stylePlugin = bpmnVisualization.getPlugin('style')!; const pathResolver = new PathResolver(elementsPlugin); diff --git a/packages/demo/src/plugins-by-name.ts b/packages/demo/src/plugins-by-name.ts index f4ed679e..716f4e62 100644 --- a/packages/demo/src/plugins-by-name.ts +++ b/packages/demo/src/plugins-by-name.ts @@ -39,7 +39,7 @@ bpmnVisualization.load(diagram, { fit: fitOptions }); new ZoomComponent(bpmnVisualization, fitOptions).render(); // Use style by name plugin to update the style of the elements -const styleRegistryByName = bpmnVisualization.getPlugin('style-by-name'); +const styleRegistryByName = bpmnVisualization.getPlugin('style-by-name')!; function clearAllStyles(): void { styleRegistryByName.resetStyle(); From 6a4f7b30f98d9d4165fb2db591d4c2788896f2c7 Mon Sep 17 00:00:00 2001 From: Thomas Bouffard <27200110+tbouffard@users.noreply.github.com> Date: Tue, 18 Aug 2026 17:14:31 +0200 Subject: [PATCH 2/2] docs: correct two statements about getPlugin and the module augmentation Follow-up of the review of this branch. `CLAUDE.md` still described `getPlugin` without its nullable return, while the README and the ADR were updated. It is the instruction source for automated changes in this repository, so a stale statement there propagates to the next generated call site. The comment in `check-ts-support` claimed that the `GlobalOptions` module augmentation is only checked there. The demo compiles with `tsc` and passes `plugins` as well, so it checks the augmentation too. What is specific to `check-ts-support` is the TypeScript version it checks it against. --- CLAUDE.md | 2 +- packages/check-ts-support/src/index.ts | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 78cab66f..232bdbc7 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -64,7 +64,7 @@ The central architectural concept is the **plugin system**. This package extends - Each plugin is constructed with `(bpmnVisualization, options)` parameters - Plugins must implement `getPluginId()` to return a unique identifier - Plugins can optionally implement the `onConfigure(options)` lifecycle hook for post-construction setup (called by `BpmnVisualization`, not by client code) - - Retrieve plugins using `bpmnVisualization.getPlugin(pluginId)` + - Retrieve plugins using `bpmnVisualization.getPlugin(pluginId)`, which returns `undefined` when no plugin is registered with this identifier 3. **Available Plugins** (in `packages/addons/src/plugins/`): - `CssClassesPlugin`: Manipulate CSS classes on BPMN elements diff --git a/packages/check-ts-support/src/index.ts b/packages/check-ts-support/src/index.ts index 17b8dbce..bec764c1 100644 --- a/packages/check-ts-support/src/index.ts +++ b/packages/check-ts-support/src/index.ts @@ -18,7 +18,8 @@ import { BpmnElementsIdentifier, BpmnVisualization, CssClassesPlugin, PathResolv // bpmn-visualization, through the BpmnVisualization subclass provided by the addons. Importing it from // `bpmn-visualization` would also compile, but would not provide plugin support. -// The `plugins` property comes from the module augmentation of `GlobalOptions`, which is only checked here. +// The `plugins` property comes from the module augmentation of `GlobalOptions`. The demo checks it too, but only +// here is it checked against the lowest supported TypeScript version. const bpmnVisualization = new BpmnVisualization({ container: 'bpmn-container', plugins: [CssClassesPlugin] }); bpmnVisualization.load(`fake BPMN content`); const bpmnElementsRegistry = bpmnVisualization.bpmnElementsRegistry;