Skip to content

fix: keep dev bundle sourcemaps usable in the usage statistics Vite plugin - #25708

Merged
Artur- merged 7 commits into
mainfrom
fix/dev-bundle-sourcemaps-usage-stats-plugin
Sep 15, 2026
Merged

Artur- merged 7 commits into
mainfrom
fix/dev-bundle-sourcemaps-usage-stats-plugin

Conversation

@totally-not-ai

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

Copy link
Copy Markdown
Contributor

Summary

The Vite plugin that preserves the Vaadin usage statistics comment returned the code of every frontend module without a sourcemap, so the bundler dropped all of them from the emitted .map files. Dev bundles built with build.sourcemap enabled got maps with no sources, and the browser could not show the original frontend files. The plugin now leaves untouched modules alone and returns a real sourcemap for the one module it rewrites.

What changed

Behavior change: for anyone building a dev bundle with build.sourcemap enabled, the emitted .map files now contain the original frontend sources instead of being empty. No public Java API behaviour changes, and no application code needs updating.

  • vite.generated.ts: the vaadin:preserve-usage-stats plugin returns null for modules it does not change, instead of returning the unchanged code with no map. Returning code without a map makes the bundler drop that module from the sourcemap of its chunk, and this hook sees every module in the bundle.
  • The one module it does change (the usage statistics comment rewrite, /** → /*! so minifiers keep it) is now rewritten with magic-string, so the plugin can return a proper sourcemap along with the code.
  • Error cases (vaadin-dev-mode:start tag missing, rewritten comment no longer matching the expected pattern) still log the same messages and now leave the module untouched.
  • Test support: the dev bundle test app builds with build.sourcemap enabled, gets a vaadin-usage-statistics-stub.js frontend module that carries the comment in its plain form (the published @vaadin/vaadin-usage-statistics already carries it rewritten), and the sourcemap assertions shared by the dev and production bundle tests moved into a new SourceMapTestUtil in flow-test-util.

Fixes #16679, follow-up to #25691.

API Changes

com.vaadin.flow.testutil.SourceMapTestUtil

// Added
public final class SourceMapTestUtil
public static List<String> assertSourceMapUsable(String name, String sourceMap) // asserts the map has sources, mappings and source contents; returns the source file names

Test summary

# Status What the test verifies Why it matters
1 ✅ Every .js.map of the dev bundle has sources, non-empty mappings, and the content of each source This is the bug: the maps were emitted empty and the browser could not map the bundle back to the sources
2 ✅ A dev bundle sourcemap refers to a normal frontend file (lit-view.ts) the plugin does not touch Covers the return null path — modules the plugin skips must stay in the chunk map
3 ✅ A dev bundle sourcemap refers to the rewritten usage statistics module itself Covers the path where the plugin does change code — its own map must chain onto the bundler's
4 ✅ In every chunk, the usage statistics dev mode comment appears in the rewritten /*! form The rewrite is the only thing the plugin does; a broken rewrite would let a minifier strip the comment
5 ✅ Production bundle sourcemaps still point to the original sources after the assertions moved to the shared helper Guards against the refactor weakening the existing production check
6 ❗ gap The error paths (missing vaadin-dev-mode:start tag, rewritten comment failing the expected pattern) log and leave the module unchanged Only reachable with a malformed module; not exercised by a test
  • DevBundleSourceMapsIT.devBundleSourceMapsPointToOriginalSources — rows 1, 2, 3
  • DevBundleSourceMapsIT.usageStatisticsCommentIsRewrittenInTheBundle — row 4
  • SourceMapsIT.bundleSourceMapsPointToOriginalSources (changed to use the shared helper) — row 5

Deliberately untested: SourceMapTestUtil itself, which is exercised through both ITs, and the plugin's console error branches (row 6), which need a deliberately malformed usage statistics module.

The plugin that preserves the usage statistics comment returned the code
of every module without a sourcemap, so the bundler left all of them out
of the maps of the chunks they ended up in. A dev bundle built with
build.sourcemap enabled got .map files with no sources at all, and the
browser could not show the original frontend files. The plugin now
returns null for the modules it does not change, and a real map for the
one module it does.

Fixes #16679, Follow-up to #25691
…ugin

The dev bundle of the test application had no module the plugin rewrites,
so only its early return was covered and a broken rewrite would have gone
unnoticed. The published @vaadin/vaadin-usage-statistics already carries
the rewritten comment, so the application now has a frontend module that
stands in for it with the plain comment form the plugin has to handle.
@github-actions

github-actions Bot commented Sep 14, 2026 •

Copy link
Copy Markdown
Contributor

Test Results

 1 475 files  +1   1 559 suites  +1   1h 37m 4s ⏱️ - 3m 37s
12 241 tests +2  12 173 ✅ +2  68 💤 ±0  0 ❌ ±0 
12 559 runs  +2  12 491 ✅ +2  68 💤 ±0  0 ❌ ±0 

Results for commit c9872a7. ± Comparison against base commit aaf20ce.

♻️ This comment has been updated with latest results.

The dev bundle and the production bundle tests checked the emitted
sourcemaps with the same assertions, written twice. They now use one
helper in flow-test-util, which returns the sources of the map so that a
test can also assert which files it refers to.
The frontend build regenerates the checked-in file with the dependency
versions of the moment, which has nothing to do with this branch.
The test looked at the first chunk that has the comment, which is the
order the file system happens to return. A chunk the plugin misses while
it rewrites another one was therefore found only by chance.
@@ -0,0 +1,10 @@
/** vaadin-dev-mode:start
console.log('Usage statistics stub');

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.

This logging code is what should actually run and what should be tested - is 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.

Good catch — it was not, and now it is. @Artur-

That code is exactly what has to run: the development mode detector reads it out of maybeGatherAndSendStats.toString() and evaluates it with new Function, which only works while the build keeps the comment in the bundle. The test used to only grep the built chunk for the comment, so a build that kept the text but broke the surrounding function would still have passed.

The stand-in module now goes through the real mechanism, like the published @vaadin/vaadin-usage-statistics does: it imports runIfDevelopmentMode from @vaadin/vaadin-development-mode-detector and passes the function to it. The code inside the comment sets an attribute on <html>, and DevBundleSourceMapsIT.usageStatisticsCodeRunsFromTheBundle asserts that attribute after a view has loaded. Development mode is forced on in the module so the test covers the build rather than the heuristics the detector uses to recognize development mode.

Two side effects of pulling the detector in:

  • The file is now .js instead of .ts, because it touches window.Vaadin, and the detector is declared with @NpmPackage so it also resolves under pnpm and bun.
  • The detector has a dev mode comment of its own, in isMinified(), which nothing rewrites. The check for the rewritten comment therefore matches the comments one by one and asserts the form of the one that belongs to the usage statistics module, instead of asserting that no chunk has the plain form anywhere.

The code inside the comment is the code that is meant to run: the
development mode detector reads it out of the source of the function it
is in and evaluates it. The test only looked for the comment in the
bundle, so it did not cover that. The stand-in module now goes through
the detector like the published package does, and the code in its comment
marks the document, which the test asserts after loading a view.

The check for the rewritten comment is now matched per comment, because
the detector brings a dev mode comment of its own into the bundle that
nothing rewrites.

@Artur- Artur- left a comment

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.

After all, let's simplify this so we test only what is actually in this repository, so the comment replacing. Not use the actual packages or test that code is run

Running the code from the dev mode comment pulled the development mode
detector package into the test application and covered how that package
reads and evaluates the comment, which is not code of this repository.
The stand-in module is a plain frontend file again, and the test checks
what the plugin does: the comment is in the bundle in the rewritten form.
@totally-not-ai

Copy link
Copy Markdown
Contributor Author

@Artur- Simplified as asked — the test is back to covering only the comment rewriting.

  • The stand-in module is a plain frontend file again, with no dependency on @vaadin/vaadin-development-mode-detector and no @NpmPackage in the test application.
  • The test that asserted the commented code runs in the browser is gone.
  • What is left: the emitted chunk has the module's dev mode comment in the rewritten /*! form, and the sourcemap test, which is what the fix itself is about.
  • The comment check matches the comments one by one rather than scanning a whole chunk for the plain form, so it stays correct if a module that keeps its own unrewritten comment ends up in the bundle.

Module integration tests pass: 10/10.

@sonarqubecloud

Copy link
Copy Markdown

@Artur-
Artur- added this pull request to the merge queue Sep 15, 2026
Merged via the queue into main with commit 6246240 Sep 15, 2026
42 checks passed
@Artur-
Artur- deleted the fix/dev-bundle-sourcemaps-usage-stats-plugin branch September 15, 2026 09:35
vaadin-bot added a commit that referenced this pull request Sep 15, 2026
…lugin (#25708) (CP: 25.3) (#25715)

This PR cherry-picks changes from the original PR #25708 to branch 25.3.
---
#### Original PR description
> ## Summary
> The Vite plugin that preserves the Vaadin usage statistics comment
returned the code of every frontend module without a sourcemap, so the
bundler dropped all of them from the emitted `.map` files. Dev bundles
built with `build.sourcemap` enabled got maps with no sources, and the
browser could not show the original frontend files. The plugin now
leaves untouched modules alone and returns a real sourcemap for the one
module it rewrites.
> 
> ## What changed
> **Behavior change:** for anyone building a dev bundle with
`build.sourcemap` enabled, the emitted `.map` files now contain the
original frontend sources instead of being empty. No public Java API
behaviour changes, and no application code needs updating.
> 
> - `vite.generated.ts`: the `vaadin:preserve-usage-stats` plugin
returns `null` for modules it does not change, instead of returning the
unchanged code with no map. Returning code without a map makes the
bundler drop that module from the sourcemap of its chunk, and this hook
sees every module in the bundle.
> - The one module it does change (the usage statistics comment rewrite,
`/**` → `/*!` so minifiers keep it) is now rewritten with
`magic-string`, so the plugin can return a proper sourcemap along with
the code.
> - Error cases (`vaadin-dev-mode:start` tag missing, rewritten comment
no longer matching the expected pattern) still log the same messages and
now leave the module untouched.
> - Test support: the dev bundle test app builds with `build.sourcemap`
enabled, gets a `vaadin-usage-statistics-stub.js` frontend module that
carries the comment in its plain form (the published
`@vaadin/vaadin-usage-statistics` already carries it rewritten), and the
sourcemap assertions shared by the dev and production bundle tests moved
into a new `SourceMapTestUtil` in `flow-test-util`.
> 
> Fixes #16679, follow-up to #25691.
> 
> ## API Changes
> 
> ### com.vaadin.flow.testutil.SourceMapTestUtil
> 
> ```java
> // Added
> public final class SourceMapTestUtil
> public static List<String> assertSourceMapUsable(String name, String
sourceMap) // asserts the map has sources, mappings and source contents;
returns the source file names
> ```
> 
> ## Test summary
> 
> | # | Status | What the test verifies | Why it matters |
> |---|--------|------------------------|----------------|
> | 1 | ✅ | Every `.js.map` of the dev bundle has sources, non-empty
mappings, and the content of each source | This is the bug: the maps
were emitted empty and the browser could not map the bundle back to the
sources |
> | 2 | ✅ | A dev bundle sourcemap refers to a normal frontend file
(`lit-view.ts`) the plugin does not touch | Covers the `return null`
path — modules the plugin skips must stay in the chunk map |
> | 3 | ✅ | A dev bundle sourcemap refers to the rewritten usage
statistics module itself | Covers the path where the plugin does change
code — its own map must chain onto the bundler's |
> | 4 | ✅ | In every chunk, the usage statistics dev mode comment
appears in the rewritten `/*!` form | The rewrite is the only thing the
plugin does; a broken rewrite would let a minifier strip the comment |
> | 5 | ✅ | Production bundle sourcemaps still point to the original
sources after the assertions moved to the shared helper | Guards against
the refactor weakening the existing production check |
> | 6 | ❗ **gap** | The error paths (missing `vaadin-dev-mode:start`
tag, rewritten comment failing the expected pattern) log and leave the
module unchanged | Only reachable with a malformed module; not exercised
by a test |
> 
> - `DevBundleSourceMapsIT.devBundleSourceMapsPointToOriginalSources` —
rows 1, 2, 3
> - `DevBundleSourceMapsIT.usageStatisticsCommentIsRewrittenInTheBundle`
— row 4
> - `SourceMapsIT.bundleSourceMapsPointToOriginalSources` (changed to
use the shared helper) — row 5
> 
> Deliberately untested: `SourceMapTestUtil` itself, which is exercised
through both ITs, and the plugin's console error branches (row 6), which
need a deliberately malformed usage statistics module.

Co-authored-by: totally-not-ai[bot] <290682512+totally-not-ai[bot]@users.noreply.github.com>
@vaadin-bot

Copy link
Copy Markdown
Collaborator

This ticket/PR has been released with Vaadin 25.4.0-alpha1.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Sourcemaps are not generated with vaadin 24

2 participants