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

Add use client directive to all components in a new build step #9833

Open
oliviertassinari opened this issue Jul 29, 2023 · 13 comments
Open
Assignees
Labels
new feature New feature or request nextjs umbrella For grouping multiple issues to provide a holistic view

Comments

@oliviertassinari
Copy link
Member

oliviertassinari commented Jul 29, 2023

Summary 💡

Same as mui/material-ui#37503

Examples 🌈

No response

Motivation 🔦

To feel the pain:

  1. Create a new project with the example linked on https://mui.com/material-ui/guides/next-js-app-router/
  2. Copy and paste this example https://mui.com/x/react-charts/lines/#data-format
  3. ❌ Crash
Screenshot 2023-07-29 at 23 27 36
  1. ✅ Add "use client" in your file

Saving step 4. would be great.

Context

This is part of improving the integration with Next.js App Router. I have also noticed bugs with it, but these are beyond our control:

@jsleemaster
Copy link

I'm having the same problem 😭

@oliviertassinari oliviertassinari added new feature New feature or request and removed enhancement This is not a bug, nor a new feature labels Aug 11, 2023
@xeofd
Copy link

xeofd commented Sep 8, 2023

Bump, was just about to raise this 👍

@oliviertassinari
Copy link
Member Author

oliviertassinari commented Dec 29, 2023

The components in the lab are getting covered mui/material-ui#40358.

Does someone has a practical use case for this issue? I guess with rich components like MUI X has its rare for the component that renders it to not already have "use client"? So maybe the value is on the ease of getting started?

@oliviertassinari
Copy link
Member Author

Oh actually, I just hit the case playing with the component, with the code copied from the docs:

Screenshot 2024-01-03 at 00 22 45

Not a major annoyance.

@joserodolfofreitas joserodolfofreitas added the umbrella For grouping multiple issues to provide a holistic view label Jan 15, 2024
@LukasTy
Copy link
Member

LukasTy commented Jan 15, 2024

Clarifying that MUI Core uses https://github.com/mui/material-ui/blob/master/packages/rsc-builder/buildRsc.ts to automate it.

@oliviertassinari
Copy link
Member Author

oliviertassinari commented Jan 15, 2024

Which is a script that I think we can make much better:

  • make the source of the script handle configurable
  • ignore files that don't have React client-side APIs
  • allow, with JavaScript comments, to support exceptions to the logic

@cherniavskii
Copy link
Member

It seems like for the Data Grid packages, we can add 'use client'; to the index files since everything is imported from there: https://github.com/mui/mui-x/blob/next/packages/grid/x-data-grid/src/index.ts

@oliviertassinari
Copy link
Member Author

The closer to the use of client APIs, the better though. If people want to use some of the API on the server, a use client in all the index would prevent them to.

Maybe one in https://github.com/mui/mui-x/blob/next/packages/grid/x-data-grid/src/DataGrid/DataGrid.tsx would cover 80% of the problem.

@cherniavskii
Copy link
Member

@oliviertassinari Overall, I agree.
However, I can't find a single component exported from the data grid packages that could be used as server components - all of them use hooks.

@oliviertassinari
Copy link
Member Author

oliviertassinari commented Jan 18, 2024

However, I can't find a single component exported from the data grid packages that could be used as server components - all of them use hooks.

@cherniavskii I wonder about the work we are doing to better support server-side integration though. We experienced cases like this in the core: mui/material-ui#42750.

@MBilalShafi
Copy link
Member

MBilalShafi commented Jan 18, 2024

Interesting point, though as of now I can not foresee any server-rendered component for the server-side integration as the initial goal is data fetching on the client, I'm not sure if that'll be the case later on too. Let's start with adding "use client" in DataGrid.tsx

@oliviertassinari
Copy link
Member Author

oliviertassinari commented Feb 22, 2024

Oh, now I might understand why nobody asked for using a data grid from a server component, but asked for the charts.

It might be because of https://nextjs.org/docs/app/building-your-application/rendering/composition-patterns#passing-props-from-server-to-client-components-serialization, the columns definitions of the data grid, and as many other props aren't serializable, so people need to add "use client" where they render them <DataGrid>.

So the benefits of adding "use client" on MUI X Data Grid is very limited.

For example, I don't understand why they do this https://www.telerik.com/kendo-react-ui/components/server-components/, for charts yes mui/material-ui#12180, but for a data grid I don't see the point.

@carl0s-sa
Copy link

Hey! Are there any plans of adding "use client" for LocalizationProvider and other components so we can use this in next 14? I'm using mui/x-date-pickers-pro and it's not letting me place the provider in the layout without exporting it from an "in-between" client file.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new feature New feature or request nextjs umbrella For grouping multiple issues to provide a holistic view
Projects
None yet
Development

No branches or pull requests

9 participants