Repository navigation
perf: lower manifest script preload priority - #8212
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Linked Issues check |  |
Issue #8546 requires low priority for generated preloads and forwarding into static Early Hints. The PR implements both: getScriptPreloadAttrs returns fetchPriority: 'low', `collectStaticHintsFrom… |
Limit the low-priority default to server-rendered matches. Preserve the existing priority for ssr: false and SPA output. Add the requested supported priority override, such as a transform result or HeadContent option, and ensure HTML an… |
| Out of Scope Changes check | |
The preload and Early Hints changes are related to issue #8546, and the added tests and documentation support that objective. The implementation also changes ssr: false, data-only, and SPA manifest … |
Restrict the changed behavior to server-rendered matches. Do not lower priority for ssr: false or SPA manifest preloads unless the issue scope is updated and the behavior is intentionally accepted. |
| Docstring Coverage | |
Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 8 files. (2 skipped: 2 … | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Description check The description clearly explains the low-priority preload change, motivation, implementation, testing, performance evidence, and scope. It does not use the template headings or include the checklist a… Title check The title is concise, specific, and accurately summarizes the main change: lowering the priority of manifest script preloads.
Full details: Linked Issues check
-
Explanation
Issue
#8546requires low priority for generated preloads and forwarding into static Early Hints. The PR implements both:getScriptPreloadAttrsreturnsfetchPriority: 'low',collectStaticHintsFromManifestusesaddEarlyHintFetchAttrs, and tests cover hint objects and serialized headers. However, the issue also requires preserving the existing priority forssr: falsematches and SPA mode, plus an escape hatch. The changed helper applieslowunconditionally, and the PR adds neither escape hatch.Resolution
Limit the low-priority default to server-rendered matches. Preserve the existing priority for
ssr: falseand SPA output. Add the requested supported priority override, such as a transform result orHeadContentoption, and ensure HTML and Early Hints use the same value.
Full details: Out of Scope Changes check
-
Explanation
The preload and Early Hints changes are related to issue
#8546, and the added tests and documentation support that objective. The implementation also changesssr: false, data-only, and SPA manifest preloads becausegetScriptPreloadAttrsnow returnslowfor every call. Issue#8546identifies those client-rendered modes as excluded from the requested default, so this behavior is outside the linked issue scope.
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 8 files. (2 skipped: 2 unsupported.)
- Fix all pre-merge checks with AI
✨ Finishing Touches: 💡 1
-
🛠️ Fix failing CI checks: 💡
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
|
This is great! We’ve also seen lower Lighthouse scores after migrating to Next.js 16.3, so hopefully this will fix it. |
We ran this patch downstream on two SSR marketing apps for a week. I've since reverted it, and the reasons may be useful input on your open questions. Measured prioritiesA probe page through headless Chrome with
Two things follow. An async module script and its static dependencies are already fetched at High. So the generated
Why an entry-only exemption doesn't workMy first instinct was your question 3's shape: keep
That makes this a genuine paint-versus-interactive tradeoff across the whole set, rather than something that can be tuned by classifying assets. Which I think sharpens your risk 1 rather than resolving it. Why we revertedMostly your own numbers. The 1.098 s mobile FCP improvement is under Lighthouse simulated throttling; under applied throttling the same deployments differ by 13 ms. Lighthouse's simulator models priority explicitly, so simulated results are exactly where a priority hint would look strongest. Your write-up is admirably clear about this — I just think the 13 ms number deserves more weight than the 1.1 s one when choosing a framework default. Against that, the hydration cost is real but also unmeasured: no extra main-thread work, but the whole graph lands after first paint, so the hydration long task shifts into the window where first interactions happen. We have no RUM on those apps, so neither side of the tradeoff is observable in the field for us, and a change we can't falsify wasn't worth the reasoning it required at each review. None of that argues the patch is wrong — for a content page where nothing interactive matters before paint, biasing to paint is defensible. It argues against it as an unconditional default, which is the question you're actually asking. An opt-in (or a per-route/per-asset override) would let apps make that call with knowledge of their own interactivity, and would sidestep your risks 2 and 5 entirely. Your point 4 stands independently, and looks like the sharper bug: Early Hints call CaveatsPriorities measured over local HTTP/1.1, headless, on a single Chrome version. Priority assignment is Blink-side so the protocol shouldn't matter, but these heuristics drift between releases. And as above, we have no field data either — this is a mechanism argument, not a measured INP delta. |
|
Ecosystem update relevant to your closing question about whether this should be a framework default. React 19.3.0 shipped react/react#36835, so Verified against the published packages, production builds, same script against both: function App() {
ReactDOM.preloadModule('/dep.js', { as: 'script', fetchPriority: 'low' })
return <div>hi</div>
}<!-- react-dom 19.2.8 -->
<link rel="modulepreload" href="/dep.js"/>
<!-- react-dom 19.3.0 -->
<link rel="modulepreload" href="/dep.js" fetchPriority="low"/>
Two things I take from it for this PR: React exposed a knob rather than changing a default. It still hardcodes It gives the React renderer a first-class path. If Start routed manifest preloads through It doesn't touch your point 4, which still looks like the sharpest issue in the list: Early Hints don't carry |
|
Some measurements on the gap between simulated and applied throttling mentioned in the description. They come from a Start app (React 19.3, Vite 8, 10 to 15 generated modulepreloads per page, about 135 KB of brotli JS) served over HTTP/2 behind Caddy, with Lighthouse 13.5 mobile:
The FCP drop comes from how Lantern builds its FCP graphs, not from bandwidth. This matches your applied-throttling result: real FCP is about 0.5 s either way, because the browser paints long before the scripts arrive. A default change would mostly move Lighthouse's FCP, and it would cost hydration time on slow links, since every preload is needed for hydration (as noted above). I'd also go with an opt-in. For now we're keeping the default priority in our app. |
|
Follow-up to my comment from Sept 10, where I said we'd reverted this patch because the hydration cost was unmeasured and the applied-throttling gain was 13 ms in your write-up. I've now measured both on one of those marketing pages (71 generated preloads, about 445 KB of JS). The data changes my conclusion: for an app like ours, Setup
Results
What this says about my earlier reasoning
Where that leaves the default questionI still wouldn't make it an unconditional default. The gain scales with preload bytes, and Caveats: one app, one network path (Southeast Asia to a Cloudflare edge), lab rather than field data, and a single interaction probed. For anyone wanting to try this without patching: |
|
Update to my data comment above: we shipped this to production on two Start sites, and I've changed my position. I now think Production before/after, with
In production on mobile and desktop Chrome: no hydration errors, menus open, client-side navigation works. TBT and TTI are unchanged within noise. What changed my mind:
Two refinements I'd suggest to this PR, both detailed in #8546:
The one regression we saw fits the first: on |
|
Data point for the open questions here, from a Start app (1.171, React 19, Vite 8 / rolldown) with transitive preloads, i.e. what #8520 would produce. Rolldown gives only direct imports, so we hoisted the transitive closure ourselves (same effect as #8520): 47 → 143 modulepreloads on the home page, ~890 KB of JS. Over HTTP/2 that blanked the page on slow networks: the stylesheets share the connection with every chunk and finish together with the last one. Chrome network emulation (Slow 4G: 150 ms, 1.6 Mbps, CPU 4x) against the real CDN over HTTP/2, median of 3:
So with a large preload set the applied-throttling gain is big, and If #8520 lands without this default, every rolldown app gets the blank first paint we hit. For now we rewrite |
|
I believe that the current priority is optimal for faster hydration and isn't actually blocking / increasing FCP. This PR just landed that addresses Lighthouse's heuristic being inaccurate - GoogleChrome/lighthouse#16869 |
Static script hints were built from getScriptPreloadAttrs but copied only href, rel, as and crossOrigin, so Chromium started hinted preloads at MEDIUM before the HTML's fetchPriority="low" arrived. Build them with the shared addEarlyHintFetchAttrs helper, which the dynamic hint path already uses, so fetchPriority (and crossOrigin) are carried into 103 hints and the response Link header. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Add a Solid server-suite SSR test for module and IIFE preloads and assert fetchpriority="low" on rendered preloads in the Solid and Vue HeadContent tests. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Mention that JavaScript Early Hints carry fetchpriority=low and extend the changeset to start-server-core. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1e9b816 to
8606e1a
Compare
🦋 Changeset detectedLatest commit: 8606e1a The changes in this PR will be included in the next version bump. This PR includes changesets to release 23 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
View your CI Pipeline Execution ↗ for commit 8606e1a
☁️ Nx Cloud last updated this comment at |
Merging this PR will not alter performanceℹ️ 31 benchmarks have no flame graph
Comparing 
Footnotes
|
What
Generated script preloads, meaning the
modulepreloadlinks (orpreload as="script"for IIFE output) thatHeadContentrenders from the Start manifest, now carryfetchPriority="low". Static Early Hints and the responseLinkheader now carry it too.Server-rendered HTML doesn't need this JavaScript to paint. Until now Chrome fetched it at High, so on bandwidth-bound connections it competed with render-blocking stylesheets, fonts and in-viewport images. With
low, those go first, and the JS still arrives in time for hydration. React already uses this default for its ownbootstrapModulespreloads, and Next.js inherits it.Changes
getScriptPreloadAttrs(router-core) returnsfetchPriority: 'low'. React, Solid and VueHeadContentall render through this helper, and so does Early Hints.collectStaticHintsFromManifest(start-server-core) builds script hints with the sharedaddEarlyHintFetchAttrs, sofetchPriorityis forwarded along withcrossOrigin. Without this, Chromium had already started the hinted fetch atMEDIUMbefore the HTML attribute arrived.fetchpriority=low.react-start/early-hintse2e: 103 and responseLinkheader assertions.Evidence
Applied-throttling measurements in this thread show large first-paint gains when the preload set is big, with hydration time unchanged:
With small preload sets, the real-world difference is about zero. Lighthouse's simulated gains on small sets are a Lantern artifact, which GoogleChrome/lighthouse#16869 addresses.
Deliberately out of scope
ssr: false,'data-only'and the SPA shell as well. Stylesheets block rendering even for content drawn by client JS, and React also lowers its bootstrap preloads unconditionally.fetchPriorityon the manifest link viatransformAssets, so that the HTML, Early Hints and hydration all see the same value.ReactDOM.preloadModule) are a follow-up. They need React ≥ 19.3 and a server-only branch.<head>element ordering:link[stylesheet]should come beforelink[modulepreload]#6749) gets its own PR, with an HTTP/1.1 measurement.Closes #8546
🤖 Generated with Claude Code
Summary by CodeRabbit