Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

experimental: css inlining #72195

Merged
merged 9 commits into from
Nov 20, 2024
Merged

experimental: css inlining #72195

merged 9 commits into from
Nov 20, 2024

Conversation

gaojude
Copy link
Contributor

@gaojude gaojude commented Nov 1, 2024

Introducing experimental flag inlineCSS. This supports both Webpack and Turbopack on App Router.

When the flag is enabled, all the places where we originally generate the <link> tag will instead be generating a <style> tag. The CSS precedence/deduping is maintained by React 19 automatically.

Note that this flag will apply to all CSS assets for now.

This is an optimization of LCP/FCP for websites that

  1. have CSS size being constant as a function of interaction
  2. fresh visits are common

This flag is experimental now, so please do not use at production.

@ijjk ijjk added created-by: Next.js team PRs by the Next.js team. type: next labels Nov 1, 2024
@gaojude gaojude force-pushed the jude/experimental-css-inline branch 2 times, most recently from 7bac059 to e6cfb50 Compare November 1, 2024 21:43
@ijjk ijjk added the tests label Nov 1, 2024
@gaojude gaojude force-pushed the jude/experimental-css-inline branch from 28574e9 to 64cdf3c Compare November 1, 2024 21:53
@gaojude gaojude requested a review from ztanner November 1, 2024 21:54
@vercel vercel deleted a comment from ijjk Nov 1, 2024
@vercel vercel deleted a comment from ijjk Nov 1, 2024
@ijjk
Copy link
Member

ijjk commented Nov 1, 2024

Tests Passed

@ijjk
Copy link
Member

ijjk commented Nov 1, 2024

Stats from current PR

Default Build (Increase detected ⚠️)
General Overall increase ⚠️
vercel/next.js canary vercel/next.js jude/experimental-css-inline Change
buildDuration 19.5s 15.8s N/A
buildDurationCached 15s 14.2s N/A
nodeModulesSize 404 MB 404 MB ⚠️ +22.9 kB
nextStartRea..uration (ms) 470ms 477ms N/A
Client Bundles (main, webpack)
vercel/next.js canary vercel/next.js jude/experimental-css-inline Change
0b69cffb-HASH.js gzip 52.6 kB 52.6 kB N/A
1924.HASH.js gzip 169 B 169 B
195-HASH.js gzip 46.5 kB 46.5 kB N/A
8589-HASH.js gzip 5.27 kB 5.28 kB N/A
framework-HASH.js gzip 57.4 kB 57.4 kB N/A
main-app-HASH.js gzip 232 B 230 B N/A
main-HASH.js gzip 33.1 kB 33.1 kB N/A
webpack-HASH.js gzip 1.71 kB 1.71 kB N/A
Overall change 169 B 169 B
Legacy Client Bundles (polyfills)
vercel/next.js canary vercel/next.js jude/experimental-css-inline Change
polyfills-HASH.js gzip 39.4 kB 39.4 kB
Overall change 39.4 kB 39.4 kB
Client Pages
vercel/next.js canary vercel/next.js jude/experimental-css-inline Change
_app-HASH.js gzip 193 B 193 B
_error-HASH.js gzip 192 B 190 B N/A
amp-HASH.js gzip 510 B 510 B
css-HASH.js gzip 341 B 343 B N/A
dynamic-HASH.js gzip 1.84 kB 1.84 kB N/A
edge-ssr-HASH.js gzip 266 B 266 B
head-HASH.js gzip 362 B 364 B N/A
hooks-HASH.js gzip 393 B 392 B N/A
image-HASH.js gzip 4.42 kB 4.42 kB N/A
index-HASH.js gzip 268 B 268 B
link-HASH.js gzip 2.77 kB 2.77 kB N/A
routerDirect..HASH.js gzip 328 B 327 B N/A
script-HASH.js gzip 398 B 396 B N/A
withRouter-HASH.js gzip 325 B 322 B N/A
1afbb74e6ecf..834.css gzip 106 B 106 B
Overall change 1.34 kB 1.34 kB
Client Build Manifests
vercel/next.js canary vercel/next.js jude/experimental-css-inline Change
_buildManifest.js gzip 746 B 749 B N/A
Overall change 0 B 0 B
Rendered Page Sizes
vercel/next.js canary vercel/next.js jude/experimental-css-inline Change
index.html gzip 522 B 523 B N/A
link.html gzip 538 B 537 B N/A
withRouter.html gzip 520 B 519 B N/A
Overall change 0 B 0 B
Edge SSR bundle Size
vercel/next.js canary vercel/next.js jude/experimental-css-inline Change
edge-ssr.js gzip 128 kB 128 kB N/A
page.js gzip 199 kB 199 kB N/A
Overall change 0 B 0 B
Middleware size
vercel/next.js canary vercel/next.js jude/experimental-css-inline Change
middleware-b..fest.js gzip 673 B 668 B N/A
middleware-r..fest.js gzip 156 B 155 B N/A
middleware.js gzip 30.9 kB 31 kB N/A
edge-runtime..pack.js gzip 844 B 844 B
Overall change 844 B 844 B
Next Runtimes
vercel/next.js canary vercel/next.js jude/experimental-css-inline Change
732-experime...dev.js gzip 322 B 322 B
732.runtime.dev.js gzip 314 B 314 B
app-page-exp...dev.js gzip 320 kB 320 kB N/A
app-page-exp..prod.js gzip 123 kB 123 kB N/A
app-page-tur..prod.js gzip 136 kB 136 kB N/A
app-page-tur..prod.js gzip 131 kB 131 kB N/A
app-page.run...dev.js gzip 310 kB 311 kB N/A
app-page.run..prod.js gzip 119 kB 119 kB N/A
app-route-ex...dev.js gzip 36 kB 36 kB
app-route-ex..prod.js gzip 24.4 kB 24.4 kB
app-route-tu..prod.js gzip 24.4 kB 24.4 kB
app-route-tu..prod.js gzip 24.2 kB 24.2 kB
app-route.ru...dev.js gzip 37.6 kB 37.6 kB
app-route.ru..prod.js gzip 24.2 kB 24.2 kB
pages-api-tu..prod.js gzip 9.57 kB 9.57 kB
pages-api.ru...dev.js gzip 11.4 kB 11.4 kB
pages-api.ru..prod.js gzip 9.56 kB 9.56 kB
pages-turbo...prod.js gzip 21 kB 21 kB
pages.runtim...dev.js gzip 26.6 kB 26.6 kB
pages.runtim..prod.js gzip 21 kB 21 kB
server.runti..prod.js gzip 916 kB 916 kB N/A
Overall change 271 kB 271 kB
build cache Overall increase ⚠️
vercel/next.js canary vercel/next.js jude/experimental-css-inline Change
0.pack gzip 2.02 MB 2.02 MB N/A
index.pack gzip 146 kB 148 kB ⚠️ +1.58 kB
Overall change 146 kB 148 kB ⚠️ +1.58 kB
Diff details
Diff for page.js

