storybookjs / storybook

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

[Bug]: Storybook 7.0.9 - react-vite appends docgenInfo for dependency's classes that were eliminated by tree-shaking #22435

Closed nstuyvesant closed 1 year ago

nstuyvesant commented 1 year ago

Describe the bug

The package @carbon/charts-react has @carbon/charts as a dependency (but not externalized). Some of the exported classes in @carbon/charts are not used by @carbon/charts-react. When @carbon/charts-react is built using vite@4.3.5, unused classes from @carbon/charts are not included in the bundle (have noticed they are in the source map).

When @storybook/react-vite builds the storybook for @carbon/charts-react, it appends docgenInfo for classes in @carbon/charts that were eliminated from @carbon/charts-react's bundle. This causes the storybook to not load and log this JavaScript console error...

[Error] ReferenceError: Can't find variable: yd

    Module Code (iframe-e3d4aa14.js:972:17829)

This is what is injected by this line of code in @storybook/react-vite ...

yd.__docgenInfo = {
    description: '',
    methods: [
        { name: 'addLineEventListener', docblock: null, modifiers: [], params: [], returns: null },
        { name: 'addNodeEventListener', docblock: null, modifiers: [], params: [], returns: null },
        {
            name: 'traverse',
            docblock: null,
            modifiers: [],
            params: [
                { name: 'e', type: null },
                { name: 't', type: null },
                { name: 'n', type: null }
            ],
            returns: null
        },
        { name: 'getRightArrowIcon', docblock: null, modifiers: [], params: [], returns: null },
        { name: 'destroy', docblock: null, modifiers: [], params: [], returns: null }
    ],
    displayName: 'yd'
}

I searched for class yd in the bundle for @carbon/charts (packages/core/dist) and found it in one of its chunks...

class yd extends ee {
  constructor() {
    super(...arguments), this.type = "alluvial", this.renderType = Y.SVG, this.gradient_id = "gradient-id-" + Math.floor(Math.random() * 99999999999);
  }
 // ...
}

It appears as though what is passed as src in @storybook/react-vite here includes classes that were (or will be) eliminated by tree-shaking when vite/esbuild processes the sources.

To Reproduce

git clone https://github.com/nstuyvesant/carbon-charts.git
cd carbon-charts
git checkout pr-1554
git pull
yarn build
cd packages/react
# edit .storybook/main.ts to switch from react-webpack5 to react-vite
# yarn build:demo
# Then search in packages/react/demo/bundle for the string ";yd.__docgenInfo"

This error does not occur with the other packages in this monorepo that use @storybook/html-vite, @storybook/vue-vite, @storybook/angular, or @storybook/svelte Storybook frameworks.

System

Environment Info:

  System:
    OS: macOS 13.3.1
    CPU: (10) arm64 Apple M1 Pro
  Binaries:
    Node: 18.16.0 - /opt/homebrew/bin/node
    Yarn: 3.5.0 - /opt/homebrew/bin/yarn
    npm: 9.6.5 - /opt/homebrew/bin/npm
  Browsers:
    Chrome: 112.0.5615.137
    Firefox: 112.0.2
    Safari: 16.4
  npmPackages:
    @storybook/addon-essentials: ^7.0.9 => 7.0.9 
    @storybook/addon-interactions: ^7.0.9 => 7.0.9 
    @storybook/addon-links: ^7.0.9 => 7.0.9 
    @storybook/blocks: ^7.0.9 => 7.0.9 
    @storybook/manager-api: ^7.0.9 => 7.0.9 
    @storybook/react: ^7.0.9 => 7.0.9 
    @storybook/react-vite: ^7.0.9 => 7.0.9 
    @storybook/testing-library: ^0.1.0 => 0.1.0 
    @storybook/theming: ^7.0.9 => 7.0.9
    @vitejs/plugin-react: ^4.0.0 => 4.0.0
    vite: ^4.3.5 => 4.3.5

Additional context

