storybookjs / storybook

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

[Bug]: Error when building storybook after upgrading to 8.2 #28576

Closed stof closed 3 weeks ago

stof commented 3 weeks ago

Describe the bug

Our storybook setup was working fine with Storybook 8.1. However, when trying to upgrade to 8.2, I'm getting this report:

storybook v8.2.2

info => Starting manager..
info => Starting preview..
info Addon-docs: using MDX3
info => Using implicit CSS loaders
info => Using default Webpack5 setup
<i> [webpack-dev-middleware] wait until bundle finished
12% building 0/2 entries 1239/1287 dependencies 21/498 modulesCannot read properties of undefined (reading 'startsWith')
/home/stof/src/Incenteev/Incenteev/node_modules/storybook/bin/index.cjs:23
  throw error;
  ^

Error: callback(): The callback was already called.
    at context.callback (/home/stof/src/Incenteev/Incenteev/node_modules/loader-runner/lib/LoaderRunner.js:119:10)
    at Object.loader (/home/stof/src/Incenteev/Incenteev/node_modules/@storybook/csf-plugin/dist/webpack-loader.js:1:1878)

The error location in @storybook/csf-plugin/dist/webpack-loader.js corresponds to the call in https://github.com/storybookjs/storybook/blob/v8.2.2/code/lib/csf-plugin/src/webpack-loader.ts#L36 Apparently callback is caused multiple time which is forbidden by loader-runner. Looking at the code quickly, I suspect this might happen when an error is thrown during the execution of the callback itself (for a successful processing) as they are inside the try block: https://github.com/storybookjs/storybook/blob/v8.2.2/code/lib/csf-plugin/src/webpack-loader.ts#L25-L29

Reproduction link

none yet

Reproduction steps

No response

System

Storybook Environment Info:

  System:
    OS: Linux 6.5 Ubuntu 22.04.4 LTS 22.04.4 LTS (Jammy Jellyfish)
    CPU: (12) x64 12th Gen Intel(R) Core(TM) i7-1250U
    Shell: 5.8.1 - /bin/zsh
  Binaries:
    Node: 18.20.3 - /tmp/yarn--1720796321002-0.9126285275693176/node
    Yarn: 1.22.22 - /tmp/yarn--1720796321002-0.9126285275693176/yarn <----- active
    npm: 10.7.0 - /usr/bin/npm
  npmPackages:
    @storybook/addon-a11y: ^8.0.8 => 8.2.2 
    @storybook/addon-essentials: ^8.0.8 => 8.2.2 
    @storybook/addon-storysource: ^8.0.8 => 8.2.2 
    @storybook/blocks: ^8.0.8 => 8.2.2 
    @storybook/components: ^8.0.8 => 8.2.2 
    @storybook/core-events: ^8.0.8 => 8.2.2 
    @storybook/html: ^8.0.8 => 8.2.2 
    @storybook/html-webpack5: ^8.0.8 => 8.2.2 
    @storybook/manager-api: ^8.0.8 => 8.2.2 
    @storybook/source-loader: ^8.0.8 => 8.2.2 
    @storybook/theming: ^8.0.8 => 8.2.2 
    storybook: ^8.0.8 => 8.2.2 
    storybook-addon-mock: ^5.0.0 => 5.0.0

Additional context

No response

greptile-apps[bot] commented 3 weeks ago

Disclaimer This information might be inaccurate, due to it being generated automatically To resolve the issue of the callback being called multiple times in webpack-loader.ts, modify the loader function to ensure the callback is only called once. Specifically, move the callback call outside of the try block and handle errors separately. Here is the updated code snippet:

async function loader(this: LoaderContext, content: string, map: any) {
  const callback = this.async();
  const options = this.getOptions();
  const id = this.resourcePath;

  const sourceCode = await fs.readFile(id, 'utf-8');

  try {
    const makeTitle = (userTitle: string) => userTitle || 'default';
    const csf = loadCsf(content, { makeTitle }).parse();
    const csfSource = loadCsf(sourceCode, { makeTitle }).parse();
    enrichCsf(csf, csfSource, options);
    const formattedCsf = formatCsf(csf, { sourceMaps: true, inputSourceMap: map }, content);

    if (typeof formattedCsf === 'string') {
      callback(null, formattedCsf, map);
    } else {
      callback(null, formattedCsf.code, formattedCsf.map);
    }
  } catch (err: any) {
    // This can be called on legacy storiesOf files, so just ignore
    // those errors. But warn about other errors.
    if (!err.message?.startsWith('CSF:')) {
      console.warn(err.message);
    }
    callback(null, content, map);
  }
}

This ensures the callback is only called once, preventing the callback(): The callback was already called error.

References