Diff too large to display

Diff for middleware.js

Diff too large to display

Diff for edge-ssr.js

Diff too large to display

Diff for image-HASH.js
@@ -1,7 +1,7 @@
 (self["webpackChunk_N_E"] = self["webpackChunk_N_E"] || []).push([
   [2983],
   {
-    /***/ 6745: /***/ (
+    /***/ 7391: /***/ (
       __unused_webpack_module,
       __unused_webpack_exports,
       __webpack_require__
@@ -9,7 +9,7 @@
       (window.__NEXT_P = window.__NEXT_P || []).push([
         "/image",
         function () {
-          return __webpack_require__(5675);
+          return __webpack_require__(1489);
         },
       ]);
       if (false) {
@@ -18,7 +18,7 @@
       /***/
     },
 
-    /***/ 9053: /***/ (module, exports, __webpack_require__) => {
+    /***/ 9313: /***/ (module, exports, __webpack_require__) => {
       "use strict";
       /* __next_internal_client_entry_do_not_use__  cjs */
       Object.defineProperty(exports, "__esModule", {
@@ -40,17 +40,17 @@
         __webpack_require__(6093)
       );
       const _head = /*#__PURE__*/ _interop_require_default._(
-        __webpack_require__(8808)
+        __webpack_require__(7964)
       );
-      const _getimgprops = __webpack_require__(1945);
-      const _imageconfig = __webpack_require__(7668);
-      const _imageconfigcontextsharedruntime = __webpack_require__(1694);
-      const _warnonce = __webpack_require__(1876);
-      const _routercontextsharedruntime = __webpack_require__(5575);
+      const _getimgprops = __webpack_require__(8821);
+      const _imageconfig = __webpack_require__(664);
+      const _imageconfigcontextsharedruntime = __webpack_require__(6418);
+      const _warnonce = __webpack_require__(7360);
+      const _routercontextsharedruntime = __webpack_require__(4203);
       const _imageloader = /*#__PURE__*/ _interop_require_default._(
-        __webpack_require__(3589)
+        __webpack_require__(2489)
       );
-      const _usemergedref = __webpack_require__(6746);
+      const _usemergedref = __webpack_require__(2454);
       // This is replaced by webpack define plugin
       const configEnv = {
         deviceSizes: [640, 750, 828, 1080, 1200, 1920, 2048, 3840],
@@ -371,7 +371,7 @@
       /***/
     },
 
-    /***/ 6746: /***/ (module, exports, __webpack_require__) => {
+    /***/ 2454: /***/ (module, exports, __webpack_require__) => {
       "use strict";
 
       Object.defineProperty(exports, "__esModule", {
@@ -432,7 +432,7 @@
       /***/
     },
 
-    /***/ 1945: /***/ (
+    /***/ 8821: /***/ (
       __unused_webpack_module,
       exports,
       __webpack_require__
@@ -448,9 +448,9 @@
           return getImgProps;
         },
       });
-      const _warnonce = __webpack_require__(1876);
-      const _imageblursvg = __webpack_require__(6704);
-      const _imageconfig = __webpack_require__(7668);
+      const _warnonce = __webpack_require__(7360);
+      const _imageblursvg = __webpack_require__(5884);
+      const _imageconfig = __webpack_require__(664);
       const VALID_LOADING_VALUES =
         /* unused pure expression or super */ null && [
           "lazy",
@@ -824,7 +824,7 @@
       /***/
     },
 
-    /***/ 6704: /***/ (__unused_webpack_module, exports) => {
+    /***/ 5884: /***/ (__unused_webpack_module, exports) => {
       "use strict";
       /**
        * A shared function, used on both client and server, to generate a SVG blur placeholder.
@@ -879,7 +879,7 @@
       /***/
     },
 
-    /***/ 965: /***/ (
+    /***/ 9345: /***/ (
       __unused_webpack_module,
       exports,
       __webpack_require__
@@ -906,10 +906,10 @@
         },
       });
       const _interop_require_default = __webpack_require__(1739);
-      const _getimgprops = __webpack_require__(1945);
-      const _imagecomponent = __webpack_require__(9053);
+      const _getimgprops = __webpack_require__(8821);
+      const _imagecomponent = __webpack_require__(9313);
       const _imageloader = /*#__PURE__*/ _interop_require_default._(
-        __webpack_require__(3589)
+        __webpack_require__(2489)
       );
       function getImageProps(imgProps) {
         const { props } = (0, _getimgprops.getImgProps)(imgProps, {
@@ -941,7 +941,7 @@
       /***/
     },
 
-    /***/ 3589: /***/ (__unused_webpack_module, exports) => {
+    /***/ 2489: /***/ (__unused_webpack_module, exports) => {
       "use strict";
 
       Object.defineProperty(exports, "__esModule", {
@@ -976,7 +976,7 @@
       /***/
     },
 
-    /***/ 5675: /***/ (
+    /***/ 1489: /***/ (
       __unused_webpack_module,
       __webpack_exports__,
       __webpack_require__
@@ -993,8 +993,8 @@
 
       // EXTERNAL MODULE: ./node_modules/.pnpm/[email protected]/node_modules/react/jsx-runtime.js
       var jsx_runtime = __webpack_require__(6322);
-      // EXTERNAL MODULE: ./node_modules/.pnpm/next@file+..+main-repo+packages+next+next-packed.tgz_react-dom@19.0.0-rc-380f5d67-20241113_re_d7fg766ptstyt4prarg74ol27i/node_modules/next/image.js
-      var next_image = __webpack_require__(1695);
+      // EXTERNAL MODULE: ./node_modules/.pnpm/next@file+..+diff-repo+packages+next+next-packed.tgz_react-dom@19.0.0-rc-380f5d67-20241113_re_k6jswiqskvoeqe45yhuljotqne/node_modules/next/image.js
+      var next_image = __webpack_require__(8106);
       var image_default = /*#__PURE__*/ __webpack_require__.n(next_image); // ./pages/nextjs.png
       /* harmony default export */ const nextjs = {
         src: "/_next/static/media/nextjs.cae0b805.png",
@@ -1024,12 +1024,12 @@
       /***/
     },
 
-    /***/ 1695: /***/ (
+    /***/ 8106: /***/ (
       module,
       __unused_webpack_exports,
       __webpack_require__
     ) => {
-      module.exports = __webpack_require__(965);
+      module.exports = __webpack_require__(9345);
 
       /***/
     },
@@ -1039,7 +1039,7 @@
     /******/ var __webpack_exec__ = (moduleId) =>
       __webpack_require__((__webpack_require__.s = moduleId));
     /******/ __webpack_require__.O(0, [636, 6593, 8792], () =>
-      __webpack_exec__(6745)
+      __webpack_exec__(7391)
     );
     /******/ var __webpack_exports__ = __webpack_require__.O();
     /******/ _N_E = __webpack_exports__;
Diff for app-page-exp..ntime.dev.js

Diff too large to display

Diff for app-page-exp..time.prod.js

Diff too large to display

Diff for app-page-tur..time.prod.js

Diff too large to display

Diff for app-page-tur..time.prod.js

Diff too large to display

Diff for app-page.runtime.dev.js

Diff too large to display

Diff for app-page.runtime.prod.js

Diff too large to display

Diff for server.runtime.prod.js

Diff too large to display

Commit: 3ec0b7c

@gaojude gaojude force-pushed the jude/experimental-css-inline branch 4 times, most recently from 6e77d34 to 4020192 Compare November 12, 2024 21:49
@gaojude gaojude force-pushed the jude/experimental-css-inline branch 3 times, most recently from 4de36bd to 0c21888 Compare November 19, 2024 22:20
@ijjk ijjk added the Turbopack Related to Turbopack with Next.js. label Nov 20, 2024
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

h/t to @bgw for authoring the Rust change

@gaojude gaojude marked this pull request as ready for review November 20, 2024 01:35
@gaojude gaojude requested review from bgw and huozhi November 20, 2024 04:42
@feedthejim
Copy link
Contributor

I think it'd be preferable to also introduce an opt in option to be able to only inline particular stylesheets rather than all of them

Copy link
Contributor

@unstubbable unstubbable left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work! 🚀

Things I noticed while testing this:

  • HMR does not work with Turbopack (throws runtime error Error: No link element found for chunk static/chunks/app_global_aef7b9.css)
  • HMR works with Webpack, but a new style element is added for every edit, without removing the old ones. This means you can only add/overwrite styles; deleted styles will be preserved. It might also be a memory concern.
  • Source-mapping (apparently a Turbopack-only feature?) is broken. I guess we would need to add a prefix to the sourceMappingURL, now that it can't be resolved relative to the css file.

Maybe worth considering shipping this as a build-only feature first. Then you can tackle the dev issues in a follow-up.

Copy link

vercel bot commented Nov 20, 2024

Notifying the following users due to files changed in this PR based on this repo's notify modifiers:

@timneutkens, @ijjk, @shuding, @huozhi:

packages/next/src/server/config.ts

@gaojude
Copy link
Contributor Author

gaojude commented Nov 20, 2024

I think it'd be preferable to also introduce an opt in option to be able to only inline particular stylesheets rather than all of them

Per discussion: This is intended to be a niche use case. We’ll consider supporting selective inlining if we see significant demand for it. For now, this is limited to canary only, so it’s not something that should be adopted at scale.

@gaojude
Copy link
Contributor Author

gaojude commented Nov 20, 2024

Great work! 🚀

Things I noticed while testing this:

  • HMR does not work with Turbopack (throws runtime error Error: No link element found for chunk static/chunks/app_global_aef7b9.css)
  • HMR works with Webpack, but a new style element is added for every edit, without removing the old ones. This means you can only add/overwrite styles; deleted styles will be preserved. It might also be a memory concern.
  • Source-mapping (apparently a Turbopack-only feature?) is broken. I guess we would need to add a prefix to the sourceMappingURL, now that it can't be resolved relative to the css file.

Maybe worth considering shipping this as a build-only feature first. Then you can tackle the dev issues in a follow-up.

Thank you for testing. I've taken your advice and narrowed the scope to "production-only". Agreed that we should revisit the dev experience of CSS inlining.

@gaojude gaojude merged commit 4d76f98 into canary Nov 20, 2024
107 of 109 checks passed
@gaojude gaojude deleted the jude/experimental-css-inline branch November 20, 2024 22:03
samcx added a commit that referenced this pull request Nov 21, 2024
## Why?

Add documentation for experimental `inlineCss`.

```
import type { NextConfig } from 'next'

const nextConfig: NextConfig = {
  experimental: {
    inlineCss: true,
  },
}
```

- x-ref: #72195
@philwolstenholme
Copy link
Contributor

philwolstenholme commented Nov 22, 2024

I think it'd be preferable to also introduce an opt in option to be able to only inline particular stylesheets rather than all of them

Per discussion: This is intended to be a niche use case. We’ll consider supporting selective inlining if we see significant demand for it. For now, this is limited to canary only, so it’s not something that should be adopted at scale.

Having it all-or-nothing helps the 'improve FCP by removing all render-blocking CSS links' use case, because if a user selectively inlined some CSS files but not others there would still be a render-blocking pause while the browser fetched any non-inlined CSS. This way the user can't forget to inline a file accidentally, or someone on their team forgets to opt into inlining a new file and the benefits are lost.

In an ideal world what you'd want is an option where the non-inlined CSS is loaded asyncronously to avoid render-blocking, but then you're into critical CSS and FOUC (flash of unstyled content) territory and things get much more fiddly, especially with dynamic/CMS content affecting what content is shown and therefore what the critical CSS needs to cover.

It's +1 from me for keeping it all-or-nothing for now at least

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
created-by: Next.js team PRs by the Next.js team. tests Turbopack Related to Turbopack with Next.js. type: next
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants