fix(theme): keep the css sourcemap chain intact when rewriting theme urls - #25710
Merged
Merged
Conversation
The plugin that rewrites the urls of application theme css files handed back the rewritten css without a sourcemap, which throws away the map the bundler has for that file. It now returns a map of the rewriting, and reports having done nothing when a file has no url to rewrite. The rewritten css is byte for byte the same as before, and no build emits a css sourcemap today, so the output of every build is unchanged. Related to #16679
A per-character sourcemap of the url rewriting is over twice the size of a boundary one for no gain: the rewriting replaces whole url(...) tokens, so every mapping a reader needs sits on a token boundary. Measured on a 94 KB theme css, the mappings shrink from 416 KB to 187 KB, and every token after an edit still traces back to its exact original position.
Artur-
approved these changes
Sep 15, 2026
Artur-
enabled auto-merge
September 15, 2026 06:58
Contributor
|
vaadin-bot
added a commit
that referenced
this pull request
Sep 15, 2026
…urls (#25710) (CP: 25.3) (#25713) This PR cherry-picks changes from the original PR #25710 to branch 25.3. --- #### Original PR description > ## Summary > The build plugin that rewrites `url(...)` references in application theme CSS returned the rewritten CSS without a sourcemap, which threw away the map the bundler had for that file. It now returns a sourcemap for the rewrite, and returns nothing at all when a file has no url to rewrite. > > ## What changed > **Behavior change (internal API only):** `rewriteCssUrls()` in `theme-loader-utils.js` no longer returns a string. It now returns `{ code, map }` when it rewrote something, and `null` when there was nothing to rewrite. Both callers in this repo are updated: the Vite theme plugin in `vite.generated.ts` returns the object straight to Vite, and the `theme-loader.js` loader uses `rewritten.code`. This is an internal build-tool helper, not public API, so applications are not affected. > > **Build output is unchanged.** The rewritten CSS is byte for byte the same as before, and no build enables CSS sourcemaps today, so nothing in the emitted bundle changes. > > Other details: > - The rewrite now runs through `MagicString` (already pinned at `1.3.1` in the generated Vite `package.json`, so no new dependency). > - The map is generated with `hires: 'boundary'` instead of per character. Since the rewrite replaces whole `url(...)` tokens, every position a reader needs is on a token boundary, so nothing is lost. On a 94 KB theme CSS file this cut the mappings from 416 KB to 187 KB. > - Returning `null` for untouched files lets Vite keep the file's existing map instead of replacing it with an identity map. > > Related to #16679 > > ## Test summary > This branch adds no tests. There is no JavaScript unit-test harness for the files under `flow-build-tools/src/main/resources/plugins/`, so the table below is all gaps — it lists what a reviewer would want pinned. > > | # | Status | What the test verifies | Why it matters | > |---|--------|------------------------|----------------| > | 1 | ❗ **gap** | `rewriteCssUrls` produces the same rewritten CSS text as before for the known url shapes (`./`, `../`, extra dot segments, quoted and unquoted) | This is the promise that build output does not change; a regression silently breaks theme asset urls | > | 2 | ❗ **gap** | The returned `map` traces a rewritten `url(...)` token, and the text after it, back to its original position | The whole point of the fix; a wrong map sends debuggers to the wrong place | > | 3 | ❗ **gap** | `rewriteCssUrls` returns `null` for a CSS file with no url to rewrite | If it returned a `{code, map}` pair instead, Vite would drop the file's real upstream map | > | 4 | ❗ **gap** | `theme-loader.js` passes the original source to `this.callback` when the rewrite returns `null`, and `rewritten.code` otherwise | A missed `null` check here would feed `undefined` CSS into the webpack-style pipeline | > > - No test methods were added or changed on this branch, so no row maps to a named test. > > Deliberately untested: the map's exact byte size (an optimisation detail, not a contract) and the logger output. The `flow-tests/test-frontend/vite-*` IT modules build real themes and would catch a crash or broken CSS, but they do not check sourcemap accuracy, so rows 2 and 3 stay uncovered. Co-authored-by: totally-not-ai[bot] <290682512+totally-not-ai[bot]@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The build plugin that rewrites
url(...)references in application theme CSS returned the rewritten CSS without a sourcemap, which threw away the map the bundler had for that file. It now returns a sourcemap for the rewrite, and returns nothing at all when a file has no url to rewrite.What changed
Behavior change (internal API only):
rewriteCssUrls()intheme-loader-utils.jsno longer returns a string. It now returns{ code, map }when it rewrote something, andnullwhen there was nothing to rewrite. Both callers in this repo are updated: the Vite theme plugin invite.generated.tsreturns the object straight to Vite, and thetheme-loader.jsloader usesrewritten.code. This is an internal build-tool helper, not public API, so applications are not affected.Build output is unchanged. The rewritten CSS is byte for byte the same as before, and no build enables CSS sourcemaps today, so nothing in the emitted bundle changes.
Other details:
MagicString(already pinned at1.3.1in the generated Vitepackage.json, so no new dependency).hires: 'boundary'instead of per character. Since the rewrite replaces wholeurl(...)tokens, every position a reader needs is on a token boundary, so nothing is lost. On a 94 KB theme CSS file this cut the mappings from 416 KB to 187 KB.nullfor untouched files lets Vite keep the file's existing map instead of replacing it with an identity map.Related to #16679
Test summary
This branch adds no tests. There is no JavaScript unit-test harness for the files under
flow-build-tools/src/main/resources/plugins/, so the table below is all gaps — it lists what a reviewer would want pinned.rewriteCssUrlsproduces the same rewritten CSS text as before for the known url shapes (./,../, extra dot segments, quoted and unquoted)maptraces a rewrittenurl(...)token, and the text after it, back to its original positionrewriteCssUrlsreturnsnullfor a CSS file with no url to rewrite{code, map}pair instead, Vite would drop the file's real upstream maptheme-loader.jspasses the original source tothis.callbackwhen the rewrite returnsnull, andrewritten.codeotherwisenullcheck here would feedundefinedCSS into the webpack-style pipelineDeliberately untested: the map's exact byte size (an optimisation detail, not a contract) and the logger output. The
flow-tests/test-frontend/vite-*IT modules build real themes and would catch a crash or broken CSS, but they do not check sourcemap accuracy, so rows 2 and 3 stay uncovered.