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

Fix disabling of built-in CSS support if there is a custom loader #31078

Merged
merged 12 commits into from
Nov 29, 2021
Merged

Fix disabling of built-in CSS support if there is a custom loader #31078

merged 12 commits into from
Nov 29, 2021

Conversation

michel-kraemer
Copy link
Contributor

Fixes #30647

Explanation:

In Next.js 11, built-in CSS loaders were removed by searching all oneOf arrays in all webpack config rules and identifying the one where the first element had a __next_css_remove: true in its options. This rule was then removed.

Now, in Next.js 12, the rule contains other (non-CSS-related) loaders too and the element with the __next_css_remove: true is not the first one anymore. This leads to the issue that CSS-related loaders are not removed from the webpack config if a custom CSS configuration is detected, which in turn fails the build.

This pull requests makes it so that __next_css_remove now stores the number of CSS-related loaders, which are later removed from oneOf using splice while retaining other loaders in the array.

An integration test has been added that uses styled-jsx/webpack/loader, which should disable built-in CSS support and remove the loaders. Without the fix, this integration test fails and the project cannot be built.

Bug

  • Related issues linked using fixes #number
  • Integration tests added
  • Errors have helpful link attached, see contributing.md

Documentation / Examples

  • Make sure the linting passes by running yarn lint

@ijjk

This comment has been minimized.

@michel-kraemer
Copy link
Contributor Author

@sokra I think I found a reasonable solution. I updated the PR.

I named the symbol __next_css_remove so it's consistent with the attribute in the CSS extraction plugin and the CSS minifier.

Please review again.

@mehulmpt
Copy link

Waiting for this PR to land, we're not able to use Next 12 because we use custom CSS/Sass loader (which in turn is because of #27953)

@DevinWallKcl
Copy link
Contributor

DevinWallKcl commented Nov 22, 2021

Waiting for this PR to land, we're not able to use Next 12 because we use custom CSS/Sass loader (which in turn is because of #27953)

Same here, except were using a custom loader configuration for styled-jsx. This is preventing my team from upgrading to Next 12.

@ijjk

This comment has been minimized.

@ijjk

This comment has been minimized.

@ijjk

This comment has been minimized.

@ijjk

This comment has been minimized.

@michel-kraemer
Copy link
Contributor Author

It would be great if this could be merged. Like the others said, this bug is really preventing us from upgrading to Next.js 12.

@michel-kraemer
Copy link
Contributor Author

michel-kraemer commented Nov 26, 2021

I don't know why the tests are failing all over sudden. I don't think it has anything to do with the pull request (according to the error messages, which seem to be unrelated to the code I changed). Everything runs successfully on my machine.

@ijjk

This comment has been minimized.

@ijjk
Copy link
Member

ijjk commented Nov 27, 2021

Stats from current PR

Default Build (Increase detected ⚠️)
General Overall increase ⚠️
vercel/next.js canary michel-kraemer/next.js fix-disable-built-in-css Change
buildDuration 25.9s 22.9s -3s
buildDurationCached 4.2s 4.5s ⚠️ +298ms
nodeModulesSize 346 MB 346 MB ⚠️ +153 B
Page Load Tests Overall increase ✓
vercel/next.js canary michel-kraemer/next.js fix-disable-built-in-css Change
/ failed reqs 0 0
/ total time (seconds) 4.006 3.697 -0.31
/ avg req/sec 624 676.18 +52.18
/error-in-render failed reqs 0 0
/error-in-render total time (seconds) 2.099 2.046 -0.05
/error-in-render avg req/sec 1191.28 1221.8 +30.52
Client Bundles (main, webpack, commons)
vercel/next.js canary michel-kraemer/next.js fix-disable-built-in-css Change
450.HASH.js gzip 179 B 179 B
framework-HASH.js gzip 42.2 kB 42.2 kB
main-HASH.js gzip 28.4 kB 28.4 kB
webpack-HASH.js gzip 1.45 kB 1.45 kB
Overall change 72.2 kB 72.2 kB
Legacy Client Bundles (polyfills)
vercel/next.js canary michel-kraemer/next.js fix-disable-built-in-css Change
polyfills-HASH.js gzip 31 kB 31 kB
Overall change 31 kB 31 kB
Client Pages Overall decrease ✓
vercel/next.js canary michel-kraemer/next.js fix-disable-built-in-css Change
_app-HASH.js gzip 1.37 kB 1.37 kB
_error-HASH.js gzip 194 B 194 B
amp-HASH.js gzip 312 B 312 B
css-HASH.js gzip 327 B 326 B -1 B
dynamic-HASH.js gzip 2.38 kB 2.38 kB
head-HASH.js gzip 350 B 350 B
hooks-HASH.js gzip 635 B 635 B
image-HASH.js gzip 4.45 kB 4.45 kB
index-HASH.js gzip 263 B 263 B
link-HASH.js gzip 1.87 kB 1.87 kB
routerDirect..HASH.js gzip 321 B 321 B
script-HASH.js gzip 383 B 383 B
withRouter-HASH.js gzip 318 B 318 B
85e02e95b279..7e3.css gzip 107 B 107 B
Overall change 13.3 kB 13.3 kB -1 B
Client Build Manifests
vercel/next.js canary michel-kraemer/next.js fix-disable-built-in-css Change
_buildManifest.js gzip 460 B 460 B
Overall change 460 B 460 B
Rendered Page Sizes
vercel/next.js canary michel-kraemer/next.js fix-disable-built-in-css Change
index.html gzip 532 B 532 B
link.html gzip 545 B 545 B
withRouter.html gzip 526 B 526 B
Overall change 1.6 kB 1.6 kB

Diffs

Diff for _buildManifest.js
@@ -5,7 +5,7 @@ self.__BUILD_MANIFEST = {
   "/amp": ["static\u002Fchunks\u002Fpages\u002Famp-8c7b17a56b7abb6e.js"],
   "/css": [
     "static\u002Fcss\u002F94fdbc56eafa2039.css",
-    "static\u002Fchunks\u002Fpages\u002Fcss-deb2f2a41a5bf511.js"
+    "static\u002Fchunks\u002Fpages\u002Fcss-97182c5b8324021a.js"
   ],
   "/dynamic": [
     "static\u002Fchunks\u002Fpages\u002Fdynamic-0a48d89fced3e24b.js"
Diff for css-HASH.js
@@ -29,7 +29,7 @@
         5893
       );
       /* harmony import */ var _css_module_css__WEBPACK_IMPORTED_MODULE_1__ = __webpack_require__(
-        3350
+        1785
       );
       /* harmony import */ var _css_module_css__WEBPACK_IMPORTED_MODULE_1___default = /*#__PURE__*/ __webpack_require__.n(
         _css_module_css__WEBPACK_IMPORTED_MODULE_1__
@@ -48,7 +48,7 @@
       /***/
     },
 
-    /***/ 3350: /***/ function(module) {
+    /***/ 1785: /***/ function(module) {
       // extracted by mini-css-extract-plugin
       module.exports = { helloWorld: "css_helloWorld__qqNwY" };

Default Build with SWC (Decrease detected ✓)
General Overall increase ⚠️
vercel/next.js canary michel-kraemer/next.js fix-disable-built-in-css Change
buildDuration 24.8s 26s ⚠️ +1.2s
buildDurationCached 4.1s 4.2s ⚠️ +128ms
nodeModulesSize 346 MB 346 MB ⚠️ +153 B
Page Load Tests Overall decrease ⚠️
vercel/next.js canary michel-kraemer/next.js fix-disable-built-in-css Change
/ failed reqs 0 0
/ total time (seconds) 4.091 4.148 ⚠️ +0.06
/ avg req/sec 611.16 602.72 ⚠️ -8.44
/error-in-render failed reqs 0 0
/error-in-render total time (seconds) 2.075 2.186 ⚠️ +0.11
/error-in-render avg req/sec 1204.63 1143.55 ⚠️ -61.08
Client Bundles (main, webpack, commons)
vercel/next.js canary michel-kraemer/next.js fix-disable-built-in-css Change
450.HASH.js gzip 179 B 179 B
framework-HASH.js gzip 42.3 kB 42.3 kB
main-HASH.js gzip 28.6 kB 28.6 kB
webpack-HASH.js gzip 1.44 kB 1.44 kB
Overall change 72.5 kB 72.5 kB
Legacy Client Bundles (polyfills)
vercel/next.js canary michel-kraemer/next.js fix-disable-built-in-css Change
polyfills-HASH.js gzip 31 kB 31 kB
Overall change 31 kB 31 kB
Client Pages
vercel/next.js canary michel-kraemer/next.js fix-disable-built-in-css Change
_app-HASH.js gzip 1.35 kB 1.35 kB
_error-HASH.js gzip 180 B 180 B
amp-HASH.js gzip 305 B 305 B
css-HASH.js gzip 321 B 321 B
dynamic-HASH.js gzip 2.37 kB 2.37 kB
head-HASH.js gzip 342 B 342 B
hooks-HASH.js gzip 622 B 622 B
image-HASH.js gzip 4.47 kB 4.47 kB
index-HASH.js gzip 256 B 256 B
link-HASH.js gzip 1.91 kB 1.91 kB
routerDirect..HASH.js gzip 314 B 314 B
script-HASH.js gzip 375 B 375 B
withRouter-HASH.js gzip 309 B 309 B
85e02e95b279..7e3.css gzip 107 B 107 B
Overall change 13.2 kB 13.2 kB
Client Build Manifests Overall increase ⚠️
vercel/next.js canary michel-kraemer/next.js fix-disable-built-in-css Change
_buildManifest.js gzip 458 B 459 B ⚠️ +1 B
Overall change 458 B 459 B ⚠️ +1 B
Rendered Page Sizes
vercel/next.js canary michel-kraemer/next.js fix-disable-built-in-css Change
index.html gzip 532 B 532 B
link.html gzip 544 B 544 B
withRouter.html gzip 526 B 526 B
Overall change 1.6 kB 1.6 kB

Diffs

Diff for _buildManifest.js
@@ -5,7 +5,7 @@ self.__BUILD_MANIFEST = {
   "/amp": ["static\u002Fchunks\u002Fpages\u002Famp-8c7b17a56b7abb6e.js"],
   "/css": [
     "static\u002Fcss\u002F94fdbc56eafa2039.css",
-    "static\u002Fchunks\u002Fpages\u002Fcss-deb2f2a41a5bf511.js"
+    "static\u002Fchunks\u002Fpages\u002Fcss-97182c5b8324021a.js"
   ],
   "/dynamic": [
     "static\u002Fchunks\u002Fpages\u002Fdynamic-0a48d89fced3e24b.js"
Diff for css-HASH.js
@@ -29,7 +29,7 @@
         5893
       );
       /* harmony import */ var _css_module_css__WEBPACK_IMPORTED_MODULE_1__ = __webpack_require__(
-        3350
+        1785
       );
       /* harmony import */ var _css_module_css__WEBPACK_IMPORTED_MODULE_1___default = /*#__PURE__*/ __webpack_require__.n(
         _css_module_css__WEBPACK_IMPORTED_MODULE_1__
@@ -48,7 +48,7 @@
       /***/
     },
 
-    /***/ 3350: /***/ function(module) {
+    /***/ 1785: /***/ function(module) {
       // extracted by mini-css-extract-plugin
       module.exports = { helloWorld: "css_helloWorld__qqNwY" };
Commit: 043b823

@timneutkens timneutkens merged commit c8457e3 into vercel:canary Nov 29, 2021
@michel-kraemer michel-kraemer deleted the fix-disable-built-in-css branch November 29, 2021 17:35
@vercel vercel locked as resolved and limited conversation to collaborators Jan 27, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Regression in Next.js 12: styled-jsx webpack loader does not work
7 participants