-
Notifications
You must be signed in to change notification settings - Fork 1.3k
fix: use client in s2 icons #9153
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
Conversation
|
Build successful! 🎉 |
| const glob = require('glob'); | ||
|
|
||
| let iconFiles = glob.sync('packages/@react-spectrum/s2/{icons,illustrations}/**/*.{cjs,mjs}'); | ||
| for (let f of iconFiles) { | ||
| let contents = fs.readFileSync(f, 'utf8'); | ||
| contents = contents.replace(/['"]use client['"];?/g, ''); | ||
| fs.writeFileSync(f, contents); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wait so is it a problem if we remove the use client from the icons? #9126 (comment) seems to indicate that they should be client components but I'm not quite familiar with the nature of the nextJS build issue
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, so it's supported in our docs, which build from src. but it's not supported outside of that yet, so we just remove it, that's why it's removed from the other two files in this script as well, external users must import s2 components in a file that already has use client
don't worry, I asked the same questions when chatting with Devon about how to fix this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
gotcha, thanks for the explanation
Closes
✅ Pull Request Checklist:
📝 Test Instructions:
🧢 Your Project: