fix: Next plugin - pages router style loading (#1365)

vanilla-extract-css · Mar 28, 2024 · ed67731 · ed67731
1 parent 03ba50d
commit ed67731
Show file tree

Hide file tree

Showing 12 changed files with 135 additions and 58 deletions.
diff --git a/.changeset/honest-pugs-tap.md b/.changeset/honest-pugs-tap.md
@@ -0,0 +1,5 @@
+---
+'@vanilla-extract/next-plugin': minor
+---
+
+Pages router: use next-style-loader in dev, output css in ssr
diff --git a/fixtures/features/src/html.ts b/fixtures/features/src/html.ts
@@ -0,0 +1,12 @@
+import * as styles from './features.css';
+import testNodes from '../test-nodes.json';
+
+export default `
+    <div id="${testNodes.mergedStyle}" class="${styles.mergedStyle}">Merged style</div>
+    <div id="${testNodes.styleWithComposition}" class="${styles.styleWithComposition}">Style with composition</div>
+    <div id="${testNodes.styleVariantsWithComposition}" class="${styles.styleVariantsWithComposition.variant}">Style variants with composition</div>
+    <div id="${testNodes.styleVariantsWithMappedComposition}" class="${styles.styleVariantsWithMappedComposition.variant}">Style variants with mapped composition</div>
+    <div id="${testNodes.compositionOnly}" class="${styles.compositionOnly}">Composition only</div>
+    <div id="${testNodes.styleCompositionInSelector}" class="${styles.styleCompositionInSelector}">Style composition in selector</div>
+    <div id="${testNodes.styleVariantsCompositionInSelector}" class="${styles.styleVariantsCompositionInSelector.variant}">Style variants composition in selector</div>
+  `;
diff --git a/fixtures/features/src/index.ts b/fixtures/features/src/index.ts
@@ -1,16 +1,7 @@
-import * as styles from './features.css';
-import testNodes from '../test-nodes.json';
+import html from './html';
 
 function render() {
-  document.body.innerHTML = `
-    <div id="${testNodes.mergedStyle}" class="${styles.mergedStyle}">Merged style</div>
-    <div id="${testNodes.styleWithComposition}" class="${styles.styleWithComposition}">Style with composition</div>
-    <div id="${testNodes.styleVariantsWithComposition}" class="${styles.styleVariantsWithComposition.variant}">Style variants with composition</div>
-    <div id="${testNodes.styleVariantsWithMappedComposition}" class="${styles.styleVariantsWithMappedComposition.variant}">Style variants with mapped composition</div>
-    <div id="${testNodes.compositionOnly}" class="${styles.compositionOnly}">Composition only</div>
-    <div id="${testNodes.styleCompositionInSelector}" class="${styles.styleCompositionInSelector}">Style composition in selector</div>
-    <div id="${testNodes.styleVariantsCompositionInSelector}" class="${styles.styleVariantsCompositionInSelector.variant}">Style variants composition in selector</div>
-  `;
+  document.body.innerHTML = html;
 }
 
 render();

diff --git a/fixtures/next-app-router/src/app/features/page.tsx b/fixtures/next-app-router/src/app/features/page.tsx
@@ -0,0 +1,10 @@
+import html from '@fixtures/features/src/html';
+
+export default function Features() {
+  return (
+    <>
+      <span id="features" />
+      <div dangerouslySetInnerHTML={{ __html: html }} />
+    </>
+  );
+}
diff --git a/fixtures/next-app-router/src/app/sprinkles/page.tsx b/fixtures/next-app-router/src/app/sprinkles/page.tsx
@@ -0,0 +1,10 @@
+import html from '@fixtures/sprinkles/src/html';
+
+export default function Sprinkles() {
+  return (
+    <>
+      <span id="sprinkles" />
+      <div dangerouslySetInnerHTML={{ __html: html }} />
+    </>
+  );
+}
diff --git a/fixtures/next-pages-router/src/pages/features/index.tsx b/fixtures/next-pages-router/src/pages/features/index.tsx
@@ -0,0 +1,10 @@
+import html from '@fixtures/features/src/html';
+
+export default function Features() {
+  return (
+    <>
+      <span id="features" />
+      <div dangerouslySetInnerHTML={{ __html: html }} />
+    </>
+  );
+}
diff --git a/fixtures/next-pages-router/src/pages/sprinkles/index.tsx b/fixtures/next-pages-router/src/pages/sprinkles/index.tsx
@@ -0,0 +1,10 @@
+import html from '@fixtures/sprinkles/src/html';
+
+export default function Sprinkles() {
+  return (
+    <>
+      <span id="sprinkles" />
+      <div dangerouslySetInnerHTML={{ __html: html }} />
+    </>
+  );
+}
diff --git a/fixtures/sprinkles/src/html.ts b/fixtures/sprinkles/src/html.ts
@@ -0,0 +1,28 @@
+import {
+  sprinkles,
+  mapResponsiveValue,
+  normalizeResponsiveValue,
+  preComposedSprinkles,
+  preComposedSprinklesUsedInSelector,
+  container,
+} from './styles.css';
+import testNodes from '../test-nodes.json';
+
+export default `
+  <div id="${testNodes.root}" class="${container}">
+    <div class="${sprinkles({
+      display: normalizeResponsiveValue('block').mobile,
+      paddingTop: mapResponsiveValue(
+        {
+          mobile: 'small',
+          desktop: 'medium',
+        } as const,
+        (x) => x,
+      ),
+    })}">
+      Sprinkles
+    </div>
+    <div class="${preComposedSprinkles}">Precomposed sprinkles</div>
+    <div class="${preComposedSprinklesUsedInSelector}">Precomposed Sprinkles Used In Selector</div>
+  </div>
+`;
diff --git a/fixtures/sprinkles/src/index.ts b/fixtures/sprinkles/src/index.ts
@@ -1,32 +1,7 @@
-import {
-  sprinkles,
-  mapResponsiveValue,
-  normalizeResponsiveValue,
-  preComposedSprinkles,
-  preComposedSprinklesUsedInSelector,
-  container,
-} from './styles.css';
-import testNodes from '../test-nodes.json';
+import html from './html';
 
 function render() {
-  document.body.innerHTML = `
-  <div id="${testNodes.root}" class="${container}">
-    <div class="${sprinkles({
-      display: normalizeResponsiveValue('block').mobile,
-      paddingTop: mapResponsiveValue(
-        {
-          mobile: 'small',
-          desktop: 'medium',
-        } as const,
-        (x) => x,
-      ),
-    })}">
-      Sprinkles
-    </div>
-    <div class="${preComposedSprinkles}">Precomposed sprinkles</div>
-    <div class="${preComposedSprinklesUsedInSelector}">Precomposed Sprinkles Used In Selector</div>
-  </div>
-`;
+  document.body.innerHTML = html;
 }
 
 render();
diff --git a/packages/next-plugin/src/index.ts b/packages/next-plugin/src/index.ts
@@ -33,23 +33,53 @@ function getSupportedBrowsers(dir: string, isDevelopment: boolean) {
 const getVanillaExtractCssLoaders = (
   options: WebpackConfigContext,
   assetPrefix: string,
+  hasAppDir: boolean,
 ) => {
   const loaders: webpack.RuleSetUseItem[] = [];
 
   // Adopt from Next.js' getClientStyleLoader
   // https://github.com/vercel/next.js/blob/6e5b935fd7a61497f6854a81aec7df3a5dbf61ac/packages/next/src/build/webpack/config/blocks/css/loaders/client.ts#L3
   if (!options.isServer) {
-    // https://github.com/vercel/next.js/blob/6e5b935fd7a61497f6854a81aec7df3a5dbf61ac/packages/next/src/build/webpack/config/blocks/css/loaders/client.ts#L44
-    // next-style-loader will mess up css order in development mode.
-    // Next.js appDir doesn't use next-style-loader either.
-    // So we always use css-loader here, to simplify things and get proper order of output CSS
-    loaders.push({
-      loader: NextMiniCssExtractPlugin.loader,
-      options: {
-        publicPath: `${assetPrefix}/_next/`,
-        esModule: false,
-      },
-    });
+    // https://github.com/vercel/next.js/blob/6e5b935fd7a61497f6854a81aec7df3a5dbf61ac/packages/next/src/build/webpack/config/blocks/css/loaders/client.ts#L16
+    // Keep next-style-loader for development mode in `pages/`
+    if (options.dev && !hasAppDir) {
+      loaders.push({
+        loader: 'next-style-loader',
+        options: {
+          insert: function (element: Node) {
+            // By default, style-loader injects CSS into the bottom
+            // of <head>. This causes ordering problems between dev
+            // and prod. To fix this, we render a <noscript> tag as
+            // an anchor for the styles to be placed before. These
+            // styles will be applied _before_ <style jsx global>.
+
+            // These elements should always exist. If they do not,
+            // this code should fail.
+            const anchorElement = document.querySelector(
+              '#__next_css__DO_NOT_USE__',
+            )!;
+            const parentNode = anchorElement.parentNode!; // Normally <head>
+
+            // Each style tag should be placed right before our
+            // anchor. By inserting before and not after, we do not
+            // need to track the last inserted element.
+            parentNode.insertBefore(element, anchorElement);
+          },
+        },
+      });
+    } else {
+      // https://github.com/vercel/next.js/blob/6e5b935fd7a61497f6854a81aec7df3a5dbf61ac/packages/next/src/build/webpack/config/blocks/css/loaders/client.ts#L44
+      // next-style-loader will mess up css order in development mode.
+      // Next.js appDir doesn't use next-style-loader either.
+      // So we always use css-loader here, to simplify things and get proper order of output CSS
+      loaders.push({
+        loader: NextMiniCssExtractPlugin.loader,
+        options: {
+          publicPath: `${assetPrefix}/_next/`,
+          esModule: false,
+        },
+      });
+    }
   }
 
   const postcss = () =>
@@ -105,7 +135,7 @@ export const createVanillaExtractPlugin = (
   return (nextConfig: NextConfig = {}): NextConfig => ({
     ...nextConfig,
     webpack(config: any, options: WebpackConfigContext) {
-      const { dir, dev, isServer, config: resolvedNextConfig } = options;
+      const { dir, dev, config: resolvedNextConfig } = options;
 
       // https://github.com/vercel/next.js/blob/1fb4cad2a8329811b5ccde47217b4a6ae739124e/packages/next/build/index.ts#L336
       // https://github.com/vercel/next.js/blob/1fb4cad2a8329811b5ccde47217b4a6ae739124e/packages/next/build/webpack-config.ts#L626
@@ -118,11 +148,7 @@ export const createVanillaExtractPlugin = (
       // Skip nextConfig check since appDir is stable feature after Next.js 13.4
       const hasAppDir = !!(findPagesDirResult && findPagesDirResult.appDir);
 
-      const outputCss = hasAppDir
-        ? // Always output css since Next.js App Router needs to collect Server CSS from React Server Components
-          true
-        : // There is no appDir, do not output css on server build
-          !isServer;
+      const outputCss = true;
 
       // https://github.com/vercel/next.js/blob/6e5b935fd7a61497f6854a81aec7df3a5dbf61ac/packages/next/src/build/webpack/config/helpers.ts#L12-L21
       const cssRules = config.module.rules.find(
@@ -143,6 +169,7 @@ export const createVanillaExtractPlugin = (
         use: getVanillaExtractCssLoaders(
           options,
           resolvedNextConfig.assetPrefix,
+          hasAppDir,
         ),
       });
 

diff --git a/test-helpers/src/startFixture/next.ts b/test-helpers/src/startFixture/next.ts
@@ -17,7 +17,7 @@ const DIST_DIR = 'dist';
 
 // these are the fixtures that are currently
 // configured as routes in the @fixtures/next-* apps
-export const nextFixtures = ['recipes'] as const;
+export const nextFixtures = ['sprinkles', 'recipes', 'features'] as const;
 
 export interface NextFixtureOptions {
   type: 'next-app-router' | 'next-pages-router';

diff --git a/tests/e2e/fixtures-next-development.playwright.ts b/tests/e2e/fixtures-next-development.playwright.ts
@@ -11,8 +11,7 @@ const testCases = [
   {
     type: 'next-pages-router',
     mode: 'development',
-    // @TODO - enable after next plugin fix
-    clientSideRouting: false,
+    clientSideRouting: true,
   },
   {
     type: 'next-app-router',