Multiple CSS files have the same name when using the Vite plugin with a scoped package

[AndrewLeedham]

Describe the bug

If the package name includes a scope e.g @vanilla-extract/example the outputted CSS files will all have the names example.vanilla.css, example.vanilla2.css, example.vanilla3.css etc. This seems to be because the sanitise file scope function adds the package name and when a scope is used it includes a / so it considers the last part as the file name. I would expect the file name to be included in the asset name, which it is when not using a scope.

Link to reproduction

https://stackblitz.com/edit/vitejs-vite-gfnuxw?file=vite.config.ts

• Run npm run build in the above and it outputs:

  vite v2.8.4 building for production...
  ✓ 9 modules transformed.
  dist/main.js                0.25 KiB / gzip: 0.20 KiB
  dist/main.css.js            0.18 KiB / gzip: 0.16 KiB
  dist/Button.js              0.13 KiB / gzip: 0.13 KiB
  dist/Heading.js             0.14 KiB / gzip: 0.14 KiB
  dist/Button.css.js          0.18 KiB / gzip: 0.16 KiB
  dist/Heading.css.js         0.18 KiB / gzip: 0.16 KiB
  dist/example.vanilla.css    0.23 KiB / gzip: 0.16 KiB
  dist/example.vanilla2.css   0.12 KiB / gzip: 0.13 KiB
  dist/example.vanilla3.css   0.04 KiB / gzip: 0.06 KiB
  

• However, if you remove the scope from package.json->name. It outputs the following:

  dist/main.js                                0.25 KiB / gzip: 0.20 KiB
  dist/main.css.js                            0.19 KiB / gzip: 0.15 KiB
  dist/Button.js                              0.13 KiB / gzip: 0.13 KiB
  dist/Heading.js                             0.14 KiB / gzip: 0.14 KiB
  dist/Button.css.js                          0.20 KiB / gzip: 0.16 KiB
  dist/Heading.css.js                         0.20 KiB / gzip: 0.16 KiB
  dist/Button.css.ts___example.vanilla.css    0.23 KiB / gzip: 0.17 KiB
  dist/main.css.ts___example.vanilla.css      0.12 KiB / gzip: 0.13 KiB
  dist/Heading.css.ts___example.vanilla.css   0.04 KiB / gzip: 0.06 KiB
  

System Info

Output of npx envinfo --system --npmPackages @vanilla-extract/css,@vanilla-extract/webpack-plugin,@vanilla-extract/esbuild-plugin,@vanilla-extract/vite-plugin,@vanilla-extract/sprinkles,webpack,esbuild,vite --binaries --browsers:

  System:
    OS: macOS 11.2.3
    CPU: (12) x64 Intel(R) Core(TM) i7-9750H CPU @ 2.60GHz
    Memory: 35.36 MB / 16.00 GB
    Shell: 5.8 - /usr/local/bin/zsh
  Binaries:
    Node: 16.13.2 - ~/.nvm/versions/node/v16.13.2/bin/node
    Yarn: 1.22.17 - /usr/local/bin/yarn
    npm: 8.1.2 - ~/.nvm/versions/node/v16.13.2/bin/npm
  Browsers:
    Chrome: 99.0.4844.83
    Edge: 99.0.1150.46
    Firefox: 94.0.1
    Safari: 14.0.3
  npmPackages:
    @vanilla-extract/esbuild-plugin: ^2.0.3 => 2.0.3 
    @vanilla-extract/vite-plugin: ^3.1.2 => 3.1.2 
    esbuild: 0.14.22 => 0.14.22 
    vite: 2.8.4 => 2.8.4

Related

• Some preliminary investigation was documented here: Vanilla Extract and Design Systems #620 (reply in thread)

[AndrewLeedham]

@graup pointed out in a conversation we had on discord that "virtual" modules should not be necessary for this case:

