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.28k forks source link

Storybook crashes in browser when array wrapper syntax is used inside an arrow-function component with arguments #17025

Open bowmsamman opened 2 years ago

bowmsamman commented 2 years ago

Describe the bug When running storybook using start-storybook, rendering a component using arrow function syntax with one or more arguments, then adding a prop inside that component that renders elements in an array, causes Storybook to crash (see example below). After running storybook and navigating to this story, storybook will completely freeze and the tab will need to be closed.

This crash doesn't occur with a static build (running build-storybook and then opening the result in a browser).

To Reproduce

The following story will crash:

<!-- Example.stories.mdx -->

import { Canvas, Meta, Story } from '@storybook/addon-docs';

export const Example = () => 'Example';

<Meta title="Components/Example" component={Example} />

<Canvas>
  <Story name="Example">{(args) => <Example array={[<div key="foo">foo</div>, <div key="bar">bar</div>]} />}</Story>
</Canvas>

Step-by-step reproduction:

  1. Add the aforementioned story to your storybook
  2. Run storybook using start-storybook (this bug doesn't happen when using a static build)
  3. Navigate to the story in your browser (note that doing so will make your browser tab crash)

The arrow function declared as a child of our <Story> element is the culprit. The crash appears to go away if you do either one of the following:

System

Environment Info:

  System:
    OS: macOS 12.0.1
    CPU: (12) x64 Intel(R) Core(TM) i7-8750H CPU @ 2.20GHz
  Binaries:
    Node: 14.17.3 - ~/.nvm/versions/node/v14.17.3/bin/node
    npm: 6.14.15 - ~/.nvm/versions/node/v14.17.3/bin/npm
  Browsers:
    Chrome: 96.0.4664.110
    Edge: 95.0.1020.53
    Firefox: 89.0.2
    Safari: 15.1
  npmPackages:
    @storybook/addon-a11y: ^6.4.9 => 6.4.9 
    @storybook/addon-controls: ^6.4.9 => 6.4.9 
    @storybook/addon-docs: ^6.4.9 => 6.4.9 
    @storybook/addon-links: ^6.4.9 => 6.4.9 
    @storybook/addon-toolbars: ^6.4.9 => 6.4.9 
    @storybook/addon-viewport: ^6.4.9 => 6.4.9 
    @storybook/addons: ^6.4.9 => 6.4.9 
    @storybook/builder-webpack5: ^6.4.9 => 6.4.9 
    @storybook/manager-webpack5: ^6.4.9 => 6.4.9 
    @storybook/react: ^6.4.9 => 6.4.9 
    @storybook/theming: ^6.4.9 => 6.4.9 

Additional context Occurs in the following browsers:

Google Chrome Version 96.0.4664.93 Firefox 89.0.2 Safari Version 15.1

ph-fritsche commented 2 years ago

I think I encountered the same error. It happens in other story formats too.

import React from 'react'
import { Meta, Story } from '@storybook/react'

export default {
    title: 'Crash',
} as Meta

function Component(props: {
    foo?: Array<{bar?: React.ReactElement}>,
}) {
    return null
}

export const WorkingA: Story<unknown> = () => (
    <Component foo={[
        {bar: <div />},
    ]}/>
)

export const WorkingB: Story<{x: unknown}> = ({x}) => (
    <Component foo={[
        {},
    ]}/>
)

export const Crash: Story<{x: unknown}> = ({x}) => (
    <Component foo={[
        {bar: <div />},
    ]}/>
)
Floffah commented 2 years ago

I get the same problem with this code:

import {Canvas, Meta, Story} from "@storybook/addon-docs";
import Carousel, {CarouselPage} from "./Carousel";

<Meta title="Components/Info/Carousel" component={Carousel}/>

# Carousel

## Stories

export const CarouselStory = (args) => (
    <div className="w-[60rem]">
      <Carousel
          items={[
              <CarouselPage
                  title="Page 1"
                  message="A basic page"
                  backgroundUrl="https://palia.xyz/assets/official/s6/wallpaper/Palia__ConceptGrassland.jpg"
                  key={0}
                  _indexOnCarousel={0}
              />,
              <CarouselPage
                  title="Page 2"
                  message="Another basic page"
                  backgroundUrl="https://palia.xyz/assets/official/s6/wallpaper/Palia__ConceptGrassland.jpg"
                  key={1}
                  _indexOnCarousel={1}
              />,
          ]}
          {...args}/>
    </div>
);

### Basic

<Canvas>
    <Story
        name="Basic"
    >
        {CarouselStory.bind({})}
    </Story>
</Canvas>

as soon as i remove the items array it unfreezes

a workaround i found that fixes the problem is by changing the array function to pass the items from a separate variable:

## Stories

export const CarouselItems = [
    <CarouselPage
        title="Page 1"
        message="A basic page"
        backgroundUrl="https://palia.xyz/assets/official/s6/wallpaper/Palia__ConceptGrassland.jpg"
        key={0}
        _indexOnCarousel={0}
    />,
    <CarouselPage
        title="Page 2"
        message="Another basic page"
        backgroundUrl="https://palia.xyz/assets/official/s6/wallpaper/Palia__ConceptGrassland.jpg"
        key={1}
        _indexOnCarousel={1}
    />,
];

export const CarouselStory = (args) => (
    <div className="w-[60rem]">
        <Carousel
            items={CarouselItems}
            {...args}/>
    </div>
);
Skladnayazebra commented 2 years ago

@Floffah Great idea, thank you! Sadly, this won't help when we need dynamic props in these JSX chunks, but at least it's possible to deal with static props.

skylarmb commented 2 years ago

I am encountering the exact same issue. This popped up immediately when migrating from 5.x to 6.4.0

I can confirm that declaring the array of elements completely outside the story function component works, but is obviously less than ideal.

Naes0 commented 2 years ago

Currently experiencing the same issue. I also need dynamic props so no workaround currently :/.

Floffah commented 2 years ago

Currently experiencing the same issue. I also need dynamic props so no workaround currently :/.

if you mean dynamic props from the controls tab i did it like this:

export const CarouselPageStory = (args) => (
    <CarouselPageStorybookWorkaround {...args}/>
)

<Canvas>
    <Story name="Basic" args={{
        title: "Basic carousel page",
        message: "Nothing fancy",
        _indexOnCarousel: 0,
    }}>
        {CarouselPageStory.bind({})}
    </Story>
</Canvas>

and in a separate file:

export function CarouselPageStorybookWorkaround(p: CarouselPageProps) {
    return (
        <div className="w-[60rem]">
            <Carousel items={[
                <CarouselPage {...p} key={0}/>,
            ]}/>
        </div>
    )
}

this is not ideal but it works well enough for me

mrclay commented 2 years ago

I'm not sure my crashes are related but, removing @storybook/addon-docs eliminated them. I was seeing deep call stacks in the react-element-to-jsx-string module. See https://github.com/algolia/react-element-to-jsx-string/issues/681

alexbchr commented 2 years ago

I'm not sure my crashes are related but, removing @storybook/addon-docs eliminated them. I was seeing deep call stacks in the react-element-to-jsx-string module. See algolia/react-element-to-jsx-string#681

For us, the breaking part in the addon-docs was the Dynamic Source Code rendering for React. So I added the following in the Storybook preview.js and that fixed the issue:

export const parameters = {
  docs: {
    source: { type: 'code' }, // Default here is 'dynamic'
  },
}

Of course, the drawback here is that the Source code showing up for every story in the Docs tab will be the source code of the story itself instead of being a JSX version of it, but for us it is definitely better to have that than having to disable the addon-docs plugin entirely.

TodBob commented 2 years ago

I'm not sure my crashes are related but, removing @storybook/addon-docs eliminated them. I was seeing deep call stacks in the react-element-to-jsx-string module. See algolia/react-element-to-jsx-string#681

For us, the breaking part in the addon-docs was the Dynamic Source Code rendering for React. So I added the following in the Storybook preview.js and that fixed the issue:

export const parameters = {
  docs: {
    source: { type: 'code' }, // Default here is 'dynamic'
  },
}

Of course, the drawback here is that the Source code showing up for every story in the Docs tab will be the source code of the story itself instead of being a JSX version of it, but for us it is definitely better to have that than having to disable the addon-docs plugin entirely.

It's fixed my issue. Thanks!

felipenmoura commented 2 years ago

Yes, having the same issue here. The weird thing is that I uninstalled @storybook/addon-docs and removed every single line that mentioned it in my project, and it was still being loaded in storybook! I had to manually rm -rf node_modules/@storybook/addon-docs to really make it go.

And yet...it is still freezing the browser when I have a story like this:

<Sample {...props}
      propList={[
        {el : <b>propListItem 1</b>},
        {el : <b>propListItem 2</b>},
        {el : <b>propListItem 3</b>}
      ]}
  />

Please notice that this propsList doesn't even need to be used in the component...just by sending it as a prop is enough.

Interestingly enough (or not), if you simply move it to somewhere els (out of the story function) it works as expected.
Of course, in this case, you simply can't use the controls to affect those elements.

const elsList = [
  {el :<b key="1">propListItem 1</b> },
  {el :<b key="3">propListItem 2</b> },
  {el :<b key="3">propListItem 3</b> },
];

<Sample {...props}
  propList={elsList}
/>

Also worth noting that if you create a function like generateList that returns that list...it will NOT work.

One more useful info.
If you use map or splice in elsList it will still work. That makes me believe it's exclusively related to rendering child elements in the story render, not dealing with child elements in it.

R-154 commented 2 years ago

My issue is related, once i add @storybook/addon-essentials and enable 'docs' or 'controls' for that one storybook is crashing on some stories that have an array of ReactElements in the template of the story.

Is there in the meantime a fix that works on having source type: 'dynamic'?

jtiala commented 1 year ago

Still experiencing the same with 7.0.0-beta.62.

Kayyow commented 1 year ago

FYI We just experienced the same issue with a JSX.Element embedded in an array and the issue disappeared after upgrading Storybook and its plugins packages from 7.0.0-beta.17 to 7.0.0-rc.2 !

StevenCreates commented 1 year ago

Still experiencing this issue with 7.0.6

lucapollani commented 1 year ago

Same with 7.1.0

Kneesal commented 1 year ago

I'm not sure my crashes are related but, removing @storybook/addon-docs eliminated them. I was seeing deep call stacks in the react-element-to-jsx-string module. See algolia/react-element-to-jsx-string#681

For us, the breaking part in the addon-docs was the Dynamic Source Code rendering for React. So I added the following in the Storybook preview.js and that fixed the issue:

export const parameters = {
  docs: {
    source: { type: 'code' }, // Default here is 'dynamic'
  },
}

Of course, the drawback here is that the Source code showing up for every story in the Docs tab will be the source code of the story itself instead of being a JSX version of it, but for us it is definitely better to have that than having to disable the addon-docs plugin entirely.

To add on to this, if you want to mitigate the drawback to one file, we can add the parameters to the Meta object in our specific story file as opposed to adding it to our preview.js file

I am using Storybook 7.3.2


const meta: Meta<typeof Component> = {
  title: 'projectName/Component',
  component: Component,
  parameters: {
    docs: {
      source: { type: 'code' }
    }
  }
}

export default meta;
kylemh commented 1 year ago

https://github.com/storybookjs/storybook/issues/17025#issuecomment-1055654634

This was also a solution for me to get around a problem where fragments as values of an object as a value of a prop cause stack overflow issues in dev server specifically.

So:

<SomeComponent
  label={{
    content: <><h1>Header</h1><span>Some description</span></>, // problem here - fragment as object value within prop value
    isHidden: false,
  }}
/>

This would crash a Story in dev server until I changed the docs generation.

hanshank commented 5 months ago

Seems to still be an issue in 7.6.19 - Was able to get around it by using the solution highlighted above:

export const parameters = {
  docs: {
    source: { type: 'code' }, // Default here is 'dynamic'
  },
}

Thanks for sharing the workaround! Does anyone know if this issue is fixed in v8?

rj-wowza commented 4 months ago

I have confirmed this is still an issue in version 8.1.3