Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

v6.1.4 generates infinite loop #13242

Closed
Shawnxkwang opened this issue Nov 24, 2020 · 8 comments
Closed

v6.1.4 generates infinite loop #13242

Shawnxkwang opened this issue Nov 24, 2020 · 8 comments

Comments

@Shawnxkwang
Copy link

Describe the bug

RangeError: Maximum call stack size exceeded
    at isPrototype (http://localhost:6006/vendors~main.4bf0ecfac21b9c0ba56f.bundle.js:97916:14)
    at baseKeys (http://localhost:6006/vendors~main.4bf0ecfac21b9c0ba56f.bundle.js:95168:8)
    at keys (http://localhost:6006/vendors~main.4bf0ecfac21b9c0ba56f.bundle.js:100247:56)
    at http://localhost:6006/vendors~main.4bf0ecfac21b9c0ba56f.bundle.js:96320:17
    at baseForOwn (http://localhost:6006/vendors~main.4bf0ecfac21b9c0ba56f.bundle.js:94489:20)
    at mapValues (http://localhost:6006/vendors~main.4bf0ecfac21b9c0ba56f.bundle.js:100405:3)
    at inferType (http://localhost:6006/vendors~main.4bf0ecfac21b9c0ba56f.bundle.js:21143:48)
    at http://localhost:6006/vendors~main.4bf0ecfac21b9c0ba56f.bundle.js:21144:14
    at http://localhost:6006/vendors~main.4bf0ecfac21b9c0ba56f.bundle.js:100406:34
    at http://localhost:6006/vendors~main.4bf0ecfac21b9c0ba56f.bundle.js:96325:11

To Reproduce
Steps to reproduce the behavior:
official doc
https://storybook.js.org/docs/react/writing-stories/introduction#using-args

// ButtonGroup.stories.js

import React from 'react';
import { ButtonGroup } from '../ButtonGroup';
import * as ButtonStories from './Button.stories';

export default {
  title: 'ButtonGroup',
  component: ButtonGroup,
}
const Template = (args) => <ButtonGroup {...args} />

export const Pair = Template.bind({});
Pair.args = {
  buttons: [ ...ButtonStories.Primary.args, ...ButtonStories.Secondary.args ],
  orientation: 'horizontal',
};

Expected behavior

I'd expect it to just work when I follow the official doc.

Screenshots
image

Code snippets
see above.

System
Please paste the results of npx sb@next info here.

 System:
    OS: Linux 5.4 Manjaro Linux
    CPU: (12) x64 Intel(R) Core(TM) i7-10710U CPU @ 1.10GHz
  Binaries:
    Node: 12.16.1 - ~/.nvm/versions/node/v12.16.1/bin/node
    Yarn: 1.22.5 - /usr/bin/yarn
    npm: 6.13.4 - ~/.nvm/versions/node/v12.16.1/bin/npm
  Browsers:
    Firefox: 81.0
  npmPackages:
    @storybook/addon-actions: ^6.1.4 => 6.1.4 
    @storybook/addon-essentials: ^6.1.4 => 6.1.4 
    @storybook/addon-links: ^6.1.4 => 6.1.4 
    @storybook/node-logger: ^6.1.4 => 6.1.4 
    @storybook/preset-create-react-app: ^3.1.5 => 3.1.5 
    @storybook/react: ^6.1.4 => 6.1.4 

Additional context

@shilman
Copy link
Member

shilman commented Nov 24, 2020

Do you have a repro repo you can share? Or at least the definitions for Button and its stories?

@shilman shilman self-assigned this Nov 24, 2020
@shilman shilman added this to the 6.1 args milestone Nov 24, 2020
@Shawnxkwang
Copy link
Author

Here is the story I wrote


import React, { ComponentProps } from 'react';
import { Story } from '@storybook/react/types-6-0';
import { Button } from '.';

const ButtonStory = {
  title: 'Button',
  component: Button,
};

const BaseTemplate: Story<ComponentProps<typeof Button>> = args => (
  <Button {...args} />
);

export const Base = BaseTemplate.bind({});
Base.args = {
  children: 'Base Button',
  primary: true,
};

export default ButtonStory;

The Button component:

import { Theme } from '@emotion/react';
import styled from '@emotion/styled';

interface ButtonProps {
  theme?: Theme;
  primary?: boolean;
}

export const Button = styled.button(({ theme, primary }: ButtonProps) => {
...
})

Am I missing anything?

@Shawnxkwang
Copy link
Author

btw I'm using React 17

@shilman
Copy link
Member

shilman commented Nov 24, 2020

Thanks @Shawnxkwang that helps. I think this is a bug:

buttons: [ ...ButtonStories.Primary.args, ...ButtonStories.Secondary.args ],

args is an object but you're spreading it into an array. I think you want:

buttons: [ ButtonStories.Primary.args, ButtonStories.Secondary.args ],

Assuming each value in the buttons array should be passed into a Button as props.

@Shawnxkwang
Copy link
Author

Thanks @Shawnxkwang that helps. I think this is a bug:

buttons: [ ...ButtonStories.Primary.args, ...ButtonStories.Secondary.args ],

args is an object but you're spreading it into an array. I think you want:

buttons: [ ButtonStories.Primary.args, ButtonStories.Secondary.args ],

Assuming each value in the buttons array should be passed into a Button as props.

Sorry for the confusion. That part is the official doc that I followed, please see this reply for the real code I wrote.

@shilman shilman assigned jonniebigodes and unassigned shilman Nov 24, 2020
@shilman shilman modified the milestones: 6.1 args, 6.1.x Nov 24, 2020
@shilman shilman self-assigned this Nov 25, 2020
@shilman
Copy link
Member

shilman commented Nov 25, 2020

Here's what I'm thinking:

  • args requires its objects to be JSON-serializable (basically)
  • the theme object contains cycles
  • the code that crashes should check for cycles and provide a better error message

I'll add the cycle check in 6.2.

Separately, the documentation with the spread args looks wrong and probably needs to be updated @jonniebigodes

@shilman
Copy link
Member

shilman commented Nov 26, 2020

Yippee!! I just released https://github.com/storybookjs/storybook/releases/tag/v6.1.7 containing PR #13263 that references this issue. Upgrade today to the @latest NPM tag to try it out!

npx sb upgrade

Closing this issue. Please re-open if you think there's still more to do.

@jonniebigodes
Copy link
Contributor

jonniebigodes commented Dec 24, 2020

@shilman looking at this as is indeed is incorrect. I believe that the correct code should be:

buttons: [
    { ...ButtonStories.Primary.args },
    { ...ButtonStories.Secondary.args }
  ],
  orientation: "horizontal"

With this we can preserve the args incoming from the Button.stories that are imported in this case.

I'm going to update the snippet and submit a pull request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants