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] Remove 'use client' from index files #331

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

michaldudak
Copy link
Member

Closes #330.

@michaldudak michaldudak added the core Infrastructure work going on behind the scenes label Apr 19, 2024
@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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Infrastructure work going on behind the scenes PR: out-of-date The pull request has merge conflicts and can't be merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[core] Remove 'use client' from index files
3 participants