Skip to content

[internal] Remove 'use client' from index files#331

Merged
michaldudak merged 9 commits intomui:masterfrom
michaldudak:remove-use-client
Sep 18, 2024
Merged

[internal] Remove 'use client' from index files#331
michaldudak merged 9 commits intomui:masterfrom
michaldudak:remove-use-client

Conversation

@michaldudak
Copy link
Member

Closes #330.

@oliviertassinari
Copy link
Member

oliviertassinari commented Apr 19, 2024

As I understand things, it's possible to use Base UI components without having to add "use client" (as a user of Base UI). For example, https://deploy-preview-331--base-ui.netlify.app/base-ui/react-number-field/ imported by a server component seems to work just fine (@mui/system is heading to work, and React.useId works with server components).

Now, sure, developers would need to add events to make use of those Base UI, so need "use client" in their code. But this might only happen higher up, meaning when developers use the design systems built with Base UI. I suspect that adding "use client" in each component file could help with the DX, delaying the time when developers hit an error that requires them to add "use client".

@michaldudak
Copy link
Member Author

Sorry, I don't quite understand what you're proposing above.

I suspect that adding "use client" in each component file could help with the DX

We must add "use client" in our component files as we use APIs not supported by RSC (context, event handlers)

@oliviertassinari
Copy link
Member

oliviertassinari commented Apr 22, 2024

The point I was trying to make is that, for example:

Doesn't have "use client" but it is seems that it should. Removing

at the same time.

But I don't know, it might not be that important: vercel/next.js#64467 (comment) if it's fixed in Next.js latest canary. All in all, 👍 to add use client at the lowest level possible, even if it leads to duplication. It feels clearer, but I can't find strong arguments either way.

@atomiks
Copy link
Contributor

atomiks commented Apr 22, 2024

Is this only necessary when using the star (*) export?

In our new API, we export each component individually. Only the types use the star (might need export type *).

Error: It's currently unsupported to use "export *" in a client boundary. Please use named exports instead.

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged. label Apr 29, 2024
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged. label May 29, 2024
@mui-bot
Copy link

mui-bot commented May 29, 2024

Netlify deploy preview

https://deploy-preview-331--base-ui.netlify.app/

Generated by 🚫 dangerJS against 0a30801

@michaldudak
Copy link
Member Author

👍 to add use client at the lowest level possible, even if it leads to duplication. It feels clearer, but I can't find strong arguments either way.

I feel the same. Technically it's the component (or hook) that must be ran on the client.
I updated the PR.

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged. label Jun 11, 2024
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged. label Sep 4, 2024
@michaldudak
Copy link
Member Author

@atomiks, I'd appreciate a review

Copy link
Member

@oliviertassinari oliviertassinari left a comment

Choose a reason for hiding this comment

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

+1 it looks like this will help with Next.js integration and ESM 👍 mui/material-ui#42750 (comment)

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged. label Sep 5, 2024
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged. label Sep 5, 2024
@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged. label Sep 17, 2024
Copy link
Contributor

@atomiks atomiks left a comment

Choose a reason for hiding this comment

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

Seems fine, but I suppose there shouldn't be a gap between 'use client' as Olivier mentioned

@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged. label Sep 18, 2024
@michaldudak michaldudak merged commit a8faac3 into mui:master Sep 18, 2024
@michaldudak michaldudak deleted the remove-use-client branch September 18, 2024 09:01
@oliviertassinari
Copy link
Member

oliviertassinari commented Sep 18, 2024

Cool cleanup 👌

but I suppose there shouldn't be a gap between 'use client' as Olivier mentioned

It's subjective. The rational was to do it a single way through the codebase of MUI. I saw the majority was without blank lines so pushed to default with the majority. I think Albert initially introduced the no blank line path.

Now, if you look at the React docs, it's different https://react.dev/reference/rsc/use-client.

For use strict, there is no consistency in https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Strict_mode. It's also a mix in https://github.com/search?q=org%3Aradix-ui+%27use+client%27&type=code.

No real preference, as long as it's consistent.

@michaldudak
Copy link
Member Author

I personally prefer more whitespace, but it's not a dealbreaker.

@oliviertassinari oliviertassinari added internal Behind-the-scenes enhancement. Formerly called “core”. and removed core labels Aug 2, 2025
@oliviertassinari oliviertassinari changed the title [core] Remove 'use client' from index files [internal] Remove 'use client' from index files Feb 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

internal Behind-the-scenes enhancement. Formerly called “core”.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[internal] Remove 'use client' from index files

4 participants