-
Notifications
You must be signed in to change notification settings - Fork 292
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
errors when using external ui library implemented with vanilla-extract in next.js app #940
Comments
Hi, thanks for the reproduction. Your app looks fine to me. I think the issue might be how your UI library is bundled. You'll need to use one of the bundler integrations to create a package that can be consumed correctly in a project that uses vanilla-extract. Take a look at the webpack or the rollup integrations for some ideas of how to get that working. Alternatively, a discord user recently shared an example repo that uses rollup to bundle a UI package that uses vanilla-extract for styling. https://github.com/graup/vanilla-extract-rollup-example/tree/main/packages/ui. Feel free to join the discord for some quicker feedback from community members if you need some help getting things working. |
Thank you for the response, Adam. Per your suggestion, I've joined the Discord and have asked about this issue. Still awaiting a response there. My assumption with bundling on the library side is that it would result in outputting To help with picking apart the example, I've pushed the example library to a repo, included its dist in the source, and have updated the library link to point to the repo instead of the npmjs.com page. I'd love to figure out how to get this working. Hopefully, it's just something I'm overlooking in my implementation. |
I'm still stumped by this. I've been trying to sort it out without diving deep into unfamiliar internals of VE, hoping it's something obviously wrong that I'm doing. Based on prior conversations, I'm curious about whether or not VE expects The fact that it works some of the time ought to be a clue. It suggests that this approach should work, but is missing something necessary for VE to handle it properly in all cases. 😕 |
I stumbled onto #620. After reading through it and experimenting with the example and its approach, it seems like the only way to use VE in the manner tried in this issue is to eliminate all code transformation on the library prior to publishing, releasing the original source—e.g. For a small project, this might be feasible. But for larger project with several intermediate (and very legacy) libraries, this means that every library adopting VE would need to publish its original source for this approach to work. That is, of course, assuming that this conclusion is accurate. As an additional quirk, when transpiling the module on the Next.js side but converting A. The external code is not used on the page.
B. The external code is added to the page.
|
Hi @jneander. Sorry for the delayed response. In my original comment I thought you were trying to achieve something different, but given the context you've provided I think I now understand the problem better. As far as I know, there's no reason your approach shouldn't work. Vanilla extract integrations should find files ending in In fact, my team at SEEK is working on a packaging tool that works by this principle, as our design system has gotten large enough that it has warranted investment in speeding up build times. Currently we just ship raw typescript and put the onus on the consumer to transpile it in their app, which as you've mentioned, doesn't scale very well. The error you mentioned in the top-level issue comment is coming from here, which is called slightly further down the call stack from the Debugging your app, I noticed that the VE webpack plugin was transforming the esm version of Hope this helps. |
Hey @askoufis. No worries about the delay. Apologies for not being more clear about what I was trying to accomplish in the original post. Your mention of the cjs/esm mismatch was a solid clue. I just tried to switch to using the Lo and behold, it worked! This warrants further debugging, as there seems to be something allowing the mismatch to happen. If the easy fix of dropping cjs holds up, then I might have a path forward for now. But for folks needing both esm and cjs for legacy purposes, this little gremlin might be a showstopper. I'll dig deeper and see what I come up with. It's as good a time as any to learn more about the VE internals. |
I came across this issue after having the same problem. First some background... I've been using Jest + Rush for a TS -> ESM-module monorepo (multiple packages). Each package has `"type": "module", with an export prop in package.json. To get Jest to correctly load (without errors) a local-package that used VE, I needed this config in my NextJS app (jest.config.cjs) For Jest
After attempting to switch to Vitest, I did the usual find-and-replace jest with vi, and got most of the tests working. However, VE started to complain again with the error in this issue. Armed with a working Jest config, I was finally able to come up with this solution (vitest.config.ts): For Vitest:
Note that the child-dependency is only compiled (not bundled) using plain ol' TypeScript. The VE docs assume that you would use ESbuild or Rollup, which isn't always the case (especially in monorepos with library packages). Or should I be bundling internal-library packages? |
Any new findings on this @jneander ? |
Hello ! So if I understand correctly, we can't use properly Because, as I see in my application, I have to import the I really want to use Vite.JS (ESM first). |
If you start a thread in discord, I can help you out. I built a UI library with VE and am using it at work. |
You can use it. They have a plug-in for integrating with vite. Message us on discord if you get stuck https://vanilla-extract.style/documentation/integrations/vite/ |
@viclafouch we had some challenges with VE UI Library to integrate it with Vite. With some tweaking in the Vite config, we succeed to make it work. The hack was to exclude the UI library packages from the "optimizeDeps". This is how our vite.config.ts looks now: import { vanillaExtractPlugin } from "@vanilla-extract/vite-plugin";
import react from "@vitejs/plugin-react-swc";
import { defineConfig } from "vite";
export default defineConfig({
plugins: [react(), vanillaExtractPlugin()],
optimizeDeps: {
exclude: [
"@filamix-ui",
"@filamix-ui/generics",
],
},
}); Worth mentioning is that from "@filamix-ui" we are exporting subpaths thru package.json. It works fine. Just make sure in your tsconfig.json to set "compilerOptions" > "moduleResolution" to "Node16". |
Are you using it with Next.js? |
I’m not. I can spin up a couple test apps with next 12 and next 13 to see if they work and let you know. |
I think I've been running into the same thing (with just the Webpack plugin). For me, it boiled down to the fact that my component library is published in CJS, and I don't believe that the Vanilla Extract tooling supports CommonJS. I think I have a fix in #970. In the meantime, I found that the workaround is to include the Object.defineProperty(exports, "__esModule", {
value: true
});
var __vanilla_filescope__ = _interopRequireWildcard(require("@vanilla-extract/css/fileScope"));
var _css = require("@vanilla-extract/css");
__vanilla_filescope__.setFileScope("src/Button/Button.css.ts", "my-library");
var button = _css.style({});
exports.button = button;
__vanilla_filescope__.endFileScope(); Then when the app consumes, this file, I don't think this is the ideal solution by any means, but it's a decent work around, and I think this can be handled upstream (maybe my PR is the right solution, maybe not). |
Both the workaround and fix don't work on my next js setup. This is a huge blocker for us :/ Update: After updating the packages and moving some things around, I've managed to get it to work... After some digging around, I found this issue: I believe this error might be on Next.js' end, but maybe there's a way to work around it on the webpack plugin? |
It looks like this is more of a feature request than a bug. Shipping styles without compilation isn't really how Vanilla Extract is meant to be used. As a UI library author, I don't see the benefit in forcing every user to compile the VE styles themselves from node_modules. My component library is working without issues in Next.js v13.1.2 app directory. For reference, here's the library source code and usage in a Next.js app. For anyone getting blocked by this, the solution might just be to add the respective bundler plugin to your library so you're shipping static styles, not TypeScript. And if there are any significant benefits to shipping TypeScript and making users compile VE styles, I'd love to see a docs page on this. |
I disagree. I don't see why functionality should differ because the styles come from node_modules? It's not that complex.
That is, of course, what we are already doing. Setting up Vanilla Extract is a requirement for using our styles.
We're decoupling every single component style. Each component style is a package that can be built individually. We have a pretty big UI Toolkit. This means that when we update a small style, only that specific package needs to be updated and rebuilt, which would not be the case if we pre-built our vanilla extract files into CSS, as that would potentially mess up the variable scoping. |
It gets complex in the context of Next.js because it doesn't transpile node_modules out of the box. And I'm not sure relying on Next.js to transpile your library is the optimal solution because there's no documentation on how that transpilation works and what's the config used. I've always seen next-transpile-modules config being used as a silver bullet to fix broken packages, it's not the happy path most devs expect with libs. next-transpile-modules has been historically used for 2 use-cases:
A published library is very different from the first use-case. And Next.js supports ESM libraries now so the second use case for a published package is redundant now.
Is that not possible with VE currently? Each component gets its own CSS file after transpilation so we can update styles of one component without affecting others. Variable scoping shouldn't be a problem as all bundlers aim for deterministic builds. So if you run a build twice on the same code, it shouldn't mean that you get different class names each time. I'd assume VE's hashing algorithm is deterministic too, it's a bug if it isn't. Can you please elaborate a bit more? I'm really not seeing the benefit in moving transpilation to consumer projects. All it'll achieve is make installation of your UI toolkit harder for users. |
As I stated in a previous edit, I think this is a problem on Next.js' part. Just by looking at the classNames generated, I can see that the server is serving from the cjs module while the client is importing from the esm module. There is a stale issue about this on their github (vercel/next.js#35581). The workaround is deleting the "module" key from our package.json.
Sure! It indeed is possible, however as I said, you may end up running into scoping issues. We extensively use contracts for our values, and there is one main theme which gets consistently referenced by all our components. If at some point, someone decides to make a small change to the main theme, it means that only the theme package needs to be updated. This would not be the case with pre-built CSS, since building the theme package would generate a new hash for the variable scope and thus all the CSS that references that would have to be rebuilt, even if they weren't changed at all. Since all of our contracts also come from the same place, it would mean that adding a new component would have to trigger a new version for all the components as well, which pretty much just invalidates the reasoning for having separate component versions. |
I think I found a definitive fix. Make this change to your next.config.js: /** @type {import('next').NextConfig} */
const nextConfig = {
reactStrictMode: true,
webpack(config, options) {
config.module.rules.unshift({
test: /@your-theme-scope/, // Can also be your individual package name
resolve: {
mainFields: ['module'],
},
});
return config;
},
}; This will make it so that the imports are consistent between client and server. You also need to write an exports field in your package as well as your main and module fields (which I thought was pretty weird but it consistently fixed the issue): {
"main": "./dist/cjs/index.css.js",
"module": "./dist/esm/index.css.js",
"types": "./dist/types/index.css.d.ts",
"exports": {
".": {
"import": "./dist/esm/index.css.js",
"require": "./dist/cjs/index.css.js",
"types": "./dist/types/index.css.d.ts"
}
},
} Finding this was a result of a very frustrating trial an error process, but I'm 100% sure that this is a problem on Next.js' fault. If the original poster could try this I think we could also close the issue :) EDIT: After we did some internal investigation, this solution was weirdly only working inside of our monorepo, not in packages consuming our toolkit. The definite absolutely final solution was actually to add our VE-dependant packages to the |
I appreciate all of the ideas and collaboration here. ❤️ I'll make my way back to this eventually. Work has been eating me alive and I don't have anything left in the tank right now for tackling this issue. |
Jest config that appears to work in this scenario: NextJS + TypeScript + Jest + VE + module-dependency that includes React components that use VE (library of React+VE components) jest.config.js
This works without the VE-babel plugin. The key step is getting the VE-Jest plugin to appear FIRST in the config.transform object. And the VE-jest plugin MUST transform both |
@otaviomad I can confirm that transpiling the vanilla-extract UI packages solved the issue. |
Can we please discourage this habit? I'd hope that you didn't come to any groundbreaking discoveries in discord, because now it's not searchable from google/bing/duckduckgo/etc. We can simply hold the conversation here in issue tickets, or the actual discussions tab ☝🏻 resulting in publicly accessible sharing of knowledge. Even if I did join discord to try an benefit from any outcome there, it'd be near impossible to find that conversation. |
Letting people know that there's a community of us that are actively on discord helping people with their issues is not something I will refrain from doing. I understand your concern and it is valid, however, people tend to get help faster when they post in discord. And there is a search feature in the app so you can keyword search for anything you want. Also, if you scroll up to the top, Adam, also suggested reaching out on discord before I did.
People ask us for help all the time and often get help right away. It's an incredible resource to be able to get help nearly 24/7 and talk with people about what they need, how they're using VE, and providing more instant feedback. Our help shouldn't be limited to what can be identified by a search engine. |
what a life-saver! this works with Vite. I'm also using subpath exports with my lib(built with VE transpiled to I'm not sure Vite API supports overwriting vanillaExtractPlugin({ packages: ['my-library', 'my-friend-library'] }) Update: |
Describe the bug
In a Next.js app using the @vanilla-extract/next-plugin to support VE locally, everything works as expected. However, when trying to use view code from an external library that has styles implemented with vanilla-extract, there are render errors.
The minimal example for can be found here:
The following behavior is observed while the Next.js app is running in dev mode:
A. The external code is not used on the page.
B. The external code is added to the page.
Reproduction
https://github.com/jneander/example-app-using-third-party-vanilla-extract-ui-library
System Info
Used Package Manager
npm
Logs
Validations
The text was updated successfully, but these errors were encountered: