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

[core] Add use client directive to all components in a new build step #37503

Closed
mj12albert opened this issue Jun 5, 2023 · 11 comments · Fixed by #37656
Closed

[core] Add use client directive to all components in a new build step #37503

mj12albert opened this issue Jun 5, 2023 · 11 comments · Fixed by #37656
Assignees
Labels
enhancement This is not a bug, nor a new feature nextjs

Comments

@mj12albert
Copy link
Member

mj12albert commented Jun 5, 2023

Part of #34905

Next.js 13 considers all components in the app directory server components by default, requiring client components to be identified with a 'use client' directive.

A new build step similar to yarn docs:api:build will be added to add 'use client' to all Material UI, Base UI and Joy UI components

@mj12albert mj12albert added the core Infrastructure work going on behind the scenes label Jun 5, 2023
@mj12albert mj12albert self-assigned this Jun 5, 2023
@oalexdoda
Copy link

I looked into this for one of my packages, and decided to NOT do it as part of the core module because it's too platform specific. The use client directive is not a generic JS directive, but rather a Next.js specific one.

I do, however, wish that MUI implemented a more SSR-friendly approach for components. Especially for super common ones like Box, Stack, Divider, and others that force us to go client-side just because of refs. Even in cases where all we use them for are just as a replacement for default flexbox / divs.

Your best bet would be to just use client in a wrapper component where you reference those components.

@MostafaKMilly
Copy link

Any updates ?

@mwskwong
Copy link
Contributor

mwskwong commented Jun 19, 2023

I think MUI can instead publish a script to handle this as a separate package instead. Taking reference from similar offerings, e.g. Chakra UI has a dedicated package @chakra-ui/next-js for Next.js integration. Not exactly related to use client but the idea is similar. Perhaps MUI can do something similar?

@mj12albert
Copy link
Member Author

@altechzilla Thanks for sharing ~

The use client directive is not a generic JS directive, but rather a Next.js specific one

You are right that it is not part of normal JS, but to my understanding it is specific to React Server Components (per the related RFC) and not Next.js, it's just that this issue is highly visible in Next 13 apps because the new app directory makes full use of RSC!

I do, however, wish that MUI implemented a more SSR-friendly approach for components. Especially for super common ones like Box, Stack, Divider

Making these common components (layout components in particular) work as React Server components is definitely on the agenda!

@mj12albert
Copy link
Member Author

Chakra UI has a dedicated package @chakra-ui/next-js for Next.js integration.

@mwskwong Thanks for sharing this, I'll try it out ~

I think MUI can instead publish a script to handle this as a separate package instead

At this point, we're just trying to get the MUI libraries working out of the box in a Next 13 app directory without requiring any extra installation steps or config changes!

@mwskwong
Copy link
Contributor

mwskwong commented Jun 20, 2023

At this point, we're just trying to get the MUI libraries working out of the box in a Next 13 app directory without requiring any extra installation steps or config changes!

Which is the best DX IMO. I think there are other projects already start doing it. If I have to say, I will properly think MUI Joy is the best place to start. I would imagine it is more approachable than doing it in MUI Material v6.

@kevinpastor
Copy link

As a temporary workaround, I have setup something that utilizes TypeScript path aliases to provide the behaviour we want.

First, create a file (I've called it material.ts) with the following content:

"use client";

// "node_modules" is required here to make sure we don't go into a recursive loop with the path alias.
export * from "node_modules/@mui/material";

Second, in the tsconfig.json file, add a new entry to compilerOptions.paths which looks like this:

{
    "compilerOptions": {
        "paths": {
            "@mui/material": [
                "./src/material"
            ]
        }
    }
}

With this, when @mui/material is imported from any files of your project, it will instead import it from the file you created, which specifies that anything from here is a Client Component.

@mwskwong
Copy link
Contributor

mwskwong commented Jun 29, 2023

As a temporary workaround, I have setup something that utilizes TypeScript path aliases to provide the behaviour we want.

First, create a file (I've called it material.ts) with the following content:

"use client";

// "node_modules" is required here to make sure we don't go into a recursive loop with the path alias.
export * from "node_modules/@mui/material";

Second, in the tsconfig.json file, add a new entry to compilerOptions.paths which looks like this:

{
    "compilerOptions": {
        "paths": {
            "@mui/material": [
                "./src/material"
            ]
        }
    }
}

With this, when @mui/material is imported from any files of your project, it will instead import it from the file you created, which specifies that anything from here is a Client Component.

That's a good idea, and it does work. Unfortunately, the tradeoff is it breaks tree shaking, so the entire @mui/material is bundled

@coder-xiaotian
Copy link

This SWC plugin can match which components to insert the 'use client' directive based on the incoming path.
https://github.com/coder-xiaotian/swc-useclient

@oliviertassinari
Copy link
Member

I think we could do the same in MUI X, issue created mui/mui-x#9833.

@oliviertassinari oliviertassinari added enhancement This is not a bug, nor a new feature and removed core Infrastructure work going on behind the scenes labels Jul 29, 2023
@oliviertassinari
Copy link
Member

oliviertassinari commented Aug 20, 2023

Another thought, it feels that it would be great to move our Next.js documentation from Pages Router to App Router, so that we are forced to add 'use client'; on all the demos of our docs that need them https://www.notion.so/mui-org/docs-infra-Migrate-docs-to-Next-App-Router-f95ecaead5c34d52bb7256446a83b014. I had cases today, where I copied the demo, and then had to add the mention, frustrating.

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
Projects
None yet
8 participants