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

[code-infra] Add "use client" directive to files with React APIs #45036

Merged
merged 5 commits into from
Jan 21, 2025

Conversation

Janpot
Copy link
Member

@Janpot Janpot commented Jan 16, 2025

This otherwise breaks react server components in Next.js when moving to full ESM.

copilot wrote me a eslint plugin that detects those and autofixes. we can't add them to index files

For reference: vercel/next.js#75128

@Janpot Janpot added the scope: code-infra Specific to the core-infra product label Jan 16, 2025
@mui-bot
Copy link

mui-bot commented Jan 16, 2025

Netlify deploy preview

https://deploy-preview-45036--material-ui.netlify.app/

Bundle size report

No bundle size changes (Toolpad)
No bundle size changes

Generated by 🚫 dangerJS against 30b6c8f

@Janpot Janpot marked this pull request as ready for review January 16, 2025 18:39
@Janpot Janpot requested a review from DiegoAndai January 16, 2025 19:05
Copy link
Member

@DiegoAndai DiegoAndai left a comment

Choose a reason for hiding this comment

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

Hey @Janpot,

What do you mean with:

we can't add them to index files

@@ -1,3 +1,4 @@
'use client';
Copy link
Member

Choose a reason for hiding this comment

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

Should we exclude test files?

@Janpot
Copy link
Member Author

Janpot commented Jan 16, 2025

Hey @Janpot,

What do you mean with:

we can't add them to index files

Didn't we uncover that in #41956?

@DiegoAndai
Copy link
Member

Didn't we uncover that in #41956?

I didn't know if you were referring to that or the eslint plugin.

The reason is to avoid 'use client' and export * in the same file, which causes the error described in #41956. Could we add another eslint plugin to cover this? Let me know if it's out of scope, and we can tackle it in a separate PR.

@Janpot
Copy link
Member Author

Janpot commented Jan 17, 2025

Could we add another eslint plugin to cover this?

🙂 It's already covered with the no-restricted-syntax. It seemed the use-case of this PR is a little too complex for this rule. I couldn't make it work.

material-ui/.eslintrc.js

Lines 254 to 258 in 820a76c

{
message:
"The 'use client' pragma can't be used with export * in the same module. This is not supported by Next.js.",
selector: 'ExpressionStatement[expression.value="use client"] ~ ExportAllDeclaration',
},

@Janpot Janpot merged commit 4f872d6 into mui:master Jan 21, 2025
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
scope: code-infra Specific to the core-infra product
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants