Performance: Coalesce JavaScript chunks around the Backoffice to reduce the number of requests#22896
Merged
Merged
Conversation
|
Claude finished @iOvergaard's task in 2m 34s —— View job PR ReviewTarget: Adds opt-in
Suggestions
Approved with Suggestions for improvementGood to go, but please carefully consider the importance of the suggestions. Labels applied: |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR is a build-config only change that opts packages/core into Rollup's experimentalMinChunkSize (10 KB) to coalesce many tiny non-entry chunks, reducing emitted .js files in dist-cms/packages/core from 981 → 272 (-72%) while keeping all 63 public subpath entries byte-stable. It's an additive perf improvement toward issue #21152 (too many small JS files at backoffice startup); full consolidation is deferred to v18 due to source-level cycles.
Changes:
- Adds optional
minChunkSizeto the sharedgetDefaultConfighelper, threading to Rollupoutput.experimentalMinChunkSize; defaultundefinedkeeps every other workspace byte-identical. - Opts
packages/core/vite.config.tsin at10_000bytes. - Adds design spec, plan, sweep/baseline/final bench notes, and a unified-workspace-build spike note.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
src/Umbraco.Web.UI.Client/src/vite-config-base.ts |
Adds optional minChunkSize arg and forwards to rollupOptions.output.experimentalMinChunkSize. |
src/Umbraco.Web.UI.Client/src/packages/core/vite.config.ts |
Opts core package in at 10 KB threshold. |
docs/superpowers/specs/2026-05-19-core-bundling-design.md |
Design spec for the chunk-coalescing approach. |
docs/superpowers/plans/2026-05-19-core-bundling.md |
Step-by-step implementation plan. |
docs/superpowers/plans/bench/bench-baseline.md |
Pre-change metrics for packages/core dist. |
docs/superpowers/plans/bench/bench-sweep.md |
Threshold-sweep table justifying 10 KB. |
docs/superpowers/plans/bench/bench-final.md |
Post-change delta and verification gate. |
docs/superpowers/plans/notes/unified-workspace-build-spike.md |
Spike note on a future unified Vite build. |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
claude
Bot
added
area/frontend
category/performance
iOvergaard
force-pushed
the
v17/improvement/67983-core-bundling
branch
from
2a20fc7 to
732daa1
Compare
5 tasks
iOvergaard
force-pushed
the
v17/improvement/67983-core-bundling
branch
from
732daa1 to
de57505
Compare
iOvergaard
changed the title
iOvergaard
force-pushed
the
v17/improvement/67983-core-bundling
branch
from
de57505 to
7613441
Compare
iOvergaard
force-pushed
the
v17/improvement/67983-core-bundling
branch
from
7613441 to
14254c4
Compare
…983) Set experimentalMinChunkSize=10_000 as the default in the shared Vite helper. Every workspace inherits the coalescing automatically; the threshold can still be overridden per workspace (pass 0 to disable). Impact on dist-cms output: - packages/core .js files: 981 -> 272 (-72%) - All workspaces combined .js files: 2194 -> 1401 (-36%) - Welcome dashboard .js requests: 510 -> 497 (-2.5%) - packages/ufm requests in particular: 23 -> 12 (-48%) - Gzipped bundle total: -1.2% - Raw bytes: +1.4% (small overhead from merged chunks; gzip wins it back) All entry chunks are preserved, so every public @umbraco-cms/backoffice/<sub> import keeps resolving without changes to package.json exports or tsconfig paths. Further consolidation (collapsing core's per-subpath entries into a single bundle with stubs) was prototyped but hits a TDZ cycle between the eager entry and its dynamic-import descendants. Tracked for v18, not part of this change.
iOvergaard
force-pushed
the
v17/improvement/67983-core-bundling
branch
from
14254c4 to
98f9a5f
Compare
Aligns the two outliers with the conventions used by the other 38
first-party packages:
- documents/umbraco-package.ts now uses the lazy bundle pattern
(type: 'bundle', js: () => import('./manifests.js')) instead of
eagerly importing manifests at module evaluation. The bundle
initializer auto-loads the manifests at boot, so behaviour is
unchanged.
- umbraco-news/manifests.ts now exports `manifests: Array<...>`
instead of a bare `dashboard` object. The bundle initializer
enumerates exports regardless of name, so behaviour is unchanged.
Preparatory cleanup so future build-time manifest aggregation can
treat every workspace uniformly.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 task
madsrasmussen
approved these changes
May 22, 2026
iOvergaard
added a commit
that referenced
this pull request
May 22, 2026
…983) (#22896) * Backoffice: Coalesce small Rollup chunks across all workspaces (AB#67983) Set experimentalMinChunkSize=10_000 as the default in the shared Vite helper. Every workspace inherits the coalescing automatically; the threshold can still be overridden per workspace (pass 0 to disable). Impact on dist-cms output: - packages/core .js files: 981 -> 272 (-72%) - All workspaces combined .js files: 2194 -> 1401 (-36%) - Welcome dashboard .js requests: 510 -> 497 (-2.5%) - packages/ufm requests in particular: 23 -> 12 (-48%) - Gzipped bundle total: -1.2% - Raw bytes: +1.4% (small overhead from merged chunks; gzip wins it back) All entry chunks are preserved, so every public @umbraco-cms/backoffice/<sub> import keeps resolving without changes to package.json exports or tsconfig paths. Further consolidation (collapsing core's per-subpath entries into a single bundle with stubs) was prototyped but hits a TDZ cycle between the eager entry and its dynamic-import descendants. Tracked for v18, not part of this change. * Backoffice: Normalise umbraco-package + manifests shapes (AB#67983) Aligns the two outliers with the conventions used by the other 38 first-party packages: - documents/umbraco-package.ts now uses the lazy bundle pattern (type: 'bundle', js: () => import('./manifests.js')) instead of eagerly importing manifests at module evaluation. The bundle initializer auto-loads the manifests at boot, so behaviour is unchanged. - umbraco-news/manifests.ts now exports `manifests: Array<...>` instead of a bare `dashboard` object. The bundle initializer enumerates exports regardless of name, so behaviour is unchanged. Preparatory cleanup so future build-time manifest aggregation can treat every workspace uniformly. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
Author
|
Picked for 17.5 with 04f0e22 |
iOvergaard
added a commit
that referenced
this pull request
May 22, 2026
…983) (#22896) * Backoffice: Coalesce small Rollup chunks across all workspaces (AB#67983) Set experimentalMinChunkSize=10_000 as the default in the shared Vite helper. Every workspace inherits the coalescing automatically; the threshold can still be overridden per workspace (pass 0 to disable). Impact on dist-cms output: - packages/core .js files: 981 -> 272 (-72%) - All workspaces combined .js files: 2194 -> 1401 (-36%) - Welcome dashboard .js requests: 510 -> 497 (-2.5%) - packages/ufm requests in particular: 23 -> 12 (-48%) - Gzipped bundle total: -1.2% - Raw bytes: +1.4% (small overhead from merged chunks; gzip wins it back) All entry chunks are preserved, so every public @umbraco-cms/backoffice/<sub> import keeps resolving without changes to package.json exports or tsconfig paths. Further consolidation (collapsing core's per-subpath entries into a single bundle with stubs) was prototyped but hits a TDZ cycle between the eager entry and its dynamic-import descendants. Tracked for v18, not part of this change. * Backoffice: Normalise umbraco-package + manifests shapes (AB#67983) Aligns the two outliers with the conventions used by the other 38 first-party packages: - documents/umbraco-package.ts now uses the lazy bundle pattern (type: 'bundle', js: () => import('./manifests.js')) instead of eagerly importing manifests at module evaluation. The bundle initializer auto-loads the manifests at boot, so behaviour is unchanged. - umbraco-news/manifests.ts now exports `manifests: Array<...>` instead of a bare `dashboard` object. The bundle initializer enumerates exports regardless of name, so behaviour is unchanged. Preparatory cleanup so future build-time manifest aggregation can treat every workspace uniformly. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
Author
|
Picked for 18.0 with 26232e0 |
5 tasks
iOvergaard
added a commit
that referenced
this pull request
May 26, 2026
…ts (AB#68478) (#22951) * Backoffice: Add Cache-Control headers to cache-busted backoffice assets (AB#68478) Adds a UseUmbracoBackOfficeCacheHeaders middleware that sets Cache-Control: public, max-age=31536000, immutable on responses served from the cache-busted backoffice path (/umbraco/backoffice/<hash>/*). The hash in the URL is derived from the Umbraco version, so the URL itself invalidates on every release - making 'immutable' safe regardless of whether individual filenames contain a content hash. In debug mode the cache-bust hash changes per request, so the header is set to 'no-cache' to avoid filling the browser disk cache with single-use entries. Design is non-destructive to consumer customisation, addressing the review feedback on the v14 attempt (#14475): - Does not touch StaticFileOptions; consumer services.Configure<StaticFileOptions>(...) and OnPrepareResponse callbacks continue to work unchanged. - Sets the header via Response.OnStarting with a ContainsKey guard, so any synchronous Cache-Control set upstream wins; consumer OnStarting callbacks registered later fire first (LIFO) and also win. - Skips non-2xx responses to avoid long-lived caching of error responses. Related: GH #21152, PR #22896. * Backoffice: Correct rationale for no-cache in debug mode Reword the XML doc on UseUmbracoBackOfficeCacheHeaders to reflect that IBackOfficePathGenerator is a singleton, so the cache-bust hash is computed once at startup even in debug mode (per Copilot review on #22951). The reason for no-cache is not "hash changes per request" but that built assets may change in place during dev iteration; no-cache allows fast 304 revalidation while no-store would force full re-downloads. No functional change. * Backoffice: Add unit tests for UseUmbracoBackOfficeCacheHeaders Covers six scenarios via a minimal in-process pipeline composed with Microsoft.AspNetCore.TestHost: - Production: 200 under hash prefix gets immutable header - Debug: 200 under hash prefix gets no-cache - Non-2xx under prefix: header not set (status gate) - Path outside prefix: header not set (path gate) - Consumer synchronous override: ContainsKey guard skips, consumer wins - Consumer OnStarting override: LIFO ordering lets consumer win Adds Microsoft.AspNetCore.TestHost to Umbraco.Tests.UnitTests (standard Microsoft package, version pinned in tests/Directory.Packages.props). * Backoffice: Extract cache-headers logic into IMiddleware class Matches the existing Umbraco middleware convention (BootFailedMiddleware, PreviewAuthenticationMiddleware, UmbracoRequestMiddleware, etc.) per Kenn's note: prefer UseMiddleware<T>() with a DI-resolved class over inline builder.Use lambdas. The new UmbracoBackOfficeCacheHeadersMiddleware: - Implements IMiddleware; registered as a singleton in AddWebComponents - Computes prefix and header value once in the constructor (both dependencies are singletons themselves, so this is stable) - Behaviour is unchanged from the inline version The UseUmbracoBackOfficeCacheHeaders extension method becomes a thin UseMiddleware<T>() wrapper. Tests updated to register the middleware in the TestServer DI container so it can be resolved through UseMiddleware. * Backoffice: Document IMiddleware convention in Web.Common CLAUDE.md Adds an explicit "Convention" note before the middleware list so future contributors (and AI assistants) default to the IMiddleware class + AddSingleton + UseMiddleware<T>() pattern rather than inline builder.Use(async ...) lambdas. Also lists the new UmbracoBackOfficeCacheHeadersMiddleware in the folder structure and middleware reference. * Backoffice: Tighten middleware convention note with full corroboration Lists every IMiddleware implementer in the codebase (10/10) and calls out the two known inline-lambda exceptions (CspNonceExtensions, WebApplicationExtensions) so the rule reads as the established convention rather than an absolute, while still steering new work toward IMiddleware + AddSingleton + UseMiddleware<T>(). * Backoffice: Register cache-headers middleware in AddBackOfficeCore DI scope validation runs in Development/CI and pre-checks every singleton's dependency graph can be constructed. The middleware was registered in AddWebComponents (which runs for every Umbraco bootstrap), but its IBackOfficePathGenerator dependency is only registered by AddBackOffice(). The previous CI run on this branch surfaced the problem in four Delivery-only/Website-only bootstrap tests (CoreWithDeliveryApi_BootsSuccessfully, DeliveryOnlyScenario_BootsSuccessfully, etc.) with "Unable to resolve service for type 'IBackOfficePathGenerator' while attempting to activate 'UmbracoBackOfficeCacheHeadersMiddleware'". Move the registration alongside IBackOfficePathGenerator in AddBackOfficeCore (Api.Management), which is the same scope as the backoffice itself. This also matches the wire-up gate in UmbracoApplicationBuilder.cs that only calls UseUmbracoBackOfficeCacheHeaders when IBackOfficeEnabledMarker is registered. CLAUDE.md updated with the rule ("register the middleware next to its dependencies' registration") and a pitfall note about DI scope validation. * Backoffice: Address review feedback from AndyButland (PR #22951) - Move UseUmbracoBackOfficeCacheHeadersTests from Umbraco.Tests.UnitTests to Umbraco.Tests.Integration. It uses HostBuilder + TestServer to exercise the real HTTP pipeline, which is integration-shaped rather than unit-shaped. Drop Microsoft.AspNetCore.TestHost from UnitTests (Mvc.Testing in Integration provides it transitively) and from tests/Directory.Packages.props. - Soften the misleading "no trailing slash" comment in UmbracoBackOfficeCacheHeadersMiddleware — we trim anyway, so the comment is now framed as defensive normalisation. - Trim the dense middleware convention note in Web.Common/CLAUDE.md to one paragraph (rule + the two known inline-lambda exceptions). Move the DI-scope-validation pitfall narrative out of CLAUDE.md and into a three-line code comment next to the AddSingleton call in AddBackOfficeCore where it actually applies. * Backoffice: HTTP verb gate, 304 inclusion, namespace + unused using (PR #22951 review) Three more from AndyButland's review: 1. Verb gate + 304 inclusion in UmbracoBackOfficeCacheHeadersMiddleware. Restrict the path-prefix match to GET and HEAD so POST/PUT/DELETE responses and OPTIONS (CORS preflight) responses don't get tagged as immutable. Include 304 alongside 2xx in the status gate so intermediate caches (CDN/proxy) receive the Cache-Control directive on revalidation responses too. Extended the test suite with four new cases: NotModifiedResponseUnderPrefix_SetsImmutable, HeadRequestUnderPrefix_SetsImmutable, OptionsRequestUnderPrefix_DoesNotSetHeader, PostRequestUnderPrefix_DoesNotSetHeader. All 10 tests pass. 2. Test namespace updated to Umbraco.Cms.Tests.Integration.* to match the convention used by ~629 other files in Umbraco.Tests.Integration (vs the 2 outliers I copied from). 3. Drop unused 'using Umbraco.Extensions;' from the test file. * Backoffice: Extract conditional checks to satisfy CodeScene complexity gate CodeScene flagged InvokeAsync with "Complex Conditional" (advisory rule, code health impact 9.69) after the verb + 304 additions in the prior commit. Extract the two checks into IsCacheableAssetRequest and ShouldSetCacheControl helper methods. No behaviour change; tests still green (10/10, 149 ms).
iOvergaard
changed the title
iOvergaard
changed the title
This was referenced Jun 26, 2026
Merged
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
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
One-line config change. Sets
experimentalMinChunkSize=10_000as the default in the shared Vite helper so every workspace coalesces sub-10 KB Rollup chunks. Pure build-config change — no source moved, no public import paths changed, runtime semantics unchanged. Entry chunks (one per public@umbraco-cms/backoffice/<sub>subpath) are never removed, so every importmap entry keeps resolving against the same dist file.Additive toward the broader perf concern in #21152 (too many tiny JS files at backoffice startup), not a full resolution.
What changed
src/Umbraco.Web.UI.Client/src/vite-config-base.tsminChunkSizearg with default10_000. Threads through to Rollup'soutput.experimentalMinChunkSize. Workspaces can override (pass0to disable).Build output (
dist-cms/).jsfiles inpackages/core.jsfiles inexternal/monaco-editor.jsfiles across dist-cmsRaw bytes tick up slightly because merged chunks add a small overhead of preserved import statements; gzip wins it back and then some.
Browser-side requests
Measured live via Playwright on https://localhost:44339/umbraco. The Performance Resource Timing API caps at 250 entries by default, so these come from Playwright's own (uncapped) network capture.
The win scales with page complexity. On the heavy editor case (the user-reported pain point in #21152),
packages/coreaccounts for −32 of the requests,packages/ufmfor −11, anddocuments/block/relations/multi-url-pickerdeliver the rest.Time-to-First-Edit, network-throttled
Fresh Chromium context per run, login NOT included in the timing. Measurement: navigate to the doc editor (RTE + Block Grid) and time to a typed character landing in the Rich Text Editor. Chrome DevTools Protocol's
Network.emulateNetworkConditionsapplied after login so only the measured navigation is throttled. Medians of n=5–10 runs per cell.Where the PR wins: typical content-editor networks (decent bandwidth, moderate latency). About one second faster to first keystroke on a RTE + Block Grid document — driven by ~50 fewer HTTP request round-trips multiplied against ~50 ms RTT.
Why mobile/3G is flat: those measurements are throughput-bound (gzipped payload is essentially identical before vs after, ~2.5 MB), so a file-count reduction without a bytes reduction doesn't move the needle there. Reducing bytes is a separate optimization tracked in the v18 followups.
Threshold rationale
10 KB sits at the knee of the sweep curve for
packages/core:10 KB gives the best file-count-vs-largest-chunk balance; 50 KB starts bloating the top chunk past 240 KB without meaningful gzip improvement.
Why stop here
A follow-up prototype consolidated
packages/corefurther (down to 253 files) by collapsing the per-subpath entries into a singleindex.jswith stubs. Build-time wins continued, but at runtime the consolidated bundle hit a TDZ error: dynamic-import descendants that subclassUmbLitElementcouldn't access the symbol because the parentindex.jshadn't finished initialising. Fixing that requires hoisting shared base classes out ofcoreinto stable libs — a multi-PR refactor that doesn't belong in v17.Test plan
npm run check:circular— PASSnpm run check:module-dependencies— PASS (14 bidirectional imports, all pre-existing; 5 illegal imports at the threshold, all pre-existing)npm run check:paths— PASSnpm run lint:errors— PASSnpm test— PASS (253 test files, 1834 tests, 0 failures)npm run build:for:cmsend-to-end — PASS