[Bug]: esbuild doesn't treeshake imports in `main.ts` and `preview.ts` (can lead to `ModuleNotFoundError`)

GreenTea124 commented 1 year ago

Describe the bug

If you have a barrel export file @libs/utils like this:

// Uses `path` or `fs` or `webpack` or something else that needs node's built-ins
export {someHelperForNode} from "someHelperForNode";

// Doesn't use node's build-ins
export {someHelper} from "someHelper";

And then you use it like this in .storybook/main.ts:

import {someHelperForNode} from "@libs/utils";

export default {
  // Creates config using that helper
}

And like this in .storybook/preview.ts:

import {someHelper} from "@libs/utils";

export default {
  // Creates config using that helper
}

Build will fail, and console will contain a bunch of logs with messages like:

ModuleNotFoundError: Module not found: Error: Can't resolve 'https' in 'C:\...'
BREAKING CHANGE: webpack < 5 used to include polyfills for node.js core modules by default.
This is no longer the case. Verify if you need this module and configure a polyfill for it.
If you want to include a polyfill, you need to:
        - add a fallback 'resolve.fallback: { "https": require.resolve("https-browserify") }'
        - install 'https-browserify'
If you don't want to include a polyfill, you can use an empty module like this:
        resolve.fallback: { "https": false }

It seems that some imports of node built-ins from someHelperForNode.ts file are leaking into preview.ts file even though it doesn't need them.

System

System:
    OS: Windows 10 10.0.19045
    CPU: (8) x64 Intel(R) Core(TM) i7-4790 CPU @ 3.60GHz
  Binaries:
    Node: 16.14.0 - C:\Program Files\nodejs\node.EXE
    npm: 8.3.1 - C:\Program Files\nodejs\npm.CMD
  Browsers:
    Chrome: 111.0.5563.65
    Edge: Spartan (44.19041.1266.0), Chromium (111.0.1661.44)
  npmPackages:
    @storybook/addon-essentials: ^7.0.0-rc.4 => 7.0.0-rc.4
    @storybook/addon-interactions: ^7.0.0-rc.4 => 7.0.0-rc.4
    @storybook/addon-links: ^7.0.0-rc.4 => 7.0.0-rc.4
    @storybook/addons: ^7.0.0-rc.4 => 7.0.0-rc.4
    @storybook/blocks: ^7.0.0-rc.4 => 7.0.0-rc.4
    @storybook/nextjs: ^7.0.0-rc.4 => 7.0.0-rc.4
    @storybook/react: ^7.0.0-rc.4 => 7.0.0-rc.4
    @storybook/theming: ^7.0.0-rc.4 => 7.0.0-rc.4

Additional context

No response

ubbe-xyz commented 1 year ago

Suffering the same here 😢

james-yeoman commented 1 year ago

I'm working in a codebase with a very similar problem.

The structure (partial) is as follows

<<root>>
| - .storybook/
|
|-\ - Shared Monorepo Package
|  | - Node-specific font cache util module
|  \ - Other utility modules
|
|-\ - Renderer Monorepo Package
|  |
|  |-\ - Node-specific Rendering Module
|  |  | - A module that uses the node-specific font cache util from Shared
|  |  \ - Server-side PDF-related code
|  |
|  |-\ - Browser/SVG Rendering Module
|  |  | - Several modules that export React elements
|  |  \ - A stories.tsx file that exposes all the React elements in this module
|  |
|  |-\ - index.ts
|  |  | - export * from "{{SVG Rendering Module}}"
|  |  \ - export {default as <node-specific module namespace name>} from "{{the node-specific module}}"

And the stories.tsx file from the Browser/SVG Rendering module is picked up in .storybook/main.ts as

../packages/<renderer package folder name>/src/**/*.stories.@(jsx|tsx)

in the stories array.

Everything was fine, until the node-specific font cache was updated to use process.env, at which point, all of the stories exposed by the Browser/SVG Rendering module's stories.tsx file, stopped working. Suddenly, it became clear that either Storybook, or the Vite Builder itself, isn't tree-shaking.

Under normal usage, even with the main application running in dev mode, Vite doesn't try to process the node-specific module. Vite's dev server appropriately tree-shakes.

The reason I know that it appropriately tree-shakes, is because the Renderer package creates a polymorphic API for defining objects, so that our designer section of our app can render objects identically to how they get produced in PDF format. So the fact that the designer functioned perfectly in dev mode proves that the problem lies somewhere in Storybook's build pipeline.

I realise that this isn't exactly the same issue (GreenTea124 was only talking about storybook config files), but from GreenTea124's description, I have a feeling that the two problems are caused by the same bit of code.

.storybook/preview.ts should only be executing code to configure the client, but is instead importing all of .storybook when it should only be importing .storybook/preview.ts. The exact same issue I'm experiencing, my issue is with story loading.

As such, I believe this issue should be expanded to talk about tree-shaking across all of storybook's dynamic file loading.

I'd also like to add the fact that I'm on Linux, so this isn't a Windows-specific bug, nor is it Next-JS-specific. I don't know why those two tags are on here, but they're incorrect

storybookjs / storybook