Open otaviomad opened 4 months ago
I'm a bit confused about the line you're talking about in the compiler. The caching is there to avoid re-evaluating the same module. It shouldn't affect how classes are registered in the adaptor when importing other modules.
I think a reproduction would help demonstrate the issue.
hi, I am part of Otavio's team and I think it will really hard to reproduce the issue, because we have complex NX monorepo setup, but I will try to provide you with some screenshots in hope to explain the issue better:
// icon.css.ts
exported from @mixer/atomic-styles
// stepper.css.ts
// style compiled in the browser
you can see that the icon
classname is missing dot(.) in the composed selector, so the styling is not applied
hi, I am part of Otavio's team and I think it will really hard to reproduce the issue, because we have complex NX monorepo setup, but I will try to provide you with some screenshots in hope to explain the issue better: [IMAGES] you can see that the
icon
classname is missing dot(.) in the composed selector, so the styling is not applied
Perhaps relevant to add that we're having this issue in Storybook specifically. I'm trying to repro it on a vanilla vite react app.
@askoufis Sorry to ping you like this but I just managed to get a repro:
https://github.com/otaviomad/ve-bug-repro
I was unable to get the same results outside storybook, but here you can clearly see in the stepper component that it built the classnames improperly:
(inspect and look for stepper.css.js)
Curiously, if the "*-styles" packages have index.ts instead of index.css.ts (and, subsequently transpiled index.css.js) files, the styling still works. It seems like this sort of error only happens when the exported styles are "two levels deep" within .css.ts modules.
If the repro misbehaves or isn't working for you, please let me know.
It's also relevant to note that this behaviour didn't happen on the previous compiler version.
Thanks for the reproduction @otaviomad!
Strange that it only happens in Storybook. I haven't managed to figure out the cause just yet, but converting your index.css.ts
files to just index.ts
files (and updating the rollup configs and package.json files accordingly) seems to fix the issue.
I though this might just be because it was reducing the depth of the .css.ts
imports, but I tried nesting a few more imports across files and it seems fine. Based on that, my guess would be that the issue is caused by the transformation(s) VE does on the index.css.ts
files.
I've pushed a fork of your reproduction with my fix: https://github.com/askoufis/ve-bug-repro/tree/rename-entry-files.
I don't think there's any reason to have .css.ts
on the end of your index
files. They're just re-exporting classnames from other .css.ts
files, so they themselves don't need to be handled by VE at all.
That being said, I can't think of a good reason why this shouldn't work. I'll try dig into the root cause when I have some more time.
@askoufis Thanks for the swift reply :)
I share your assessment. Changing from index.css.ts to index.ts probably won't make a difference, but I will try today on our main repo to see if anything breaks. We have a pretty large toolkit with visual regression testing, so we should pick up on any changes.
I also share your assessment on the fact that yeah, this should work. I will try to isolate the issue later as well, and I can open a PR if I find a solution.
@askoufis I tried it out on my repo and I managed to find something different. I'm unsure if this should be discussed on a separate issue (I'd be happy to open one if so).
The repro is here: https://github.com/otaviomad/ve-bug-repro/tree/other-bug
I changed "base" to extend another classname:
This base classname is used to create the button style:
and, in another package, to create the stepper container style:
However, something weird happens when you use both of these classes in a selector, like so:
This selector compiles wrongfully to:
Notice how there's the .base class there sandwiched between the two desired classnames. What's interesting about this is that if the base classname does not extend typographyBase, it works. It seems like the requirements for this bug are specifically somewhat like this:
It's also noticeable how this wasn't an issue in the previous compiler. Anyways, that's it from my side, I'll keep investigating to see why this is happening.
EDIT: Opened a separate issue https://github.com/vanilla-extract-css/vanilla-extract/issues/1430
Describe the bug
We've been using Vanilla Extract to build our styles on our Design System for a few versions now. We have, admittedly, a very specific setup:
Not all of those details are relevant to this problem, however I think the main one is having packages depending on packages. Pre-4.0 compiler worked nicely with this approach, but once we updated to 4.0, we started having issues. Specifically, when the dependency css is required more than once, the class name registration is skipped. What would happen would be something like this:
After some investigation, I believe I may have found the cause of the issue. I am somewhat familiar with Vanilla Extract's internals (having done some investigation on other issues in the past) so I think the cause may be here:
https://github.com/vanilla-extract-css/vanilla-extract/blob/master/packages/integration/src/compiler.ts
On line 244, we check if there is a cached built file. If there is, we return the pre-built file (which is only classnames and the css side-effect import). If there isn't then we start building the adapter and then we build it, registering the classNames and composition. What I think may help (heavy emphasis on think, I'm not a maintainer), is that the cached file should keep in memory all the compositions and classname registry, and when it is accessed, the adapter should also receive those informations that are cached.
I still haven't had time to build a proper repro of this, so if there is an absolute necessity to do so I will invest the time, but for now I hope this explanation will suffice.
Reproduction
https://github.com/otaviomad/ve-bug-repro
System Info
Used Package Manager
yarn
Logs
No response
Validations