Skip to content

feat(app-utils): support ES module remote plugins with lazy loading - #2764

Draft
mofojed wants to merge 6 commits into
deephaven:mainfrom
mofojed:DH-22885-plugin-modules
Draft

mofojed wants to merge 6 commits into
deephaven:mainfrom
mofojed:DH-22885-plugin-modules

Conversation

@mofojed

@mofojed mofojed commented Sep 22, 2026

Copy link
Copy Markdown
Member

Allow remotely-loaded plugins to be modern ES modules that can code-split
and lazy-load parts of their components, while resolving the same host
singletons (react, @deephaven/*, ...) as the existing CommonJS plugins.

  • Add esmPluginLoader with ESM/CJS auto-detection (es-module-lexer), a
    runtime import map exposing host singletons via blob re-export modules,
    and es-module-shims polyfill loading via importShim.
  • Wire detection/loading into PluginUtils; CommonJS plugins still supported.
  • Add example ESM widget plugin (packages/plugin-example) with a
    lazy-loaded SCSS component and a dependency-free dev server.
  • Add unit tests, README guidance, and a CJS->ESM migration guide.

@mofojed
mofojed requested a balanced review from Copilot September 22, 2026 14:31
@mofojed mofojed self-assigned this Sep 22, 2026
@codecov

codecov Bot commented Sep 22, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 96.96970% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 53.06%. Comparing base (6be18b5) to head (9c5f33d).

Files with missing lines Patch % Lines
packages/app-utils/src/plugins/esmPluginLoader.ts 96.38% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2764      +/-   ##
==========================================
+ Coverage   52.84%   53.06%   +0.21%     
==========================================
  Files         816      817       +1     
  Lines       47388    47483      +95     
  Branches    12245    12261      +16     
==========================================
+ Hits        25044    25195     +151     
+ Misses      22324    22268      -56     
  Partials       20       20              
Flag Coverage Δ
unit 53.06% <96.96%> (+0.21%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Cross-plugin ESM resolution is broken for real manifest shapes, and the example server has a path-traversal vulnerability.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 2 High severity · 2 Medium severity · 4 Low severity

Open (8)
What changed in this PR

Adds ES module remote-plugin loading with lazy chunks while retaining CommonJS support.

Changes:

  • Adds format detection, runtime import maps, and es-module-shims loading.
  • Adds unit coverage and an example lazy-loaded ESM plugin.
  • Documents plugin authoring and migration.
File Description
README.md Documents ESM plugin usage.
plans/​esm-remote-plugins.md Records design and implementation.
packages/​plugin-example/​vite.config.ts Configures the example ESM build.
packages/​plugin-example/​tsconfig.json Configures example TypeScript.
packages/​plugin-example/​src/​vite-env.d.ts Declares style modules.
packages/​plugin-example/​src/​LazyExampleContent.tsx Adds lazy widget content.
packages/​plugin-example/​src/​LazyExampleContent.scss Styles lazy content.
packages/​plugin-example/​src/​index.ts Exports the example plugin.
packages/​plugin-example/​src/​ExampleWidgetView.tsx Demonstrates lazy loading.
packages/​plugin-example/​serve.js Serves plugin assets and manifest.
packages/​plugin-example/​README.md Documents the example workflow.
packages/​plugin-example/​package.json Defines example dependencies and scripts.
packages/​app-utils/​src/​plugins/​PluginUtils.ts Routes ESM and CommonJS loading.
packages/​app-utils/​src/​plugins/​PluginUtils.test.ts Tests integrated loading behavior.
packages/​app-utils/​src/​plugins/​loadRemoteModule.ts Removes the previous remote loader.
packages/​app-utils/​src/​plugins/​loadCommonJsModule.ts Evaluates fetched CommonJS source.
packages/​app-utils/​src/​plugins/​loadCommonJsModule.test.ts Tests CommonJS evaluation.
packages/​app-utils/​src/​plugins/​esmPluginLoader.ts Implements ESM loading and import maps.
packages/​app-utils/​src/​plugins/​esmPluginLoader.test.ts Tests ESM loader utilities.
packages/​app-utils/​src/​declarations.d.ts Declares the shim module.
packages/​app-utils/​package.json Adds ESM loader dependencies.
packages/​app-utils/​docs/​migrating-plugins-to-esm.md Adds migration guidance.
package.json Adds the example startup script.
package-lock.json Locks new workspace dependencies.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread packages/app-utils/src/plugins/esmPluginLoader.ts
Comment thread packages/plugin-example/serve.js Outdated
Comment thread package.json Outdated
Comment thread packages/plugin-example/serve.js Outdated
Comment thread README.md Outdated
Comment thread packages/app-utils/docs/migrating-plugins-to-esm.md Outdated
Comment thread plans/esm-remote-plugins.md Outdated
Comment thread plans/esm-remote-plugins.md Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

CommonJS caching regresses, browser integration remains untested, and several developer-facing instructions are inaccurate.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 3 Low severity

Open (3)
Resolved since last review (8)
Previously missed (4)

In code that hasn't changed since last review

Medium severity Preserve per-URL module caching

packages/​app-utils/​src/​plugins/​PluginUtils.ts:67

Replacing createLoadRemoteModule also removes its URL-level promise cache. Repeated calls for the same CommonJS URL now fetch and evaluate the bundle again, so module-scope plugin side effects can run multiple times; ESM entries remain cached by the module loader. Preserve the previous per-URL caching behavior, with an explicit policy for whether rejected loads are retryable.

Medium severity Add browser coverage for real shim integration

packages/​app-utils/​src/​plugins/​esmPluginLoader.ts:223

The tests mock es-module-shims, so they never exercise the integration this line introduces: loading the real shim, consuming the source hook, resolving host blob modules, rewriting a dynamic import, and fetching its lazy CSS chunk. Add an automated browser test using the example plugin (including shared React and deferred JS/CSS assertions); otherwise the core cross-browser behavior can regress while all unit tests pass.

Medium severity Use cross-platform concurrency for the dev command

packages/​plugin-example/​package.json:18

Using shell & makes the documented dev command platform-dependent: under Windows cmd, the long-running watch command prevents node serve.js from starting. This repository already uses run-p for concurrent scripts (for example, root package.json:42); use that mechanism here so the example remains runnable cross-platform.

Low severity Update the Relevant files entry for the deleted file

plans/​esm-remote-plugins.md:122

This “Relevant files” entry points to a file deleted by this PR and says it is unchanged. Point readers to the new CommonJS evaluator instead.

Comment thread packages/app-utils/docs/migrating-plugins-to-esm.md Outdated
Comment thread plans/esm-remote-plugins.md Outdated
Comment thread plans/esm-remote-plugins.md Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Host named-export mapping and plugin reload behavior can produce module-linking failures and stale dependency URLs.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 1 High severity

Open (1)
Resolved since last review (3)
Previously missed (3)

In code that hasn't changed since last review

Medium severity Refresh plugin mappings when pluginsUrl changes

packages/​app-utils/​src/​plugins/​esmPluginLoader.ts:244

This first-wins registry becomes stale when pluginsUrl changes. PluginsBootstrap explicitly reloads on that prop (PluginsBootstrap.tsx:39-64), so entries are fetched from the new base URL, but cross-plugin bare imports still resolve to the previous base and can mix plugin versions or origins. Support replacing plugin mappings (with shim map overrides configured before initialization) or scope mappings by plugin base URL.

Medium severity Use a portable concurrent process runner instead of shell &

packages/​plugin-example/​package.json:17

Using shell & makes this workspace script non-portable, so the documented example command will not launch both processes on Windows. Repository concurrent scripts use run-p instead (for example, root package.json:42); use the same mechanism here.

Medium severity Include the example package in TypeScript project references

packages/​plugin-example/​tsconfig.json:2

This package is omitted from the repository's TypeScript project-reference tree, so npm run types never checks any of the new example source; Vite only transpiles it. It is also the only package config not extending ../../tsconfig.json, contrary to the invariant documented at root tsconfig.json:20-24. Extend the root config, add references for its workspace dependencies, and add this top-level package to the root references.

Comment thread packages/app-utils/src/plugins/esmPluginLoader.ts
Allow remotely-loaded plugins to be modern ES modules that can code-split
and lazy-load parts of their components, while resolving the same host
singletons (react, @deephaven/*, ...) as the existing CommonJS plugins.

- Add esmPluginLoader with ESM/CJS auto-detection (es-module-lexer), a
  runtime import map exposing host singletons via blob re-export modules,
  and es-module-shims polyfill loading via importShim.
- Wire detection/loading into PluginUtils; CommonJS plugins still supported.
- Add example ESM widget plugin (packages/plugin-example) with a
  lazy-loaded SCSS component and a dependency-free dev server.
- Add unit tests, README guidance, and a CJS->ESM migration guide.
Detect the module format from the fetched source and hand that same source
to es-module-shims (shim mode, source hook) or a CommonJS evaluator, so the
plugin entry is no longer fetched twice. Import maps are registered via
importShim.addImportMap instead of injected script tags.
Address PR review feedback:

- buildPluginImportMap read a top-level `package` field, but the manifest
  contract stores it at `loader.package`, so real manifests produced an empty
  plugin import map and cross-plugin package imports failed to resolve.
- The example plugin manifest emitted `package` at the top level instead of
  `loader.package`.
- The example dev server's traversal guard compared a string prefix, which an
  encoded separator could bypass to reach sibling directories; it now validates
  the dist-relative path.
- `start:plugin-example` matched the `start:*` glob that `npm start` runs, so
  the example server was launched twice and failed with EADDRINUSE. Renamed to
  `plugin-example`.
- Docs described es-module-shims polyfill mode with native passthrough, but the
  loader ships `shimMode: true` and loads every plugin through importShim().

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

CommonJS memoization regresses and the example’s development command is not cross-platform.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 1 Medium severity · 2 Low severity

Open (3)
Resolved since last review (1)

Comment thread packages/app-utils/src/plugins/PluginUtils.ts
Comment thread packages/plugin-example/package.json Outdated
Comment thread packages/plugin-example/src/LazyExampleContent.scss Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🔵 Needs a closer look

ESM mappings become stale when plugin URLs change, and the example package is excluded from repository type checking.

Review effort: Balanced
Findings: None

Resolved since last review (3)
Previously missed (2)

In code that hasn't changed since last review

Medium severity Plugin import mappings cannot update when plugin URLs change

packages/​app-utils/​src/​plugins/​esmPluginLoader.ts:243

pluginImports is process-global and permanently keeps the first URL for each package. PluginsBootstrap explicitly reloads when pluginsUrl changes (packages/app-utils/src/components/PluginsBootstrap.tsx:63), but a second manifest using the same package name will still resolve ESM cross-plugin imports against the old server. Allow plugin-package mappings to be replaced (es-module-shims supports this with mapOverrides) or scope mappings per plugin base URL while continuing to protect host singleton entries.

Low severity Add the package to the root TypeScript project references

packages/​plugin-example/​tsconfig.json:2

This new top-level TypeScript package is not reachable from the root project-reference tree, so npm run types/watch:types never checks any of its source. That conflicts with the explicit root convention in tsconfig.json:23-27; add the package to the root references and give this config the appropriate workspace dependency references so the runnable example cannot silently accumulate type errors.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants