storybookjs / storybook

Storybook is the industry standard workshop for building, documenting, and testing UI components in isolation
https://storybook.js.org
MIT License
84.68k stars 9.32k forks source link

[Bug]: Dependencies collide in pnpm monorepo with two SB projects of differing versions #20659

Closed justintemps closed 1 year ago

justintemps commented 1 year ago

Describe the bug

I have a monorepo with two Storybook packages. One is a React package that I just migrated to storybook@next and the other is a Wingsuit project for twig that uses storybook@6.5.15.

I'm using pnpm to manage dependencies and turborepo to orchestrate tasks.

Before migrating my react package to @next in anticipation of version 7, both projects were on the same version of both Storybook and webpack and everything worked fine. After I bumped the React version, however, I can only ever get one of the packages to build.

It seems like the Wingsuit/Twig project (which uses storybook@6.5.15 will only build if I install dependencies with pnpm install with node-linker = hoisted set in .npmrc. However, in that case, the React package seems unable to find its own hoisted dependencies:

react:build: ERR! Error: Cannot find module '@storybook/react-vite/preset'

Conversely, I can get the React package (which uses storybook@next) to build by installing dependencies normally without anything set in .npmrc, but the Twig package throws build errors:

twig:build: /Users/justinsmith/Dev/designsystem/node_modules/.pnpm/@storybook+mdx2-csf@1.0.0-next.4/node_modules/@storybook/mdx2-csf/loader.js:23
twig:build:   const options = Object.assign({}, this.getOptions(), {
twig:build:                                          ^
twig:build: 
twig:build: TypeError: this.getOptions is not a function

I think this is because Webpack 4 resolves loaders incorrectly unless it's hoisted, but I can't figure out why hoisting should break the storybook@next dependency graph.

To Reproduce

Here is a basic a reproduction repo: https://github.com/justintemps/repro-storybook-versions

To reproduce:

1. Clone the repo
2. Install dependencies with pnpm install
3. Run pnpm build:react. It should build successfully
4. Run pnpm build:twig. It should throw a build error
5. Run pnpn clean to remove deps and generated files and start over.
6. Uncomment the line `node-linker = hoisted` in .npmrc
7. Install dependencies again with pnpm install
8. Run pnpm build:twig. It should build successfully
9. Run pnpm built:react. It should throw a build error

System

System:
    OS: macOS 13.0
    CPU: (10) x64 Apple M1 Max
  Binaries:
    Node: 16.14.0 - ~/.nvm/versions/node/v16.14.0/bin/node
    Yarn: 1.22.10 - ~/.config/yarn/global/node_modules/.bin/yarn
    npm: 8.3.1 - ~/.nvm/versions/node/v16.14.0/bin/npm
  Browsers:
    Chrome: 108.0.5359.124
    Firefox: 107.0.1
    Safari: 16.1

Additional context

No response

optimistiks commented 1 year ago

We're having a similar issue with yarn v3 workspaces (no plug and play), where one package depends on Storybook v6 and the other on Storybook v7.

The storybook package get's hoisted to the top level node_modules, whereas the @storybook/react-webpack5 remains in the package node_modules.

monorepo root
    node_modules
        @storybook 
            (no react-webpack5 package here)
        storybook ("version": "7.0.0-beta.29")
    packages
        our-library
            node_modules
                @storybook
                    react-webpack5 ("version": "7.0.0-beta.29")
                (no storybook package here)

That causes an error if you try to run yarn storybook in our-library

WARN   Failed to load preset: "@storybook/react-webpack5/preset"
ERR! Error: Cannot find module '@storybook/react-webpack5/preset'
ERR! Require stack:
ERR! - /monorepoRoot/node_modules/@storybook/core-common/dist/index.js
ERR! - /monorepoRoot/node_modules/@storybook/telemetry/dist/index.js
ERR! - /monorepoRoot/node_modules/@storybook/cli/dist/generate.js
ERR! - /monorepoRoot/node_modules/@storybook/cli/bin/index.js
ERR! - /monorepoRoot/node_modules/storybook/index.js
IanVS commented 1 year ago

I'll be honest, I'm not sure if this is something that is officially supported. I've had trouble in the past with multiple different storybook versions in packages in a monorepo. Is there any way to keep storybook packages from being hoisted? I know there used to be a way with older versions of yarn, but I'm not sure about recent versions or pnpm. But I'll try taking a look to see if I can figure something out.

justintemps commented 1 year ago

@IanVS may be this could work for older versions of Storybook that use webpack 4? https://github.com/webpack/webpack/issues/5087#issuecomment-1157689388

justintemps commented 1 year ago

I've looked at this today and I'm starting to think that it might be an unsolvable problem and that the best way to avoid it for now is just not to try to build the two projects together. That's annoying, because before I was able to set styles for the same component in two different storybook projects at the same time, but I don't see a way around it until Wingsuit upgrades to SB 7 or at least to Webpack 5.

herzaso commented 1 year ago

I have the same problem with @storybook/react-vite/preset. Storybook v7.0.0-beta.34 Vite v4.0.0 Yarn 3.3.1 with workspaces (no PnP)

ta1m1kam commented 1 year ago

I have same too...

Storybook v7.0.0-beta.36
Vite v4.0.0
build management: lerna
justintemps commented 1 year ago

I eventually worked around this by installing the two Storybook projects separately. My package.json ended up looking like this:

"scripts": {
    "clean:deps": "rm -rf pnpm-lock.yaml node_modules && pnpm -r exec rm -rf node_modules",
    "storybook7:install": "pnpm clean:deps && pnpm --filter storybook7... install",
    "storybook6:install": "pnpm clean:deps && pnpm --node-linker=hoisted --filter storybook6... install"
}

where storybook6and storybook7are two different Storybook packages in a monorepo. You obviously need to build the projects separately also and it's not possible to work on both at once, but I don't think there's a better solution for now.

herzaso commented 1 year ago

For future reference, I managed to solve my problem using Yarn's no-hoist option - https://yarnpkg.com/configuration/yarnrc#nmHoistingLimits This is less ideal, but it works...

justintemps commented 1 year ago

@IanVS given that this issue probably doesn't have an official solution, at least not without upgrading older versions of Storybook or at least using them with Webpack5, but that it does have a workaround, perhaps I should close this issue? Happy to leave it if it's useful to have open.

IanVS commented 1 year ago

I don't think there's really anything we can do to support this any better than the workarounds that have been identified, so yeah let's close it out. Lord knows we have enough open issues. 😅

Thanks for bringing it up and for sharing the workarounds!

kerryj89 commented 1 month ago

My root package.json had

{
    ...
        "nohoist": [
            "@storybook",
            "**/@storybook",
            "**/@storybook/**"
        ]
    ...
}

Running yarn would install it to the right spot, but running npx storybook upgrade would cause the no-hoisted @storybook packages to get hoisted and install to root.

I already had the following in my main.ts which when output resolve to the proper path.

/**
 * This function is used to resolve the absolute path of a package.
 * It is needed in projects that use Yarn PnP or are set up within a monorepo.
 */
function getAbsolutePath(value: string): any {
    return dirname(require.resolve(join(value, 'package.json')));
}

Finally, I did something we are all familiar with... tossing our hands in the air and deleting the lockfiles and node_modules out of frustration and seeing that it worked.