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

Improve Next.js support #34905

Open
13 of 22 tasks
mnajdova opened this issue Oct 27, 2022 · 75 comments
Open
13 of 22 tasks

Improve Next.js support #34905

mnajdova opened this issue Oct 27, 2022 · 75 comments
Assignees
Labels
enhancement This is not a bug, nor a new feature nextjs umbrella For grouping multiple issues to provide a holistic view

Comments

@mnajdova
Copy link
Member

mnajdova commented Oct 27, 2022

Summary 💡

This issue will serve as an umbrella for all issues related to Next.js app router integration. It will be easier to have an overview of all the problems in one place.

Done

Opportunities to make it even better

const router = useRouter();
React.useEffect(() => {
const handleRouteChangeStart = (url, { shallow }) => {
if (!shallow) {
nProgressStart();
}
};

@mnajdova mnajdova added the umbrella For grouping multiple issues to provide a holistic view label Oct 27, 2022
@mnajdova mnajdova self-assigned this Oct 27, 2022
@RPDeshaies

This comment was marked as resolved.

@mnajdova

This comment was marked as resolved.

@mnajdova
Copy link
Member Author

There have been some movement on emotion's side for supporting it in ./app directory. I've forked their stackblitz and added few Material UI components and a custom theme, and it seems like it works 🎉 Here is the link to stackblitz. You can check the ./blog route that uses server component.

I would appreciate feedback if someone wants to try it. I personally want to try this on the blog page in the https://mui.com site to verify that everything works. I will create an example project based on the stackblitz later this week.

@barthicus
Copy link

barthicus commented Nov 24, 2022

@mnajdova That's a great news! I hope some day we can use mui in app dir for real :)
I tried your example and I was curious if that works with @mui-treasury/layout I use. Unfortunately some @emotion related errors pop up: TypeError: React.createContext is not a function so I stopped testing this further.
I only modified the root layout and added the package.
Link to my edited stackblitz: https://stackblitz.com/edit/vercel-next-js-zmhzbd

EDIT: Not sure if that helps in any way, maybe there is just a problem in that layout package but I thought I will share it anyway.

@mnajdova
Copy link
Member Author

After doing some more investigation, I noticed that not all Material UI components can be render as server components. I was lucky in the example that the Typography component was one of them. We will need to revisit this. I will be posting more updates here, or create a new issue that will track the progress on all components.

@mnajdova
Copy link
Member Author

Based on emotion-js/emotion#2928 (comment)

Note that you should only use Emotion with the so-called client components (we might add "use client"; directive to our files to make this obvious). They can render on the server just fine - for the initial SSRed/streamed HTML but server "refetches" won't render them on the server, they might be rendered on the client when the "refetch" response comes back. This is not Emotion's limitation - it's a limitation for all CSS-in-JS libs like Emotion (including Styled Components, styled-jsx and more)

Looks like the best way forward is to add the "use client" directive to all Material UI components, at least until emotion supports server components.

@Andarist
Copy link
Contributor

Looks like the best way forward is to add the "use client" directive to all Material UI components, at least until emotion supports server components.

You don't have to add it to all the client components (although that's certainly the easiest solution here). If you have a set of completely headless components that don't rely on Emotion (nor on Styled-Components or other CSS-in-JS libs) then, to the best of my current understanding, you don't have to add that directive there.

Server Components were designed in a way that is at fundamental odds with CSS-in-JS. So don't expect Emotion (or other popular CSS-in-JS libs) to introduce support for them. They just don't allow us to integrate with them anyhow.

However, that doesn't mean that you can't SSR client components using Emotion. The initial HTML response from the server can freely use the rendered client components. The problem is only when it comes to "server component (re)fetches" (where I'm not even exactly sure how to trigger them, although Next.js probably does those on navigation or smth).

You can still do this:

function ServerComponent({ id }) {
  const data = use(fetchData(id))
  return <ClientComponent data={data} />
}

Basically, with server components, you might want to move your data-loading concerns out of your client components. That's it. Note that server components are also unique as they support using async/await - so I expect that many people will want to split those concerns anyway (regardless of the fact if they are using CSS-in-JS or not).

Of course, full support of server components would be neat and would make things easier for consumers - and would only require "splitting" if you really want to use async/await, or if you have another reason to do so. But as mentioned - this is probably not going to happen for CSS-in-JS libs as the server component's design doesn't quite allow this.

@robcaldecott
Copy link
Contributor

I've started experimenting with this using a large app at work and there is an initial FOUC before the styles are applied which is proving tricky to debug. I don't see this on the Stackblitz example shared above but I'll see if I can provide an example, although it may be something the emotion team need to worry about.

@Andarist
Copy link
Contributor

I can't address something that I can't debug 😉 If you provide a repro case then I will happily jump onto it

@mnajdova
Copy link
Member Author

mnajdova commented Nov 29, 2022

You don't have to add it to all the client components (although that's certainly the easiest solution here). If you have a set of completely headless components that don't rely on Emotion (nor on Styled-Components or other CSS-in-JS libs) then, to the best of my current understanding, you don't have to add that directive there.

Totally 👍 MUI Base components that don’t have any interactions could be server components. All Material UI components anyway load emotion, my comment was referring to them.

Basically, with server components, you might want to move your data-loading concerns out of your client components. That's it. Note that server components are also unique as they support using async/await - so I expect that many people will want to split those concerns anyway (regardless of the fact if they are using CSS-in-JS or not).

100% agree with this. I think the misconception comes from the difference that using plain CSS would allow you do convert the UI components too.

@robcaldecott
Copy link
Contributor

I can't address something that I can't debug 😉 If you provide a repro case then I will happily jump onto it

Hey @Andarist @mnajdova here's a repo that shows the FOUC:

https://github.com/robcaldecott/nextjs-mui-app-folder

It's a single page with Container, Grid and two different TextField components. It's using [email protected]. The FOUC happens in both dev and production modes: just refresh the page and you'll see it.

I also see the following console error which may be relevant:

Warning: Each child in a list should have a unique "key" prop. See https://reactjs.org/link/warning-keys for more information.
    at style

I am using the EmotionRootStyleRegistry.tsx from the example above.

@Andarist
Copy link
Contributor

100% agree with this. I think the misconception comes from the dofference that using plain CSS would allow you do convert the UI components too.

Yeah, this is very well put. It's true that some components with "plain css" can be rendered using server components but the majority of them just can't. From my PoV... I wouldn't like to bother thinking in those terms when writing components in the first place. It's creating a mental overhead - "can I use styles in this server component?". It's much easier to just "forbid" this in the code and make the distinction clear (even with a lint rule or something). Having to restructure your server component with plain CSS just because you want to add some interactivity would also be a chore - it's just, IMHO, easier to start with an explicit split between those concerns and leave server components as something without styles at all.


@robcaldecott this was an interesting case to investigate!

You need to add an explicit <head/> element to your layout to fix this:

diff --git a/app/layout.tsx b/app/layout.tsx
index 9c052a8..68bb86c 100644
--- a/app/layout.tsx
+++ b/app/layout.tsx
@@ -4,6 +4,7 @@ import EmotionRootStyleRegistry from "./EmotionRootStyleRegistry";
 export default function RootLayout({ children }: { children: ReactNode }) {
   return (
     <html>
+      <head></head>
       <body>
         <EmotionRootStyleRegistry>{children}</EmotionRootStyleRegistry>
       </body>

@robcaldecott
Copy link
Contributor

@Andarist OMG thank you so much! I thought the original example had left that in by mistake, ha!

kodiakhq bot pushed a commit to vercel/next.js that referenced this issue Dec 2, 2022
### Update

We removed the `<head>` element checking for root layout in #41621. Since we also need `<head>` for preload in the future, and also css-in-js will require that. We're adding back the `head` element checking to make sure user always provide valid root layout including it.

### Issue

An issue was reported [here](mui/material-ui#34905 (comment)) that the Emotion/MUI site was suffering from FOUC.

After an inspection, I noticed that the SSRed HTML didn't contain the inserted styles at all - despite them being inserted through `useServerInsertedHTML`. I managed to debug it down and discovered that their layout was missing `<head></head>` and thus the stream transformer skipped the insertion altogether cause of this check:
https://github.com/vercel/next.js/blob/fbc98abab31a54dbc2b6c88a064b579a10f34871/packages/next/server/node-web-streams-helper.ts#L177-L183

I've figured that at the very least we could surface this as a console error in development to nudge the user to fix the missing `<head/>`

cc @huozhi 



Co-authored-by: Jiachi Liu <[email protected]>
@garronej
Copy link
Contributor

garronej commented Dec 22, 2022

tss-react now provides a setup helper for Next 13 appDir.
If you use it, emotion and consequently MUI will work as well.

@barthicus
Copy link

tss-react now provides a setup helper for Next 13 appDir. If you use it, emotion and consequently MUI will work as well.

@garronej You link navigates to a 404 page. Url was encoded and hash was replaced ;)
Working link: https://docs.tss-react.dev/ssr/next.js#app-dir

@ayushrajniwal-jtg
Copy link

ayushrajniwal-jtg commented Dec 22, 2022

@barthicus, would you mind creating a code sandbox for the same? I have used the same example mentioned in the document but couldn't SSR the MUI component.

I'm still getting the same error:

error - (sc_server)/node_modules/@mui/base/node/FormControlUnstyled/FormControlUnstyledContext.js (13:54) @ eval
error - TypeError: React.createContext is not a function
    at eval (webpack-internal:///(sc_server)/./node_modules/@mui/base/node/FormControlUnstyled/FormControlUnstyledContext.js:48:60)
    at (sc_server)/./node_modules/@mui/base/node/FormControlUnstyled/FormControlUnstyledContext.js (/home/e-ubuntu20/Desktop/Experiment/next-13-mui/.next/server/app/page.js:3087:1)
    at __webpack_require__ (/home/e-ubuntu20/Desktop/Experiment/next-13-mui/.next/server/webpack-runtime.js:33:42)
    at eval (webpack-internal:///(sc_server)/./node_modules/@mui/base/node/FormControlUnstyled/FormControlUnstyled.js:12:58)
    at (sc_server)/./node_modules/@mui/base/node/FormControlUnstyled/FormControlUnstyled.js (/home/e-ubuntu20/Desktop/Experiment/next-13-mui/.next/server/app/page.js:3076:1)
    at __webpack_require__ (/home/e-ubuntu20/Desktop/Experiment/next-13-mui/.next/server/webpack-runtime.js:33:42)
    at eval (webpack-internal:///(sc_server)/./node_modules/@mui/base/node/FormControlUnstyled/index.js:35:51)
    at (sc_server)/./node_modules/@mui/base/node/FormControlUnstyled/index.js (/home/e-ubuntu20/Desktop/Experiment/next-13-mui/.next/server/app/page.js:3109:1)
    at __webpack_require__ (/home/e-ubuntu20/Desktop/Experiment/next-13-mui/.next/server/webpack-runtime.js:33:42)
    at eval (webpack-internal:///(sc_server)/./node_modules/@mui/base/node/index.js:245:52) {
  type: 'TypeError',
  page: '/'
}

Here is the sandbox for the same: https://codesandbox.io/s/white-meadow-ft6ok7?file=/app/layout.tsx

@garronej
Copy link
Contributor

garronej commented Dec 23, 2022

@ayushrajniwal-jtg There you go, your playground fixed a demo setup

You can't use providers in server components: https://beta.nextjs.org/docs/rendering/server-and-client-components#rendering-third-party-context-providers-in-server-components

@ayushrajniwal-jtg
Copy link

ayushrajniwal-jtg commented Dec 23, 2022

Thanks for looking into it @garronej. However, the playground is not working either. Getting the same error as mentioned above

@barthicus, would you mind creating a code sandbox for the same? I have used the same example mentioned in the document but couldn't SSR the MUI component.

I'm still getting the same error:

error - (sc_server)/node_modules/@mui/base/node/FormControlUnstyled/FormControlUnstyledContext.js (13:54) @ eval
error - TypeError: React.createContext is not a function
    at eval (webpack-internal:///(sc_server)/./node_modules/@mui/base/node/FormControlUnstyled/FormControlUnstyledContext.js:48:60)
    at (sc_server)/./node_modules/@mui/base/node/FormControlUnstyled/FormControlUnstyledContext.js (/home/e-ubuntu20/Desktop/Experiment/next-13-mui/.next/server/app/page.js:3087:1)
    at __webpack_require__ (/home/e-ubuntu20/Desktop/Experiment/next-13-mui/.next/server/webpack-runtime.js:33:42)
    at eval (webpack-internal:///(sc_server)/./node_modules/@mui/base/node/FormControlUnstyled/FormControlUnstyled.js:12:58)
    at (sc_server)/./node_modules/@mui/base/node/FormControlUnstyled/FormControlUnstyled.js (/home/e-ubuntu20/Desktop/Experiment/next-13-mui/.next/server/app/page.js:3076:1)
    at __webpack_require__ (/home/e-ubuntu20/Desktop/Experiment/next-13-mui/.next/server/webpack-runtime.js:33:42)
    at eval (webpack-internal:///(sc_server)/./node_modules/@mui/base/node/FormControlUnstyled/index.js:35:51)
    at (sc_server)/./node_modules/@mui/base/node/FormControlUnstyled/index.js (/home/e-ubuntu20/Desktop/Experiment/next-13-mui/.next/server/app/page.js:3109:1)
    at __webpack_require__ (/home/e-ubuntu20/Desktop/Experiment/next-13-mui/.next/server/webpack-runtime.js:33:42)
    at eval (webpack-internal:///(sc_server)/./node_modules/@mui/base/node/index.js:245:52) {
  type: 'TypeError',
  page: '/'
}

Here is the sandbox for the same: https://codesandbox.io/s/white-meadow-ft6ok7?file=/app/layout.tsx

@garronej
Copy link
Contributor

@ayushrajniwal-jtg there you go a working demo:

https://github.com/garronej/mui-next-appdir-demo

Screen.Recording.2022-12-23.at.12.05.46.mov

@francismanansala
Copy link

@ayushrajniwal-jtg there you go a working demo:

https://github.com/garronej/mui-next-appdir-demo

Screen.Recording.2022-12-23.at.12.05.46.mov

I want to use this tss-react solution you provided for a personal project.

However, I tried running your demo and ran into an issue. I see it load initially then it runs into an error when trying to render the client components:
image

I'm using node v18.12.1 and on windows 10 if that helps. Do you know why I would be seeing this error when I'm using your demo?

@garronej
Copy link
Contributor

Hi @francismanansala,
Have you run the repo exactly as it's provided as per the instruction in the README?

I'm running it on MacOS with node v16.15.1

@mnajdova
Copy link
Member Author

@samuelsycamore Saw your reply on another thread. That's interesting. I didn't know about it. Is that mean MUI is going a similar direction as Chakra with it's Panda runtime, instead of relying on a 3rd party runtime?

@mwskwong yes, we are about to post the RFC for it today/next week.

@mj12albert mj12albert self-assigned this Jul 21, 2023
@oliviertassinari oliviertassinari added the enhancement This is not a bug, nor a new feature label Jul 22, 2023
@mnajdova
Copy link
Member Author

@mwskwong here is the Zero-runtime CSS in JS RFC - #38137

@bilalAlgoace

This comment was marked as off-topic.

@DiegoAndai DiegoAndai added this to the Material UI: v6 milestone Dec 27, 2023
@mj12albert mj12albert changed the title Improve Next.js 13 support Improve Next.js support Jan 25, 2024
@danilo-leal danilo-leal moved this to Selected in Material UI Feb 26, 2024
@DiegoAndai DiegoAndai removed this from the Material UI: v6 milestone Mar 4, 2024
@DiegoAndai
Copy link
Member

Moving this umbrella issue out of v6 as it includes both package and documentation issues. We'll review each issue separately to decide if it's included in the v6 scope.

@sgoodrow
Copy link

sgoodrow commented Mar 6, 2024

Does anyone have a working example of how to integrate NextJS' Link component into MUI theme for app router?

I haven't found an official example. The simple forward ref example I have been using neither supports the props exposed by NextJS' Link nor does it seem to work reliably (I had to disable prefetch as sometimes links wouldn't work?).

import { createTheme } from '@mui/material/styles';
import NextLink from 'next/link';
import { forwardRef } from 'react';

const LinkBehaviour = forwardRef(function LinkBehaviour(props, ref) {
    return <NextLink ref={ref} prefetch={false} {...props} />;
});

const theme = createTheme({
    components: {
        MuiLink: {
            defaultProps: {
                component: LinkBehaviour
            }
        },
        MuiButtonBase: {
            defaultProps: {
                LinkComponent: LinkBehaviour
            }
        }
    }
});

@ianldgs
Copy link

ianldgs commented Mar 6, 2024

@sgoodrow if it's appdir router, have you tried the following?

<Link component={NextLink} href="/" underline="hover" color="white">Home</Link>

@sgoodrow
Copy link

sgoodrow commented Mar 6, 2024

Not exactly, but I did set that as the default component for the Link in the theme. It doesn't seem that propagates automatically to the buttons, or that the type definitions for their props are expanded to support those provided by NextLink such as prefetch.

Is there no way to do this via the theme?

@siriwatknp

This comment was marked as resolved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement This is not a bug, nor a new feature nextjs umbrella For grouping multiple issues to provide a holistic view
Projects
Status: Selected
Development

No branches or pull requests