but the Vite documentation says that "virtual" should not be necessary for cases where the output is derived from a real file
https://vitejs.dev/guide/api-plugin.html#virtual-modules-convention

So I had a go at removing them, this is very rough around the edges and may re-introduce past bugs (e.g the .. thing) and likely break other stuff. But for the issue at hand it fixes paths: master...AndrewLeedham:AL-proper-paths.

This seems like the right direction to me, as Vite will see the CSS file properly, and it will have a proper file system path for generating the name. Curious on your thoughts here @mattcompiles?

[mattcompiles]

I have thought about this in the past as well. VE CSS is derived from a real file, however it is not derived from an AST transform or compilation process. Meaning it can't be sensibly mapped with a source map. Therefore I believe they do need to represented as virtual files. However feel free to correct me if I'm wrong and there is a way to do this I'm not aware of.

Either way though I think issue can be resolved even with virtual files. We might just need to update the file scope serialization.

[AndrewLeedham]

I see why sourcemaps wouldn't be possible, but is there any benefit of using virtual files over an "actual" path without sourcemaps?

[AndrewLeedham]

Reading through both the Vite and Rollup docs some more I guess it prevents other plugins from processing the import. Although that seems to be a feature of \0 which isn't in use here. Also it would seem we do want other plugins to process the import e.g vite:css.

A few potential options:

1. Replace virtual:vanilla-extract:style.vanilla.css$$$example-package with ./style.css, and just don't provide sourcemaps.
2. Refactor virtual:vanilla-extract:style.vanilla.css$$$example-package to something like virtual:vanilla-extract:style.css?package=example-package.
3. Provide some-sort of plugin option that allows you to format the last part of the path given the filescope. E.g:
   fileNames: (fileScope) => fileScope.filePath which would be used to create something like virtual:vanilla-extract:<user-value>?package=example-package.
4. Something else?

Curious on your thoughts here @mattcompiles. Happy to take a swing at implementing an approach, just not sure which direction to go.

[mattcompiles]

Would this issue be resolved if we just make the file path be the last thing in the filescope string instead? I don't remember any specific reason it was last.

e.g.

export function stringifyFileScope({
  packageName,
  filePath,
}: FileScope): string {
  return packageName ? `${packageName}$$$${filePath}` : filePath;
}

If so, happy to accept a PR. 😄

[graup]

Would this issue be resolved if we just make the file path be the last thing in the filescope string instead? I don't remember any specific reason it was last.

e.g.

export function stringifyFileScope({
  packageName,
  filePath,
}: FileScope): string {
  return packageName ? `${packageName}$$$${filePath}` : filePath;
}

Yes I believe that would be a slight improvement and fix this particular issue.

[AndrewLeedham]

That would definitely fix this issue yes. It still doesn't feel like the right solution to me though, because we are relying on the fact that Vite uses chunk.name which in this case will be the basename of the virtual ID. But probably better to get a solution in for now as it shouldn't affect anything else.

[graup]

Referring back to @AndrewLeedham 's comment, how about something like this? (Untested poc: remove prefix and change serialization to filePath?package=packageName)

As far as I can tell the packageName is only there for the hot reload and injectStyles, right?

[mattcompiles]

The package name is in the filescope as the filename is relative to that package. If you're using a package that contains VE files then all the file paths coming from that package will be relative to their package.json. What we need to be careful of here is allowing two separate files from having the same ID. This solution is probably ok from that regard.

One thing I'm not clear on though is how this solution is more ideal than what I suggested? Could someone just explain why this creates a better build output?

[graup]

The main advantage is that having the generated css files as proper (non-virtual) modules, with usable names and paths, works better with preserveModules and also possibly other plugins/options. Seems more flexible to me. (Didn't check but it might also enable us to remove the postcss code from the plugin and just have css files be processed by the rest of the rollup plugin chain.)

[mattcompiles]

This issue, along with a few others have lead me to doing a decent refactor of file scopes across the board. Hoping this PR will make everyone happy. #647