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]: bare import path resolution doesn't work under Yarn PNPM linker #29620

Open
ethanwu10 opened this issue Nov 14, 2024 · 1 comment · May be fixed by #29639
Open

[Bug]: bare import path resolution doesn't work under Yarn PNPM linker #29620

ethanwu10 opened this issue Nov 14, 2024 · 1 comment · May be fixed by #29639

Comments

@ethanwu10
Copy link

Describe the bug

SB attempts to strip all leading node_modules/ components off of the path of an import to get Vite to bundle correctly:

// We need to convert from an absolute path, to a traditional node module import path,
// so that vite can correctly pre-bundle/optimize
export function stripAbsNodeModulesPath(absPath: string) {
// TODO: Evaluate if searching for node_modules in a yarn pnp environment is correct
const splits = absPath.split(`node_modules${sep}`);
// Return everything after the final "node_modules/"
const module = normalizePath(splits[splits.length - 1]);
return module;
}

This doesn't work correctly under the Yarn PNPM linker, which constructs a node_modules tree via symlinks into a node_modules/.store directory. So, all packages are stored under /node_modules/.store/<packagename>-virtual-<hash>/package/, and require.resolve will return absolute paths into here. Thus, stripAbsNodeModulesPath is called since node_modules is in the path, and ti will return a path like .store/..., which is not a valid import path. Vite then throws errors about these paths:

3:37:05 PM [vite] Pre-transform error: Failed to resolve import ".store/@storybook-react-virtual-d2ec5c3452/package/dist/entry-preview.mjs" from "/virtual:/@storybook/builder-vite/vite-app.js". Does the file exist?
3:37:05 PM [vite] Internal server error: Failed to resolve import ".store/@storybook-react-virtual-d2ec5c3452/package/dist/entry-preview.mjs" from "/virtual:/@storybook/builder-vite/vite-app.js". Does the file exist?
  Plugin: vite:import-analysis
  File: /virtual:/@storybook/builder-vite/vite-app.js:7:81
  5  |      
  6  |    const getProjectAnnotations = async (hmrPreviewAnnotationModules = []) => {
  7  |      const configs = await Promise.all([hmrPreviewAnnotationModules[0] ?? import('.store/@storybook-react-virtual-d2ec5c3452/package/dist/entry-preview.mjs'),
     |                                                                                  ^
  8  |  hmrPreviewAnnotationModules[1] ?? import('.store/@storybook-react-virtual-d2ec5c3452/package/dist/entry-preview-docs.mjs'),

Reproduction link

https://github.com/ethanwu10/sb-yarn-pnpm-vite-repro

Reproduction steps

  1. Clone repro
  2. Run yarn storybook
  3. Observe errors from Vite + storybook iframe does not load

System

System:
    OS: macOS 14.7
    CPU: (10) arm64 Apple M2 Pro
    Shell: 5.9 - /bin/zsh
  Binaries:
    Node: 23.1.0 - /private/var/folders/44/y4hzbjt52nv8ddgrvy_p85g00000gn/T/xfs-d59be415/node
    Yarn: 4.5.1 - /private/var/folders/44/y4hzbjt52nv8ddgrvy_p85g00000gn/T/xfs-d59be415/yarn <----- active
    npm: 10.9.0 - /opt/homebrew/bin/npm
  Browsers:
    Safari: 18.0
  npmPackages:
    @storybook/addon-essentials: ^8.4.4 => 8.4.4 
    @storybook/addon-interactions: ^8.4.4 => 8.4.4 
    @storybook/addon-onboarding: ^8.4.4 => 8.4.4 
    @storybook/blocks: ^8.4.4 => 8.4.4 
    @storybook/react: ^8.4.4 => 8.4.4 
    @storybook/react-vite: ^8.4.4 => 8.4.4 
    @storybook/test: ^8.4.4 => 8.4.4 
    eslint-plugin-storybook: ^0.11.0 => 0.11.0 
    storybook: ^8.4.4 => 8.4.4

Additional context

No response

@valentinpalkovic
Copy link
Contributor

Hi @ethanwu10

Could you please try out the following canary release?

npx [email protected] upgrade

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

Successfully merging a pull request may close this issue.

2 participants