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

[examples] Fix CSS modules integration #30381

Merged

Conversation

oliviertassinari
Copy link
Member

@oliviertassinari oliviertassinari commented Dec 23, 2021

This is also important when using Tailwind CSS

@oliviertassinari oliviertassinari added the docs Improvements or additions to the documentation label Dec 23, 2021
@mui-pr-bot
Copy link

mui-pr-bot commented Dec 23, 2021

No bundle size changes

Generated by 🚫 dangerJS against c0dbba9

@oliviertassinari oliviertassinari force-pushed the SSR-style-injection-order branch from 3b645e9 to ffe52eb Compare December 23, 2021 11:34
@oliviertassinari oliviertassinari force-pushed the SSR-style-injection-order branch from ffe52eb to 88a00a6 Compare December 23, 2021 11:34
@oliviertassinari oliviertassinari marked this pull request as draft December 23, 2021 12:46
@oliviertassinari oliviertassinari force-pushed the SSR-style-injection-order branch 6 times, most recently from 795c5ac to c2854e0 Compare December 23, 2021 13:46
@@ -11,7 +11,7 @@ import { useUserLanguage } from 'docs/src/modules/utils/i18n';
*
* - /docs/src/modules/components/Link.tsx
* - /examples/nextjs/src/Link.tsx
* - /examples/nextjs-with-typescript/src/components/Link.tsx
* - /examples/nextjs-with-typescript/src/Link.tsx
Copy link
Member Author

@oliviertassinari oliviertassinari Dec 23, 2021

Choose a reason for hiding this comment

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

Flatten the src folder to make it easier to discover for new users (less overwhelming).

Comment on lines +27 to +29
<!-- #default-branch-switch -->

[![Edit on StackBlitz](https://developer.stackblitz.com/img/open_in_stackblitz.svg)](https://stackblitz.com/github/mui-org/material-ui/tree/master/examples/create-react-app-with-typescript)
Copy link
Member Author

Choose a reason for hiding this comment

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

An alternative

Comment on lines -21 to -25
or:

<!-- #default-branch-switch -->

[![Edit on CodeSandbox](https://codesandbox.io/static/img/play-codesandbox.svg)](https://codesandbox.io/s/github/mui-org/material-ui/tree/master/examples/gatsby)
Copy link
Member Author

Choose a reason for hiding this comment

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

It's not working and Gatsby's popularity is low, trending downward, so probably not worth trying to fix this.

import * as React from 'react';
import { CacheProvider } from '@emotion/react';
import createEmotionServer from '@emotion/server/create-instance';
import { renderToString } from 'react-dom/server';
import getEmotionCache from './getEmotionCache';

// Inject MUI styles first to match with the prepend: true configuration.
export const onPreRenderHTML = ({ getHeadComponents, replaceHeadComponents }) => {
Copy link
Member Author

@oliviertassinari oliviertassinari Dec 23, 2021

Choose a reason for hiding this comment

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

This fixes the initial HTML output (before JS hydration), #30358 was not enough.

Comment on lines -1 to -5
{
"container": {
"node": "12"
}
}
Copy link
Member Author

Choose a reason for hiding this comment

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

Legacy file for an old bug with Codesandbox

@@ -8,7 +8,6 @@
"@emotion/styled": "latest",
"@emotion/server": "latest",
"@mui/material": "latest",
"clsx": "latest",
Copy link
Member Author

Choose a reason for hiding this comment

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

No longer needed with the styled() API

Comment on lines +1 to +3
module.exports = {
reactStrictMode: true,
};
Copy link
Member Author

Choose a reason for hiding this comment

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

A healthy default for any new project. It was added in #29974 for TypeScript but we forgot about the JS version.

@@ -21,7 +21,6 @@ export default function Head(props) {
const preview = card.startsWith('http') ? card : `${HOST}${card}`;
return (
<NextHead>
<meta name="viewport" content="initial-scale=1, width=device-width" />
Copy link
Member Author

Choose a reason for hiding this comment

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

It's stable, no need to set it on each page. _app should do it.

@@ -6,7 +6,8 @@
"dev": "next dev",
"build": "next build",
"start": "next start",
"lint": "next lint"
"lint": "next lint",
"post-update": "echo \"codesandbox preview only, need an update\" && yarn upgrade --latest"
Copy link
Member Author

Choose a reason for hiding this comment

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

Fix regression, it was removed in #29974.

@@ -19,7 +19,6 @@ export default function MyApp(props: MyAppProps) {
return (
<CacheProvider value={emotionCache}>
<Head>
<title>Change title in _app.tsx</title>
Copy link
Member Author

Choose a reason for hiding this comment

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

Should be set on each page, not once for all.

@oliviertassinari oliviertassinari marked this pull request as ready for review December 23, 2021 13:48
@oliviertassinari oliviertassinari changed the title [examples] Fix CSS modules integration in Gatsby [examples] Fix CSS modules integration Dec 23, 2021
Comment on lines -21 to -25
or:

<!-- #default-branch-switch -->

[![Edit on CodeSandbox](https://codesandbox.io/static/img/play-codesandbox.svg)](https://codesandbox.io/s/github/mui-org/material-ui/tree/master/examples/ssr)
Copy link
Member Author

Choose a reason for hiding this comment

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

It's not working.

noLinkStyle?: boolean;
} & Omit<NextLinkComposedProps, 'to' | 'linkAs' | 'href'> &
Omit<MuiLinkProps, 'href'>;

// A styled version of the Next.js Link component:
// https://nextjs.org/docs/#with-link
// https://nextjs.org/docs/api-reference/next/link
Copy link
Member Author

Choose a reason for hiding this comment

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

Regression introduced in #29974

@oliviertassinari oliviertassinari force-pushed the SSR-style-injection-order branch from c2854e0 to c0dbba9 Compare December 23, 2021 13:53
@oliviertassinari oliviertassinari added the examples Relating to /examples label Dec 23, 2021
Copy link
Member

@siriwatknp siriwatknp left a comment

Choose a reason for hiding this comment

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

👍 Great improvement, especially for the CSS modules integration!

@oliviertassinari oliviertassinari merged commit 31924ee into mui:master Dec 24, 2021
@oliviertassinari oliviertassinari deleted the SSR-style-injection-order branch December 24, 2021 21:21
@RemyMachado
Copy link

I tried the latest nextjs-with-typescript example and I couldn't get it to work with CSS-modules.
There's still a FOUC.

Since the title of this PR is [examples] Fix CSS modules integration I was hoping it would be solved.
Do you have inputs on this? Should it be already working, but I'm not doing it the right way?
I would greatly appreciate indications.

@oliviertassinari
Copy link
Member Author

@RemyMachado Do you have a reproduction I can look at?

@RemyMachado
Copy link

@oliviertassinari
Here's the codesandbox https://codesandbox.io/s/fooc-mui-nextjs-css-modules-ozdpo?file=/pages/index.module.css

I just used the latest nextjs-typescript example available on mui repository and added an index.module.css

When inspecting the network devtool. I can see that the server is only responsding with the emotion styling. Completely ignoring the css modules.

We are getting:
image

Instead of:
image

@ShuPink
Copy link
Contributor

ShuPink commented Dec 30, 2021

Is the FOUC issue present in your production build as well?
I think Next.js handles the loading of cssmodules a bit differently on dev and prod for hot reloading purposes

I tried running your sandbox in prod and I could see the css sheets show up in the head:
image

@RemyMachado
Copy link

RemyMachado commented Dec 30, 2021

Indeed, these links are showing up only on production:

     <link rel="preload" href="/_next/static/css/b8bc79d340957044.css" as="style"/>
     <link rel="stylesheet" href="/_next/static/css/b8bc79d340957044.css" data-n-p=""/>

I can't tell if it's the right file b8bc79d....css However, the FOUC is still there. It seems that the css stylesheet isn't properly used. Maybe it's overridden, I have no clue.

@oliviertassinari
Copy link
Member Author

oliviertassinari commented Dec 31, 2021

@RemyMachado Ok thanks for the extra details, This is not related to MUI AFAIK. It's specific to Next.js. I have taken your reproduction and removed MUI to prove it: https://github.com/oliviertassinari/fooc-nextjs-css-modules.

Now, AFAIK, there is no FOUC regardless of the content of the HTML output in dev mode. Why? Next.js loads the CSS using a JS script that is blocking until the first pain. Still, feel free to report the pain point to Next.js. I was also confused to not see the CSS loading with JavaScript disabled in dev mode at first.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Improvements or additions to the documentation examples Relating to /examples
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[examples] Nextjs CSS modules insert before emotion
5 participants