/code/lib/csf-plugin/src/webpack-loader.ts /code/lib/csf-plugin

#### About Greptile This response provides a starting point for your research, not a precise solution. Help us improve! Please leave a ๐Ÿ‘ if this is helpful and ๐Ÿ‘Ž if it is irrelevant. [Ask Greptile](https://app.greptile.com/chat/github/storybookjs/storybook/next) ยท [Edit Issue Bot Settings](https://app.greptile.com/apps/github)
stof commented 3 weeks ago

As the error about a callback called multiple times seems to be triggered by another error, I also tried to modify https://github.com/storybookjs/storybook/blob/v8.2.2/code/lib/csf-plugin/src/webpack-loader.ts#L34 to log the whole error instead of only the message (Cannot read properties of undefined (reading 'startsWith')) to try to identify the cause of the first issue (which would likely still need to be resolved). I got this:

TypeError: Cannot read properties of undefined (reading 'startsWith')
    at contextifySourceUrl (/home/stof/src/Incenteev/Incenteev/node_modules/webpack/lib/NormalModule.js:124:13)
    at /home/stof/src/Incenteev/Incenteev/node_modules/webpack/lib/NormalModule.js:154:3
    at Array.map (<anonymous>)
    at contextifySourceMap (/home/stof/src/Incenteev/Incenteev/node_modules/webpack/lib/NormalModule.js:153:39)
    at NormalModule.createSource (/home/stof/src/Incenteev/Incenteev/node_modules/webpack/lib/NormalModule.js:799:5)
    at processResult (/home/stof/src/Incenteev/Incenteev/node_modules/webpack/lib/NormalModule.js:875:24)
    at /home/stof/src/Incenteev/Incenteev/node_modules/webpack/lib/NormalModule.js:971:5
    at /home/stof/src/Incenteev/Incenteev/node_modules/loader-runner/lib/LoaderRunner.js:407:3
    at iterateNormalLoaders (/home/stof/src/Incenteev/Incenteev/node_modules/loader-runner/lib/LoaderRunner.js:233:10)
    at iterateNormalLoaders (/home/stof/src/Incenteev/Incenteev/node_modules/loader-runner/lib/LoaderRunner.js:240:10)
    at /home/stof/src/Incenteev/Incenteev/node_modules/loader-runner/lib/LoaderRunner.js:255:3
    at context.callback (/home/stof/src/Incenteev/Incenteev/node_modules/loader-runner/lib/LoaderRunner.js:124:13)
    at Object.loader (/home/stof/src/Incenteev/Incenteev/node_modules/@storybook/csf-plugin/dist/webpack-loader.js:1:1758)

And the starting call at node_modules/@storybook/csf-plugin/dist/webpack-loader.js:1:1758 corresponds to https://github.com/storybookjs/storybook/blob/v8.2.2/code/lib/csf-plugin/src/webpack-loader.ts#L29

TylerNRobertson commented 3 weeks ago

Same thing happening on my end as well, exact same error too

Removing the @storybook/preview-api packages from addons seems to resolve the build error but I think this will break anything that uses the preview API in production builds

yannbf commented 3 weeks ago

@stof seems like the issue happens in this line in NormalModule from webpack: https://github.com/webpack/webpack/blob/277460b33bcc49c51acbbcd688672aa4ec685732/lib/NormalModule.js#L124

Could you try to check what's being passed to it and why is source undefined?

Can you also share more context in what your main.js file looks like?

yannbf commented 3 weeks ago

@TylerNRobertson do I understand correctly that you had a main.js file with something like:

import { StorybookConfig } from '@storybook/react-vite'

const config: StorybookConfig = {
  // ...
  addons: [
    '@storybook/preview-api', <-- did you have this?
  ]
}
export default config

If so, why would you have it? Such package was never intended to be used like that.

Can you share as much detail as possible in your project and provide a reproduction, so we can look into this further?

TylerNRobertson commented 3 weeks ago

@yannbf I just removed the package altogether and opted to use react state. Also @storybook/addon-storysource causes the same issue, so I've temporarily removed that as well. If neither of those are present then compilation passes without issue

Just can't show the story source previews for now but that's not critical in my case. However unsure why both those addons don't get bundled correctly

yannbf commented 3 weeks ago

@TylerNRobertson thanks for elaborating! Seems like both of you had @storybook/addon-storysource. Could you all please share as much info as possible to reproduce this issue? At least your .storybook/main.ts file.

This would help us immensely!

TylerNRobertson commented 3 weeks ago

Definitely @yannbf heres out main.ts, nothing major aside from a few customizations to load TS paths and styles as we'd like but the rest is pretty normal:

import path from 'path'
import TsconfigPathsPlugin from 'tsconfig-paths-webpack-plugin'

import { eventProps, ariaProps } from './constants'

const config = {
  stories: ['../stories/**/*.mdx', '../src/**/*.stories.@(js|jsx|ts|tsx)'],
  addons: [
    'storybook-dark-mode',
    '@storybook/addon-links',
    '@storybook/addon-essentials',
    '@storybook/addon-interactions',
    '@storybook/addon-jest',
    '@storybook/addon-a11y',
    '@storybook/addon-viewport',
    '@storybook/addon-storysource',
    '@storybook/addon-themes',
    {
      name: '@storybook/addon-styling-webpack',
      options: {
        sass: {
          implementation: require('sass'),
          sassOptions: {
            includePaths: [path.join(__dirname, 'src/styles'), path.join(__dirname, 'src/theme')]
          }
        }
      }
    }
  ],
  framework: {
    name: '@storybook/nextjs',
    options: { builder: { useSWC: true } }
  },
  docs: { autodocs: 'tag', defaultName: 'Documentation' },
  staticDirs: ['../public'],
  typescript: {
    reactDocgen: 'react-docgen-typescript',
    reactDocgenTypescriptOptions: {
      propFilter: {
        skipPropsWithName: ['children', ...eventProps, ...ariaProps],
        skipPropsWithoutDoc: true
      },
      shouldExtractLiteralValuesFromEnum: true,
      shouldRemoveUndefinedFromOptional: true
    }
  },
  webpackFinal: async (config) => {
    if (!config.resolve) return config

    config.resolve.plugins = [new TsconfigPathsPlugin()]
    config.resolve.alias = {
      ...config.resolve.alias,
      styles: require('path').resolve(__dirname, '../src/styles'),
      theme: require('path').resolve(__dirname, '../src/theme')
    }

    return config
  }
}
export default config
stof commented 3 weeks ago

here is my main.js:

const path = require('node:path')

const encoreConfig = require('../../webpack.config')
const babelExclude = encoreConfig.module.rules.find(rule => rule.test instanceof RegExp && rule.test.test('foo.js')).exclude

/**
 * @type {import('@storybook/html-webpack5').StorybookConfig}
 */
module.exports = {
  features: {
    postcss: false
  },
  stories: ['../../assets/scss/**/*.@(common|app).stories.js', '../../assets/scss/**/*.@(common|app).mdx', '../../assets/scss/*.mdx'],
  addons: [
    '@whitespace/storybook-addon-html',
    '@storybook/addon-a11y',
    '@storybook/addon-essentials',
    '@storybook/addon-storysource',
    'storybook-addon-mock'
  ],
  webpackFinal: async (config) => {
    config.resolve.fallback.crypto = false
    config.module.rules.push({
      test: /\.scss$/,
      use: ['style-loader', 'css-loader', {
        loader: 'sass-loader',
        options: {
          sassOptions: {
            includePaths: [path.resolve(__dirname, '../../node_modules')]
          }
        }
      }],
      include: path.resolve(__dirname, '../scss/')
    })

    const jsRule = config.module.rules.find(rule => rule.test instanceof RegExp && rule.test.test('foo.js'))

    if (jsRule) {
      jsRule.exclude = babelExclude
    }

    return config
  },
  framework: {
    name: '@storybook/html-webpack5',
    options: {}
  },
  env: (config) => ({
    ...config,
    DEVICE: 'desktop'
  })
}

When adding some console logging in webpack's NormalModule in contextifySourceMap, it got called with a sourceMap looking like that:

{
  version: 3,
  file: undefined,
  names: [],
  sourceRoot: undefined,
  sources: [ undefined ],
  sourcesContent: [
    '<REDACTED>'
  ],
  mappings: '',
  ignoreList: []
}

This undefined in sources is what causes the issue. However, I don't know where it was produced.

I tried commenting all addons except @storybook/addon-essentials in my main.js file but it still triggers the error

shilman commented 3 weeks ago

@valentinpalkovic I wonder if this is related to the sourcemap fixes you added in 8.2?

valentinpalkovic commented 3 weeks ago

@shilman Can definitely be related! I am taking a look!

ckifer commented 3 weeks ago

+1 recharts having the same issue with addon-storysource https://github.com/recharts/recharts/blob/3.x/storybook/main.ts#L15 Thanks!

valentinpalkovic commented 3 weeks ago

I have figured out the issue, and I will provide a fix soon. We will prepare a hotfix release on Monday at the latest

valentinpalkovic commented 3 weeks ago

@shilman This PR will fix the issue: https://github.com/storybookjs/storybook/pull/28585

@ckifer, @stof, @TylerNRobertson Could you please try out the following canary version?: 0.0.0-pr-28585-sha-af48f0d6