Skip to content

refactor: load client scripts with @JsModule instead of executeJs - #412

Open
totally-not-ai[bot] wants to merge 8 commits into
mainfrom
refactor/load-client-scripts-as-files
Open

totally-not-ai[bot] wants to merge 8 commits into
mainfrom
refactor/load-client-scripts-as-files

Conversation

@totally-not-ai

@totally-not-ai totally-not-ai Bot commented Sep 25, 2026 •

Copy link
Copy Markdown
Contributor

Summary

The browser metrics collector and the dev-mode Copilot panel now load as normal frontend modules through @JsModule. Before, they were read from the classpath, stripped of comments and sent with executeJs. The Copilot panel also now appears when the kit has no license, with a notice that explains why its sections are empty.

What changed

Breaking: ClientResourceLoader (a public class) is removed. Code that called ClientResourceLoader.loadOnce(...) no longer compiles.

Behavior change: In development mode, the Copilot "Observability" panel is now always added, even without a valid license. Before, it only appeared when the license check passed. When unlicensed, it shows a license notice and "Not collected without a license." in place of the insights and meters. This affects all dev-mode users without a license.

Behavior change: VaadinMetricsClient.js is now part of the frontend bundle, including production bundles. Loading the module only defines <vaadin-metrics-collector>. Collection starts only when the server attaches that element, which it still does only when client metrics are enabled.

  • MetricsCollectorElement now declares @JsModule("./VaadinMetricsClient.js") and no longer overrides onAttach.
  • The server now sends the "collect error messages" flag as a details attribute on the collector element. It no longer sets the window.__vaadinMicrometerDetails global.
  • ObservabilityDevToolsHandler declares the Copilot panel as a development-only @JsModule. This replaces the per-UI injection that MetricsServiceInitListener added. The service now registers one fewer UIInitListener.
  • MetricsServiceInitListener records the result of the license check. The dev-tools meter snapshot now includes a licensed field.
  • ObservabilityDevToolsClient (package-private) and the comment-stripping integrity test are removed. The panel script no longer has to avoid regex literals and URLs.

Use case

An application has an internal admin/health view. The team wants to warn operators when Observability Kit is installed but not collecting anything because the license is missing:

if (ObservabilityKit.isLicenseMissing()) {
    add(new Span("Observability Kit has no valid license - no metrics are collected."));
}

API Changes

com.vaadin.observability.micrometer.ClientResourceLoader

// Removed
public final class ClientResourceLoader
public static void loadOnce(UI ui, String initKey, String resource, Class<?> owner)
public static void loadOnce(UI ui, String initKey, String resource, Class<?> owner, String prelude)

com.vaadin.observability.micrometer.ObservabilityKit

// Added
public static boolean isLicenseMissing() // true if the last license check at serviceInit failed

com.vaadin.observability.micrometer.client.MetricsCollectorElement

// Removed
protected void onAttach(AttachEvent event) // script now loaded via @JsModule

// Changed
- public final class MetricsCollectorElement extends Component
+ @JsModule("./VaadinMetricsClient.js") public final class MetricsCollectorElement extends Component // annotation added

com.vaadin.observability.micrometer.devtools.ObservabilityDevToolsHandler

// Changed
- public class ObservabilityDevToolsHandler implements DevToolsMessageHandler
+ @JsModule(value = "./VaadinObservabilityDevTools.js", developmentOnly = true) public class ObservabilityDevToolsHandler implements DevToolsMessageHandler // annotation added

Test summary

License check (service init):

  • Missing-license flag set when the license is invalid
  • Flag cleared by a valid license
  • No extra per-UI listener for the dev-tools panel

Dev-tools handler:

  • Still answers when unlicensed, with licensed=false and no meters

Collector element:

  • details attribute set only when insights details are on and client errors are kept

Collector script:

  • Loading the module defines the element but installs nothing
  • Element not defined twice on a second load
  • Collector installs and listens for errors once the element is attached
  • Error messages follow the element's details attribute

Copilot panel script:

  • Panel added without waiting for the server
  • License notice shown when unlicensed; not shown when licensed
  • Unlicensed text does not blame the insights setting or ask the user to interact with the app

totally-not-ai Bot and others added 3 commits September 25, 2026 13:03
The collector and the dev-mode Copilot panel scripts are currently read
from the classpath and sent as executeJs text. Capture that both should
reach the browser as regular runtime-loaded file dependencies.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
The in-browser collector and the dev-mode Copilot panel were read from
the classpath, comment-stripped and sent as executeJs text by
ClientResourceLoader. Load them as regular frontend modules instead:

- MetricsCollectorElement declares @jsmodule for VaadinMetricsClient.js.
  The module defines <vaadin-metrics-collector> and only starts
  collecting when the server attaches the element.
- The error-message flag is the element's details attribute rather than
  a window global set by a script prelude.
- ObservabilityDevToolsHandler declares the Copilot panel as a
  development-only @jsmodule, replacing the per-UI injection.
- ClientResourceLoader, ObservabilityDevToolsClient and the
  comment-stripping integrity test are removed.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
* frontend bundle, like any other Flow dependency, not as script text sent
* through {@code executeJs}.
*/
class ClientScriptLoadingTest {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

All these tests seem pretty pointless

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@Artur- Agreed, removed ClientScriptLoadingTest. It only restated the @JsModule annotations and the details attribute. The ITs (StandaloneMetricsIT, ClientProblemsIT, ClientTimingIT) run against a production bundle, so they already cover the scripts actually being bundled and collecting. The JS suite keeps the check that loading the module alone installs nothing.

@Artur-
Artur- marked this pull request as ready for review September 25, 2026 13:44
totally-not-ai Bot and others added 2 commits September 25, 2026 13:47
It only restated the @jsmodule annotations and the details attribute;
the ITs cover the scripts actually loading and collecting.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
* The panel itself is the development-only module declared here, which the
* frontend build picks up because this class is a dev-tools entry point.
*/
@JsModule(value = "./VaadinObservabilityDevTools.js", developmentOnly = true)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Before this change the panel script only reached the browser after serviceInit had passed the license and configuration checks. Now the module is bundled and registers itself with Copilot on every dev-mode page load, and the handler is always registered through the service loader. So an unlicensed or unconfigured app still gets the Observability panel in Copilot, polling an empty registry and showing "instrumentation: inactive".

If we want to keep the old behaviour (no license, no panel), the panel could ask the handler whether the kit is active before adding itself. Or the handler could answer with nothing when there is no active registry, and the panel could stay hidden in that case.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@heruan Good catch, this now keeps the old "no license, no panel" behaviour. I combined your two suggestions:

  • ObservabilityDevToolsHandler stays silent until the kit has an active registry, i.e. it is licensed and serviceInit has run. It still claims its commands, so nothing gets queued for other handlers.
  • The panel script no longer calls addPanel in init. It adds itself to Copilot, once, when the first metrics or insights answer arrives. It keeps polling, so if the app restarts in the same tab with a valid license, the panel shows up.

An unlicensed or unconfigured app now gets no Observability entry in Copilot, and never sees instrumentation: inactive from an unbound kit. Tests: kitNotActive_answersNothingSoThePanelStaysHidden in ObservabilityDevToolsHandlerTest, and a case in VaadinObservabilityDevTools.test.js checking there is no panel before an answer and exactly one after.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Would it be better if the panel always showed up and even had the correct content but all numbers would be missing + there would be a text about needing a license for it

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@Artur- Agreed, done. The panel is back to always registering with Copilot, and the "wait for the first answer" gating is gone. What changed:

  • MetricsServiceInitListener records a failed license check in ObservabilityKit.isLicenseMissing(), a new public getter next to getActiveMeterRegistry().
  • The meter snapshot the handler sends now includes licensed.
  • When licensed is false, the panel shows a notice at the top: "Observability Kit needs a license. Without one, no metrics or insights are collected", with a link to the license terms. The Insights and Metrics sections keep their usual layout but read "Not collected without a license." That replaces the hints about the insights setting and about interacting with the app, which would be misleading here.

Tests: licenseMissing_stillAnswersAndSaysSo in ObservabilityDevToolsHandlerTest, an assertion in the license test that the flag is recorded, and a JS case checking the notice appears when unlicensed and not otherwise.

totally-not-ai Bot and others added 3 commits September 25, 2026 16:33
The panel module is now in every development bundle with the kit, so it
no longer depends on serviceInit having passed the license check. The
dev-tools handler now stays silent until a registry is bound, and the
panel only adds itself to Copilot once the server has answered.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Rather than hiding the panel while the kit is not active, always add
it. serviceInit records a failed license check, the meter snapshot
carries it as licensed=false, and the panel shows a notice with empty
sections instead of reading like an idle application.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
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