-
Notifications
You must be signed in to change notification settings - Fork 450
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
fix: handle circular import issue in a compiler friendly way #6812
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Ignored Deployment
|
No changes to documentation |
240b325
to
7ef7ac1
Compare
New and removed dependencies detected. Learn more about Socket for GitHub ↗︎
🚮 Removed packages: npm/@sanity/[email protected], npm/@sanity/[email protected], npm/@sanity/[email protected], npm/@sanity/[email protected], npm/@sanity/[email protected], npm/@sanity/[email protected], npm/@sanity/[email protected], npm/@tanstack/[email protected], npm/@tanstack/[email protected], npm/@testing-library/[email protected], npm/@testing-library/[email protected], npm/@testing-library/[email protected], npm/[email protected] |
Component Testing Report Updated May 30, 2024 2:29 PM (UTC)
|
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.
makes sense to me, thanks!
Description
By moving the import to colocate with the
StyledCard
definition we're able to avoid the circular deps issue in CJS, and regain the benefit of declaring thestyled(PreviewCard)
on the top-level instead of at runtime in that the CSS is known ahead of time forstyled-components
.What to review
It makes sense what it's named and its internal warning.
Testing
I tested it manually, but I'm not familiar with the original cjs bundle issue being referred to, can @bjoerge advice?
Notes for release
N/A as it's an internal refactor that allows React Compiler but is otherwise the same as before