fix: keep dev bundle sourcemaps usable in the usage statistics Vite plugin - #25708
Conversation
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.
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'); | |||
There was a problem hiding this comment.
This logging code is what should actually run and what should be tested - is it?
There was a problem hiding this comment.
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
.jsinstead of.ts, because it toucheswindow.Vaadin, and the detector is declared with@NpmPackageso 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-
left a comment
There was a problem hiding this comment.
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.
|
@Artur- Simplified as asked — the test is back to covering only the comment rewriting.
Module integration tests pass: 10/10. |
|
…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>
|
This ticket/PR has been released with Vaadin 25.4.0-alpha1. |



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
.mapfiles. Dev bundles built withbuild.sourcemapenabled 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.sourcemapenabled, the emitted.mapfiles 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: thevaadin:preserve-usage-statsplugin returnsnullfor 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./**→/*!so minifiers keep it) is now rewritten withmagic-string, so the plugin can return a proper sourcemap along with the code.vaadin-dev-mode:starttag missing, rewritten comment no longer matching the expected pattern) still log the same messages and now leave the module untouched.build.sourcemapenabled, gets avaadin-usage-statistics-stub.jsfrontend module that carries the comment in its plain form (the published@vaadin/vaadin-usage-statisticsalready carries it rewritten), and the sourcemap assertions shared by the dev and production bundle tests moved into a newSourceMapTestUtilinflow-test-util.Fixes #16679, follow-up to #25691.
API Changes
com.vaadin.flow.testutil.SourceMapTestUtil
Test summary
.js.mapof the dev bundle has sources, non-empty mappings, and the content of each sourcelit-view.ts) the plugin does not touchreturn nullpath — modules the plugin skips must stay in the chunk map/*!formvaadin-dev-mode:starttag, rewritten comment failing the expected pattern) log and leave the module unchangedDevBundleSourceMapsIT.devBundleSourceMapsPointToOriginalSources— rows 1, 2, 3DevBundleSourceMapsIT.usageStatisticsCommentIsRewrittenInTheBundle— row 4SourceMapsIT.bundleSourceMapsPointToOriginalSources(changed to use the shared helper) — row 5Deliberately untested:
SourceMapTestUtilitself, which is exercised through both ITs, and the plugin's console error branches (row 6), which need a deliberately malformed usage statistics module.