-
Notifications
You must be signed in to change notification settings - Fork 329
turbopack support #1639
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
base: master
Are you sure you want to change the base?
turbopack support #1639
Conversation
🦋 Changeset detectedLatest commit: eb2a5f9 The changes in this PR will be included in the next version bump. This PR includes changesets to release 11 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
@mattcompiles @askoufis Can anyone take a look at this? This is a pretty important feature for next.js users. Or should we consider the project dead? Last PR was merged in early August, basically no activity from maintainers since then. Not meaning to bash anyone, I know it's thankless work maintaining open source software, but it would be nice to know what we can expect from this project. Thanks in advance! |
A PR was merged in September. Reviews take time and ongoing work is often slow. All current maintainers have professional commitments and/or families. I try to look at PRs when I have spare time/motivation, which is often quite sporadic. I'm currently on holiday, but I intend to make some headway on open PRs and repo maintenance in when I get back in early November. |
|
Thanks for your reply! I was a bit too dramatic, sorry about that. Enjoy your holiday, and let me know if I can help with anything to get this PR going. |
|
I see there have been some recent commits pushed up to this PR. Are there any bits that you need help with that could be provided to speed up getting this merged in? Appreciate that it's been a holiday period, but a lot of apps have performance issues with Next which has various potential solutions but the main recommendation seems to boil down to "just use Turbopack" vercel/next.js#48748. Obviously, the lack of support for Turbopack with Vanilla is effectively blocking us from getting those gains, which would be greatly appreciated ahead of Black Friday. It would be great if there were a list of tasks that I or people within my company could help out with to speed this up, whether that's commits or just QAing. Cheers and thanks for the work on this so far! |
This reverts commit 568bd26.
|
If you're interested in trying this out, I've been using a You can install that temporary build with a link: Although, you probably want to pin to a specific commit instead of the branch for the sake of stability: Feedback or issues appreciated! I haven't seen many stats yet on speed or size of builds (compared to webpack) so if you have those I'd love to see some details. The biggest issue with this at the moment is I'm gonna do a quick cleanup pass this week to prep this for merge, it's basically ready at this point but is still a bit messy while I'm experimenting with some stuff. TLDR:
|
|
Hmmm, strange, I followed your steps and got this error That seemed to point at this specific line in the dist It's probably worth saying that we don't have a pages dir at all, we're fully on app dir. To be fair as well, I'm not sure that this would work for us anyway as we make heavy use of When you say it's basically ready, do you mean basically ready minus support for next modules due to dependency on things turbopack isn't yet providing to help you preprocess styles? If you have a solution to the above error, more than happy to give it a whirl |
|
The error looks like resolution error in the webpack plugin setup. Should've seen that coming. As far as next/image support, you're free to style next images as this doesn't route through the plugin: The specific scenario that doesn't work is importing an image into a This is unlikely to change until we get better tools from turbopack. For The specific scenario that doesn't work is extending font classNames in your styles: This is also unlikely to change until we get better tools from turbopack. TLDR using next modules is fine, but using next modules in your styles themselves is likely to fail unless I've specifically worked around it. |
For me is fine, thanks, I also had next font issues with webpack so I don't mind this approach, thank you very much for your work, I'll try this ASAP |
turbopack: improve deduplication
|
When is this is likely to be merged? more testing required? |
|
Try out the prerelease using overrides: It's built from this branch and is a merge of this PR with #1659 // package.json
"pnpm": {
"overrides": {
"@vanilla-extract/compiler": "https://pkg.pr.new/RJWadley/vanilla-extract/@vanilla-extract/compiler@c1d4a88",
"@vanilla-extract/css": "https://pkg.pr.new/RJWadley/vanilla-extract/@vanilla-extract/css@c1d4a88",
"@vanilla-extract/integration": "https://pkg.pr.new/RJWadley/vanilla-extract/@vanilla-extract/integration@c1d4a88",
"@vanilla-extract/next-plugin": "https://pkg.pr.new/RJWadley/vanilla-extract/@vanilla-extract/next-plugin@c1d4a88",
"@vanilla-extract/turbopack-plugin": "https://pkg.pr.new/RJWadley/vanilla-extract/@vanilla-extract/turbopack-plugin@c1d4a88",
"@vanilla-extract/webpack-plugin": "https://pkg.pr.new/RJWadley/vanilla-extract/@vanilla-extract/webpack-plugin@c1d4a88"
},
}If you have issues, do let me know! As far as merging, next step is a review from a maintainer. @askoufis let me know if there's anything I can do to make review easier! Happy to answer questions, revert bits, pull bits into their own PRs, etc. where possible for easier review. I know by nature it's a bit of a large PR so happy to do what I can to make it easier for you :) |
@RJWadley the Panda CSS library gets around this by telling users to use the Sorry in advance if this wouldn't work, I thought it was a potential way to unblock this particular issue. It'd need to be a breaking change for |
|
btw. Wanted to call out the fantastic work of this library. I love how it works |
|
@exequielbc ended up finding an approach that works for now. Imports from images imports now work as expected: import coolThing from "./my-cool-image.png"
export const background = style({
backgroundImage: `url(${coolThing.src})`, // works ✅
width: `${coolThing.width}px`, // works ✅
height: `${coolThing.height}px`, // works ✅
})Perhaps worth noting, this is just a stub for Try out the updated prerelease using overrides: // package.json
"pnpm": {
"overrides": {
"@vanilla-extract/compiler": "https://pkg.pr.new/RJWadley/vanilla-extract/@vanilla-extract/compiler@edaedbb",
"@vanilla-extract/css": "https://pkg.pr.new/RJWadley/vanilla-extract/@vanilla-extract/css@edaedbb",
"@vanilla-extract/integration": "https://pkg.pr.new/RJWadley/vanilla-extract/@vanilla-extract/integration@edaedbb",
"@vanilla-extract/next-plugin": "https://pkg.pr.new/RJWadley/vanilla-extract/@vanilla-extract/next-plugin@edaedbb",
"@vanilla-extract/turbopack-plugin": "https://pkg.pr.new/RJWadley/vanilla-extract/@vanilla-extract/turbopack-plugin@edaedbb",
"@vanilla-extract/webpack-plugin": "https://pkg.pr.new/RJWadley/vanilla-extract/@vanilla-extract/webpack-plugin@edaedbb"
},
} |
adds turbopack support to
@vanilla-extract/next-pluginturbopack doesn't have any sort of external API yet, so in order to evaluate the contents of our
*.css.tsfiles we need to compile it separately using@vanilla-extract/compiler. To avoid import resolution mismatches, we defer module resolution to turbopack via its webpack-compatible api.next/fontdoesn't work out of the box because turbopack has custom logic for it, so we transform any modules that use it in a css context to contain static references to the generated font family, style, and weight. most other next modules won't work incss.tsfilesfixes #1367