-
Notifications
You must be signed in to change notification settings - Fork 26.8k
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
SWC emotion transform plugin #34687
SWC emotion transform plugin #34687
Conversation
packages/next-swc/crates/core/src/emotion/global_parent_cache.rs
Outdated
Show resolved
Hide resolved
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
3bfdf51
to
075606b
Compare
This comment has been minimized.
This comment has been minimized.
075606b
to
dd35632
Compare
dd35632
to
0c919fb
Compare
This comment has been minimized.
This comment has been minimized.
0c919fb
to
68aa625
Compare
This comment has been minimized.
This comment has been minimized.
68aa625
to
83acb30
Compare
9144711
to
dd44f4a
Compare
This comment has been minimized.
This comment has been minimized.
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.
Could you add telemetry tracking of this feature? I'll work on adding better documentation, but these two PRs should help:
7957c6e
to
5d5f254
Compare
Stats from current PRDefault Build (Decrease detected ✓)General Overall increase
|
vercel/next.js canary | vercel/next.js swc-emotion-plugin | Change | |
---|---|---|---|
buildDuration | 18.2s | 18.7s | |
buildDurationCached | 7.3s | 7.4s | |
nodeModulesSize | 456 MB | 456 MB |
Page Load Tests Overall decrease ⚠️
vercel/next.js canary | vercel/next.js swc-emotion-plugin | Change | |
---|---|---|---|
/ failed reqs | 0 | 0 | ✓ |
/ total time (seconds) | 3.612 | 3.743 | |
/ avg req/sec | 692.05 | 667.85 | |
/error-in-render failed reqs | 0 | 0 | ✓ |
/error-in-render total time (seconds) | 1.566 | 1.637 | |
/error-in-render avg req/sec | 1596.65 | 1527.53 |
Client Bundles (main, webpack)
vercel/next.js canary | vercel/next.js swc-emotion-plugin | Change | |
---|---|---|---|
925.HASH.js gzip | 179 B | 179 B | ✓ |
framework-HASH.js gzip | 42 kB | 42 kB | ✓ |
main-HASH.js gzip | 27.9 kB | 27.9 kB | ✓ |
webpack-HASH.js gzip | 1.44 kB | 1.44 kB | ✓ |
Overall change | 71.6 kB | 71.6 kB | ✓ |
Legacy Client Bundles (polyfills)
vercel/next.js canary | vercel/next.js swc-emotion-plugin | Change | |
---|---|---|---|
polyfills-HASH.js gzip | 31 kB | 31 kB | ✓ |
Overall change | 31 kB | 31 kB | ✓ |
Client Pages
vercel/next.js canary | vercel/next.js swc-emotion-plugin | Change | |
---|---|---|---|
_app-HASH.js gzip | 1.36 kB | 1.36 kB | ✓ |
_error-HASH.js gzip | 192 B | 192 B | ✓ |
amp-HASH.js gzip | 309 B | 309 B | ✓ |
css-HASH.js gzip | 327 B | 327 B | ✓ |
dynamic-HASH.js gzip | 2.57 kB | 2.57 kB | ✓ |
head-HASH.js gzip | 351 B | 351 B | ✓ |
hooks-HASH.js gzip | 920 B | 920 B | ✓ |
image-HASH.js gzip | 5.09 kB | 5.09 kB | ✓ |
index-HASH.js gzip | 263 B | 263 B | ✓ |
link-HASH.js gzip | 2.26 kB | 2.26 kB | ✓ |
routerDirect..HASH.js gzip | 320 B | 320 B | ✓ |
script-HASH.js gzip | 387 B | 387 B | ✓ |
withRouter-HASH.js gzip | 319 B | 319 B | ✓ |
85e02e95b279..7e3.css gzip | 107 B | 107 B | ✓ |
Overall change | 14.8 kB | 14.8 kB | ✓ |
Client Build Manifests
vercel/next.js canary | vercel/next.js swc-emotion-plugin | Change | |
---|---|---|---|
_buildManifest.js gzip | 460 B | 460 B | ✓ |
Overall change | 460 B | 460 B | ✓ |
Rendered Page Sizes
vercel/next.js canary | vercel/next.js swc-emotion-plugin | Change | |
---|---|---|---|
index.html gzip | 531 B | 531 B | ✓ |
link.html gzip | 545 B | 545 B | ✓ |
withRouter.html gzip | 526 B | 526 B | ✓ |
Overall change | 1.6 kB | 1.6 kB | ✓ |
Default Build with SWC (Decrease detected ✓)
General Overall increase ⚠️
vercel/next.js canary | vercel/next.js swc-emotion-plugin | Change | |
---|---|---|---|
buildDuration | 21.9s | 22.9s | |
buildDurationCached | 7.7s | 7.3s | -389ms |
nodeModulesSize | 456 MB | 456 MB |
Page Load Tests Overall decrease ⚠️
vercel/next.js canary | vercel/next.js swc-emotion-plugin | Change | |
---|---|---|---|
/ failed reqs | 0 | 0 | ✓ |
/ total time (seconds) | 3.707 | 3.664 | -0.04 |
/ avg req/sec | 674.45 | 682.37 | +7.92 |
/error-in-render failed reqs | 0 | 0 | ✓ |
/error-in-render total time (seconds) | 1.531 | 1.668 | |
/error-in-render avg req/sec | 1633.1 | 1499.04 |
Client Bundles (main, webpack)
vercel/next.js canary | vercel/next.js swc-emotion-plugin | Change | |
---|---|---|---|
925.HASH.js gzip | 178 B | 178 B | ✓ |
framework-HASH.js gzip | 42.3 kB | 42.3 kB | ✓ |
main-HASH.js gzip | 28.2 kB | 28.2 kB | ✓ |
webpack-HASH.js gzip | 1.45 kB | 1.45 kB | ✓ |
Overall change | 72.1 kB | 72.1 kB | ✓ |
Legacy Client Bundles (polyfills)
vercel/next.js canary | vercel/next.js swc-emotion-plugin | Change | |
---|---|---|---|
polyfills-HASH.js gzip | 31 kB | 31 kB | ✓ |
Overall change | 31 kB | 31 kB | ✓ |
Client Pages
vercel/next.js canary | vercel/next.js swc-emotion-plugin | Change | |
---|---|---|---|
_app-HASH.js gzip | 1.35 kB | 1.35 kB | ✓ |
_error-HASH.js gzip | 179 B | 179 B | ✓ |
amp-HASH.js gzip | 313 B | 313 B | ✓ |
css-HASH.js gzip | 324 B | 324 B | ✓ |
dynamic-HASH.js gzip | 2.56 kB | 2.56 kB | ✓ |
head-HASH.js gzip | 351 B | 351 B | ✓ |
hooks-HASH.js gzip | 921 B | 921 B | ✓ |
image-HASH.js gzip | 5.23 kB | 5.23 kB | ✓ |
index-HASH.js gzip | 261 B | 261 B | ✓ |
link-HASH.js gzip | 2.33 kB | 2.33 kB | ✓ |
routerDirect..HASH.js gzip | 322 B | 322 B | ✓ |
script-HASH.js gzip | 388 B | 388 B | ✓ |
withRouter-HASH.js gzip | 317 B | 317 B | ✓ |
85e02e95b279..7e3.css gzip | 107 B | 107 B | ✓ |
Overall change | 15 kB | 15 kB | ✓ |
Client Build Manifests
vercel/next.js canary | vercel/next.js swc-emotion-plugin | Change | |
---|---|---|---|
_buildManifest.js gzip | 458 B | 458 B | ✓ |
Overall change | 458 B | 458 B | ✓ |
Rendered Page Sizes
vercel/next.js canary | vercel/next.js swc-emotion-plugin | Change | |
---|---|---|---|
index.html gzip | 533 B | 533 B | ✓ |
link.html gzip | 547 B | 547 B | ✓ |
withRouter.html gzip | 527 B | 527 B | ✓ |
Overall change | 1.61 kB | 1.61 kB | ✓ |
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.
As there was no rust code change after my latest approval, I think it's fine
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.
Only feedback I have is that this is missing an integration test to check for the hydration issue the transform solves similar to styled-components. You can have a look at test/development/basic/styled-components
for inspiration.
I believe the SSR feature of |
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 then this should be fine 👍
Wow, this is great, thanks @Brooooooklyn (and @kdy1 @timneutkens) !! 🙌 Super looking forward to using this! I guess this will be in Also, will there be a migration guide? Edit: working migration guide below 👇 Migration guide from
|
Great thanks @Brooooooklyn , I've updated the migration guide above, I guess the guide above is complete now...? I'll give it a shot. |
Yes, you can try it with |
Ah, I mean my 3-step migration guide above is complete, which I'm not 100% sure about. But I'll try it out, thanks! |
Hi, thank you so much for putting this together. I have identified something which I think should be supported but doesn't seem to be (as it's supported by the babel plugin). When you wrap an existing component with
Here's a minimal example page: import styled from "@emotion/styled";
const TextWrapper = styled.span`
color: red;
`;
const AltTextWrapper = styled(TextWrapper)`
color: orange;
`;
const Container = styled.div`
background: blue;
/* fine */
${TextWrapper} {
color: green;
}
/* problem */
${AltTextWrapper} {
color: green;
}
`;
export default function Index() {
return (
<Container>
<TextWrapper>Test</TextWrapper>
<AltTextWrapper>Test</AltTextWrapper>
</Container>
);
} Note that the error is still present when wrapping a non-styled component e.g. function SomeComponent({ className }) {
return <div className={className}>SomeComponent</div>;
}
const WrappedSomeComponent = styled(SomeComponent)`
color: yellow;
`;
...
const Container = styled.div`
/* still a problem */
${WrappedSomeComponent} {
color: green;
}
`; Since this is closed, if you'd like, I can file a separate issue to track this. |
@tills13 I think this is reported in the issue: #30804 (comment) |
@karlhorky yep. That's the same issue. Thanks. |
My build times have gotten worse since enabling this. Am I missing something? I'm running
in my Before:
After:
System information: Windows 10 + WSL |
@tills13 This happened to me as well and surprised me a bit. Build times got worse when compiling Emotion using SWC. Adding |
With |
Did you remove |
haha, yes. I was sure to remove the |
@tills13 are those numbers you shared all with an empty cache? Would you mind sharing the |
CleanShot.2022-03-09.at.15.04.34.mp4
Close #30804