Not sure if relevant, but I logged the vite config in viteFinal and the storybook:react-docgen-plugin has enforce = 'pre'...

  plugins: [
    {
      name: 'storybook:react-docgen-plugin',
      enforce: 'pre',
      transform: [AsyncFunction: transform]
    }
   // ...
]
nstuyvesant commented 1 year ago

Was able to temporarily workaround the issue by switching from @storybook/react-vite to @storybook/react-webpack5.

gtolarc commented 1 year ago

While not quite the same problem, the Since upgrading @vitejs/plugin-react to 4.0.0, I've also been getting the twin.macro module not found error. [vite:react-babel] /virtual:/@storybook/builder-vite/vite-app.js: Cannot find module 'twin.macro' from '/virtual:/@storybook/builder-vite'

The problem occurs with the combination pnpm monorepo & twin.macro & vite & storybook/react-vite.

I think this might be related. https://github.com/vitejs/vite-plugin-react/issues/16

nstuyvesant commented 1 year ago

@shilman - while I was able to workaround the issue using @storybook/react-webpack5, it's the only thing in the entire monorepo that requires webpack. As the trend is to move away from webpack to vite, I suspect this will be a serious problem. I spent quite a bit of time looking for a workaround until I threw in the towel and switched to webpack.

Along these lines, I would suggest a higher severity for this issue.

nstuyvesant commented 1 year ago

Per https://github.com/storybookjs/storybook/issues/21710, there's also the need to add this to .yarnrc.yml as this is a monorepo otherwise we get Error: Cannot find module '@storybook/react-webpack5/preset'...

packageExtensions:
  "@storybook/core-common@*":
      dependencies:
        "@storybook/react-webpack5": "^7.0.18"

So there's more friction to use @storybook/react-webpack5 vs. @storybook/react-vite.

nstuyvesant commented 1 year ago

I am now using 7.0.24 and storybook still crashes if I try to use @storybook/react-vite but the error in the JavaScript console has changed to: "SyntaxError: Importing binding name 'default' cannot be resolved by star export entries." without any line reference or other details.

nstuyvesant commented 1 year ago

In Firefox, the error is "Uncaught SyntaxError: ambiguous indirect export: default react-18.mjs:2:7"

@storybook/react-dom-shim/dist/react-18.mjs:2 is:

import ReactDOM from "/node_modules/react-dom/client.js";
nstuyvesant commented 1 year ago

In Chrome, the error is "Uncaught SyntaxError: The requested module '/node_modules/react/index.js' does not provide an export named 'default' (at react-18.mjs:1:8)"

The file, react-dom/client.js does not have a default export...

'use strict';

var m = require('react-dom');
if ("development" === 'production') {
  exports.createRoot = m.createRoot;
  exports.hydrateRoot = m.hydrateRoot;
} else {
  var i = m.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED;
  exports.createRoot = function(c, o) {
    i.usingClientEntryPoint = true;
    try {
      return m.createRoot(c, o);
    } finally {
      i.usingClientEntryPoint = false;
    }
  };
  exports.hydrateRoot = function(c, h, o) {
    i.usingClientEntryPoint = true;
    try {
      return m.hydrateRoot(c, h, o);
    } finally {
      i.usingClientEntryPoint = false;
    }
  };
}
developerdizzle commented 1 year ago

This is obviously not ideal but disabling docgen should fix this:

  typescript: {
    reactDocgen: false,
  },
nstuyvesant commented 1 year ago

Since upgrading to Storybook 7.1, I am getting a different (show-stopping) error when trying to run with @storybook/react-vite. I am not getting the original error.

The new error is Uncaught SyntaxError: ambiguous indirect export: default react-18.mjs:2:7. It corresponds to:

import ReactDOM from "/node_modules/react-dom/client.js";

Usually, I would expect the import to be

import * as ReactDOM from 'react-dom';
nstuyvesant commented 1 year ago

As the error is different now that I'm on Storybook 7.1 (and I have an open issue for it), I think the right thing to do is close this issue.