Skip to content
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

[Bug]: Chromatic TurboSnap incompatible with preview-stats.json generated from builder-vite #27172

Closed
AlexAtVista opened this issue May 16, 2024 · 1 comment · Fixed by #27218

Comments

@AlexAtVista
Copy link
Contributor

Describe the bug

In Storybook 8 the vite-plugin-turbosnap package was ported into @storybook/builder-vite

I had previously reported an issue with vite-plugin-turbosnap as on Windows it generates files paths for name and id using the system path separators (\\) instead of posix separators (/).

With Webpack, even on Windows, name and id always use posix separators. This has been assumed to always be the case for the chromatic-cli tool.

With the current preview-stats.json file generated by builder-vite, it is incompatible with turbosnap in Chromatic.

To fix this, the normalize function in webpack-stats-plugin.ts needs to be modified to convert workingDir and filename to posix paths, using the same posix transformer as chromatic-cli.

export function pluginWebpackStats({ workingDir }: WebpackStatsPluginOptions): WebpackStatsPlugin {
+ // Replaces Windows-style backslash path separators with POSIX-style forward slashes, because the
+ // Webpack stats use forward slashes in the `name` and `moduleName` fields. Note `changedFiles`
+ // already contains forward slashes, because that's what git yields even on Windows.
+ const posix = (localPath: string) => localPath.split(path.sep).filter(Boolean).join(path.posix.sep);
  /**
   * Convert an absolute path name to a path relative to the vite root, with a starting `./`
   */
  function normalize(filename: string) {
    // Do not try to resolve virtual files
    if (filename.startsWith('/virtual:')) {
      return filename;
    }
    // Otherwise, we need them in the format `./path/to/file.js`.
    else {
+     const posixWorkingDir = posix(workingDir);
+     const posixFilename = posix(filename);

+     const relativePath = path.posix.relative(posixWorkingDir, stripQueryParams(posixFilename));
-     const relativePath = path.relative(workingDir, stripQueryParams(filename));
      // This seems hacky, got to be a better way to add a `./` to the start of a path.
      return `./${relativePath}`;
    }
  }

To Reproduce

  1. Create a storybook project using builder-vite
  2. Run storybook build --stats-json on a Window environment
  3. Check the name and id fields in storybook-static/preview-stats.json for Windows path separators

System

Storybook Environment Info:

  System:
    OS: Windows 10 10.0.19045
    CPU: (8) x64 11th Gen Intel(R) Core(TM) i7-1165G7 @ 2.80GHz
  Binaries:
    Node: 18.18.2 - C:\Program Files\nodejs\node.EXE
    Yarn: 1.22.22 - C:\Program Files\nodejs\yarn.CMD
    npm: 9.8.1 - C:\Program Files\nodejs\npm.CMD <----- active
  Browsers:
    Edge: Chromium (124.0.2478.105)
  npmPackages:
    @storybook/addon-a11y: ^8.0.10 => 8.0.10
    @storybook/addon-essentials: ^8.0.10 => 8.0.10
    @storybook/addon-interactions: ^8.0.10 => 8.0.10
    @storybook/addon-links: ^8.0.10 => 8.0.10
    @storybook/addon-onboarding: ^8.0.10 => 8.0.10
    @storybook/addon-viewport: ^8.0.10 => 8.0.10
    @storybook/blocks: ^8.0.10 => 8.0.10
    @storybook/builder-vite: ^8.0.10 => 8.0.10
    @storybook/manager-api: ^8.0.10 => 8.0.10
    @storybook/react: ^8.0.10 => 8.0.10
    @storybook/react-vite: ^8.0.10 => 8.0.10
    @storybook/test: ^8.0.10 => 8.0.10
    @storybook/test-runner: ^0.18.0 => 0.18.0
    @storybook/theming: ^8.0.10 => 8.0.10
    chromatic: ^10.5.0 => 10.9.6
    eslint-plugin-storybook: ^0.8.0 => 0.8.0
    msw-storybook-addon: ^2.0.2 => 2.0.2
    storybook: ^8.0.10 => 8.0.10
    storybook-addon-remix-react-router: ^3.0.0 => 3.0.0

Additional context

No response

@valentinpalkovic
Copy link
Contributor

Hi @AlexAtVista

Thank you for reporting such a detailed issue.

Would you like to contribute and create a PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants