fix!: declare that getPlugin can return undefined

[tbouffard]

Problem

BpmnVisualization.getPlugin casts the result of a Map.get to T and declares a non-nullable return type:

getPlugin<T extends Plugin>(id: PluginIds): T {
  return this.plugins.get(id) as T;
}

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 surfaces later, as a TypeError on the first method call, far from its cause.

Fix

Declare T | undefined, so the compiler forces the call site to deal with the missing plugin. The runtime behavior is unchanged: the same value was already returned, it was only mistyped.

The change makes the defect visible where it was hidden. Building the demo, which compiles in strict, produced 14 TS18048 errors across overlays.ts, path-resolver.ts and plugins-by-name.ts: every one of those call sites relied on a guarantee the compiler never had. They now assert non-null, since each registers the plugin it retrieves a few lines above. The tests and the README example do the same.

Also in this PR

• getPlugin gains a JSDoc documenting the returned value, and the ADR states that the lookup returns undefined for an unregistered id.
• A test covers the lookup of an unknown identifier on an instance that does have plugins. The existing test only covered an instance with no plugin at all.
• check-ts-support 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 now validates the GlobalOptions module augmentation, the new return type, and both documented retrieval forms against the lowest supported TypeScript version, 4.5.2, with strict and skipLibCheck: false. Removing the optional chaining there fails with TS2532, so the check does bite. This item was initially planned for the tooling PR of docs: analyze the plugin system against browser extension mechanisms #552 and lands here instead, because it is the natural place to prove the new signature holds on the oldest supported compiler.

Breaking change

getPlugin returns T | undefined instead of T. Consumer code compiled with strictNullChecks that chains directly on the result, such as getPlugin<MyPlugin>('my-plugin').aMethod(), no longer compiles. Add a non-null assertion when the plugin is known to be registered, or handle undefined. Nothing changes at runtime, and code that already handled the value defensively is unaffected.

Found while writing the plugin system analysis in #552, section 2.3.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud