search

microsoft / griffel

CSS-in-JS with ahead-of-time compilation ⚡️
https://griffel.js.org
MIT License
1.2k stars 61 forks source link

extraction-plugin: support for appDir in NextJS 13 #266

Closed Dwlad90 closed 1 year ago

Dwlad90 commented 2 years ago

Hi, I was trying to use NextJS v13 appDir mode with @griffel and I get the following errors:

Attempted import error: '__css' is not exported from '@griffel/react' (imported as '__css').

and

TypeError: core.createDOMRenderer is not a function
    at 4415 (/home/projects/nextjs-ltekvi/.next/server/chunks/792.js:3446:88)
    at __webpack_require__ (/home/projects/nextjs-ltekvi/.next/server/webpack-runtime.js:25:43)
    at 5933 (/home/projects/nextjs-ltekvi/.next/server/chunks/792.js:3829:23)
    at __webpack_require__ (/home/projects/nextjs-ltekvi/.next/server/webpack-runtime.js:25:43)
    at 9692 (/home/projects/nextjs-ltekvi/.next/server/chunks/792.js:3710:18)
    at __webpack_require__ (/home/projects/nextjs-ltekvi/.next/server/webpack-runtime.js:25:43)
    at 4312 (/home/projects/nextjs-ltekvi/.next/server/app/page.js:99:72)
    at __webpack_require__ (/home/projects/nextjs-ltekvi/.next/server/webpack-runtime.js:25:43)
    at eval (/home/projects/nextjs-ltekvi/node_modules/next/dist/compiled/react-server-dom-webpack/client.js:918:337)

There are errors only when using the '@griffel/next-extraction-plugin' package.

To reproduce these errors g to the Sandbox and run the command:

npm install && npx next build
layershifter commented 2 years ago

Hey, looks pretty strange TBH as __css is exported:

image

I will look into it in upcoming weeks.

Dwlad90 commented 2 years ago

Hey, looks pretty strange TBH as __css is exported:

image

I will look into it in upcoming weeks.

@layershifter, Yes, I checked, these lines are present in the built code, as well as the createDOMRenderer export. This is a very strange bug.

Thank you!

iammathew commented 2 years ago

Linking this discussion here, since I think it may be related 🤔 https://github.com/microsoft/fluentui/discussions/25384 I may be mistaken though

layershifter commented 1 year ago

@Dwlad90 it seems that I traced the problem, looks that it's happening in webpack loader from webpack-extraction-plugin.

Looks like Next.js (or its Webpack part) does not like how we emit imports there:

https://github.com/microsoft/griffel/blob/ebd004e9e8935e099c1873bf2d4841c773bb8417/packages/webpack-extraction-plugin/src/webpackLoader.ts#L107-L114

If imports for CSS are not there - then it works 😳 I did following change and then build is passing:

            const request = `import ${JSON.stringify(this.utils.contextify(this.context || this.rootContext, `griffel.css!=!${virtualLoaderPath}!${resourcePath}?style=${toURIComponent(result.cssRules.join('\n'))}`))};`;
-            this.callback(null, `${result.code}\n\n${request};`, result.sourceMap);
+            this.callback(null, `${result.code}`, result.sourceMap);
            return;

image

I will look more to understand why it happens and how to fix it later this/next week.

layershifter commented 1 year ago

Linking this discussion here, since I think it may be related 🤔 microsoft/fluentui#25384 I may be mistaken though

No, it's not related to that. This issue is purely related to CSS extraction.

Dwlad90 commented 1 year ago

@Dwlad90 it seems that I traced the problem, looks that it's happening in webpack loader from webpack-extraction-plugin.

Looks like Next.js (or its Webpack part) does not like how we emit imports there:

https://github.com/microsoft/griffel/blob/ebd004e9e8935e099c1873bf2d4841c773bb8417/packages/webpack-extraction-plugin/src/webpackLoader.ts#L107-L114

If imports for CSS are not there - then it works 😳 I did following change and then build is passing:

            const request = `import ${JSON.stringify(this.utils.contextify(this.context || this.rootContext, `griffel.css!=!${virtualLoaderPath}!${resourcePath}?style=${toURIComponent(result.cssRules.join('\n'))}`))};`;
-            this.callback(null, `${result.code}\n\n${request};`, result.sourceMap);
+            this.callback(null, `${result.code}`, result.sourceMap);
            return;

image

I will look more to understand why it happens and how to fix it later this/next week.

Thanks for the answer! Good to know. I'll be waiting for the fix.

layershifter commented 1 year ago
image

* my face when I found the reason *

@Dwlad90 For older version you just need a following patch in node_modules/@griffel/webpack-extraction-plugin/src/webpackLoader.js:

if (result) {
-       if (result.cssRules) {
+       if (result.cssRules.length > 0) {

For newer, I have a fix already in #289.

layershifter commented 1 year ago

289 fixed the problem, but #291 added an another one. For unknown reason React wants to execute our import:

image

layershifter commented 1 year ago

Build is passing if it will not be execute for server i.e. the whole condition will be wrapped into !isServer:

https://github.com/microsoft/griffel/blob/d144cdef12e57d575732a19a51daa4da7ede3707/packages/next-extraction-plugin/src/lib/next-extraction-plugin.ts#L71-L79

The problem is that then CSS does not load while it's generated: image

Need to check what happens there. Will look into it next week.

Dwlad90 commented 1 year ago

@layershifter, Thanks for the solution of the problem.

But I noticed that when I add the condition to process styles on the client assembly only, then Webpack successfully compiled and extract the styles. However on the page the CSS file wasn't loaded and, accordingly, there are no styles on the page.

To reproduce these errors go to the Sandbox and run the command:

npm install && npx next build && npm next start

Also I found another issue, when using the RendererProvider in the ./app/layout.tsx I get the following error:

info  - Collecting page data .TypeError: React__namespace.createContext is not a function
    at 4947 (file:///home/projects/nextjs-ikosio/.next/server/chunks/854.js:6871:60)
    at __webpack_require__ (file:///home/projects/nextjs-ikosio/.next/server/webpack-runtime.js:25:43)
    at 2562 (file:///home/projects/nextjs-ikosio/.next/server/chunks/854.js:7197:23)
    at __webpack_require__ (file:///home/projects/nextjs-ikosio/.next/server/webpack-runtime.js:25:43)
    at 3294 (file:///home/projects/nextjs-ikosio/.next/server/chunks/854.js:7089:18)
    at __webpack_require__ (file:///home/projects/nextjs-ikosio/.next/server/webpack-runtime.js:25:43)
    at 8748 (file:///home/projects/nextjs-ikosio/.next/server/app/page.js:140:72)
    at Function.__webpack_require__ (file:///home/projects/nextjs-ikosio/.next/server/webpack-runtime.js:25:43)
    at async collectGenerateParams (file:///home/projects/nextjs-ikosio/node_modules/next/dist/build/utils.js:713:17)
    at async eval (file:///home/projects/nextjs-ikosio/node_modules/next/dist/build/utils.js:853:36)

To reproduce these errors go to the Sandbox and run following command:

npm install && npx next build

Maybe it happens because of the limitations of the Server Components, currently the use of CSS-in-JS is not supported, proof.

If you uncomment the line

'use client'

in the ./app/layout.tsx file, so turn the component into Client Component, then the error disappears.

layershifter commented 1 year ago

But I noticed that when I add the condition to process styles on the client assembly only, then Webpack successfully compiled and extract the styles. However on the page the CSS file wasn't loaded and, accordingly, there are no styles on the page.

Yeah, it's the same problem that I faced. Will look into it more.

Also I found another issue, when using the RendererProvider in the ./app/layout.tsx I get the following error:

Out of curiosity, why do you need it with CSS extraction? It's not used when CSS extraction is in place.

in the ./app/layout.tsx file, so turn the component into Client Component, then the error disappears.

It's a thing that I don't really like. Components that use useState or useContext can be only Client Components, but makeStyles (__styles & __css) consuming a context for TextDirection...

taishinaritomi commented 1 year ago

303

I cloned the code and checked it out a bit, but I have never used nx, etc. and it seemed to take me a while to understand it, so I will share about the next-extraction-plugin here.

The first thing I did was to tell getGlobalCssLoader that it is app-dir. This way the client component should be able to extract the CSS.

https://github.com/microsoft/griffel/blob/16250ce781f4f8329270ad83fdead9922bdfc234/packages/next-extraction-plugin/src/lib/next-extraction-plugin.ts#L81-L96

const isAppDir = nextConfig.experimental?.appDir === true;
const appDirOptions = isAppDir
  ? {
      hasAppDir: true,
      experimental: { appDir: true },
    }
  : {};

  cssRules.unshift({
    test: /griffel\.css$/i,
    sideEffects: true,
    use: getGlobalCssLoader(
      {
        assetPrefix: config.assetPrefix,
        isClient: !isServer,
        isServer,
        isDevelopment: dev,
        future: nextConfig.future || {},
        experimental: nextConfig.experimental || {},
        ...appDirOptions,
      } as ConfigurationContext,
      () => lazyPostCSS(dir, getSupportedBrowsers(dir, dev), undefined),
      [],
    ),
  });

For the react server components, since hooks are not available You will need to rethink your design in order to get it to work. If griffel can be designed without using hooks, the css for "nextjs13 react server components" cannot be extracted by the virtualLoader, so it can be written out to a file once and it will work.(example)

Dwlad90 commented 1 year ago

Hi @layershifter, Maybe it will be helpful, a new webpack plugin of style9 now supports this feature. This is a link to a discussion about NextJS appDir and CSS-in-JS support. Can you check if this is relevant to @griffel?

layershifter commented 1 year ago

@Dwlad90 yes, this looks relevant and explains what current thing does work. I will look into it in upcoming weeks, but feel free to do the same if you are interested 😉

SukkaW commented 1 year ago

@Dwlad90 yes, this looks relevant and explains what current thing does work. I will look into it in upcoming weeks, but feel free to do the same if you are interested 😉

FYI, I have described the Next.js issue here: https://github.com/SukkaW/style9-webpack/issues/1#issuecomment-1406730266, and I quote:

https://github.com/vercel/next.js/blob/3a9bfe60d228fc2fd8fe65b76d49a0d21df4ecc7/packages/next/src/build/webpack/plugins/flight-client-entry-plugin.ts#L425-L429

const modRequest: string | undefined =
  mod.resourceResolveData?.path + mod.resourceResolveData?.query

Here, only path and query are passed down, and all loaders are ignored.

And you can find my workaround here: https://github.com/SukkaW/style9-webpack/compare/f468bf88782c8f578e45ae9dd9291e90bc45f97f...696f4e0645cbe1865de3249e5588a55e7d8c9775

In short, you can't use resource match query (!=!) and inline loader (!virtual-file-loader.js?{}!), as they will be dropped by Next.js Server Component CSS Extraction. You should pass the virtual CSS information directly to the noop.css (The actual file existing on the disk, required by the webpack) like this:

import 'noop.css?{ filename, source }'

Then you will need to create a loader to extract { filename, source } from the noop.css request.

layershifter commented 1 year ago

@Dwlad90 can you please re-test with the latest Next.js? I was able to compile your example without any issues:

image
Dwlad90 commented 1 year ago

@layershifter, Thank you. I confirm, sandbox of the comment have been compiled successful with latest versions of next and @griffel/* packages when I added 'use client' to layout.tsx.

layershifter commented 1 year ago

@Dwlad90 thanks for confirmation, closing.