storybookjs / storybook

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

React.forwardRef cause invalid prop in PropTable for the withInfo addon #3389

Closed DefoyPhilip closed 5 years ago

DefoyPhilip commented 6 years ago

Bug or support request summary

Using React.forwardRef cause the prop type of PropTable to be incorrect.

Warning: Failed prop type: Invalid prop `type` of type `object` supplied to `PropTable`, expected `function`.
    in PropTable
    in Unknown (created by Story)
    in div (created by Story)
    in div (created by Story)
    in div (created by Story)
    in div (created by Story)
    in div (created by Story)
    in ThemeProvider (created by Story)
    in Story
    in WrapStory

It causes bugs in the story source and proptypes table

Story without the forwardRef wrapper:

working-exemple

Story with the forwardRef wrapper: (Note the Unknown tag)

not-working-exemple

Steps to reproduce

Add the wrapper React.forwardRef to a working component that is used in a story React.forwardRef(({ text, onDelete, ...rest }, ref) => { ... }

My code example uses styled-components but normal DOM element reproduce the bug too.

Please specify which version of Storybook and optionally any affected addons that you're running

Affected platforms

Chrome V65.0.3325.181, windows 10 V1709 I'm pretty sure its not related but my version of styled-components is 3.2.5

Screenshots / Screencast / Code Snippets (Optional)

This is the working exemple code:

import React from 'react';
import PropTypes from 'prop-types';
import styled from 'styled-components';

const Container = styled.div`
    background-color: rgba(0, 0, 0, 0.15);
    color: rgba(0, 0, 0, 0.87);
    height: 36px;
    display: inline-block;
    padding: ${({ hasDelete }) => (hasDelete ? ' 0px 4px 0px 12px' : '0px 12px')};
    font-family: 'Roboto', sans-serif;
    border-radius: 36px;
    position: relative;
`;

const Text = styled.p`
    display: inline-block;
    height: 16px;
    margin: 10px 0px;
    font-size:14px;
    position: relative;
`;

const Chip = ({ text, onDelete, ...rest }) => {
    return (
        <Container hasDelete={onDelete !== undefined} {...rest} >
            <Text>{text}</Text>
        </Container>
    );
};

Chip.defaultProps = {
    onDelete: undefined,
};

Chip.propTypes = {
    text: PropTypes.string.isRequired,
    onDelete: PropTypes.func,
};

export default Chip;

This is the bug exemple code:

import React from 'react';
import PropTypes from 'prop-types';
import styled from 'styled-components';

const Container = styled.div`
    background-color: rgba(0, 0, 0, 0.15);
    color: rgba(0, 0, 0, 0.87);
    height: 36px;
    display: inline-block;
    padding: ${({ hasDelete }) => (hasDelete ? ' 0px 4px 0px 12px' : '0px 12px')};
    font-family: 'Roboto', sans-serif;
    border-radius: 36px;
    position: relative;
`;

const Text = styled.p`
    display: inline-block;
    height: 16px;
    margin: 10px 0px;
    font-size:14px;
    position: relative;
`;

const Chip = React.forwardRef(({ text, onDelete, ...rest }, ref) => {
    return (
        <Container innerRef={ref} hasDelete={onDelete !== undefined} {...rest} >
            <Text>{text}</Text>
        </Container>
    );
});

Chip.defaultProps = {
    onDelete: undefined,
};

Chip.propTypes = {
    text: PropTypes.string.isRequired,
    onDelete: PropTypes.func,
};

export default Chip;

Story

import React from 'react';
import { storiesOf } from '@storybook/react';
import { withInfo } from '@storybook/addon-info';
import Chip from '../src/chip';

storiesOf('Chip', module)
    .add('Basic Usage', withInfo({
        inline: true,
        text: `
            <div>
                <h3>Description</h3>
                <p>This is the basic usage of the Chip.</p>
                <h3>Requirement</h3>
                <p>None</p>
                <h3>Props</h3>
                <ul>
                    <li>text: Text added in the chip</li>
                    <li>onDelete: Function that is called when the delete button is clicked</li>
                </ul>
            </div>
    `,
    })((() => (
        <div>
            <Chip
                text="This is a test"
                onDelete={() => { console.log('test'); }}
            />
            <Chip
                text="This can't be deleted"
                style={{ marginLeft: '20px' }}
            />
        </div>
    ))));
stale[bot] commented 6 years ago

Hi everyone! Seems like there hasn't been much going on in this issue lately. If there are still questions, comments, or bugs, please feel free to continue the discussion. Unfortunately, we don't have time to get to every issue. We are always open to contributions so please send us a pull request if you would like to help. Inactive issues will be closed after 30 days. Thanks!

danieltott commented 6 years ago

I had this same issue and solved it by using jest.mock to remove storybook-info from this process. This has the added benefit of not including the markup that info creates into the snapshots.

I added the following code to Storyshots.test.js:

jest.mock('@storybook/addon-info', () => ({
  withInfo: () => storyFn => storyFn,
  setDefaults: () => {},
}))

Hope this helps someone

stale[bot] commented 6 years ago

Hi everyone! Seems like there hasn't been much going on in this issue lately. If there are still questions, comments, or bugs, please feel free to continue the discussion. Unfortunately, we don't have time to get to every issue. We are always open to contributions so please send us a pull request if you would like to help. Inactive issues will be closed after 30 days. Thanks!

brennancheung commented 6 years ago

The new React context does not provide access to the context from lifecycle methods such as constructors and componentDidMount. In order to get around that we need to wrap it with something that takes the context and injects it into props. Unfortunately it breaks storybook with the error first mentioned in the issue.

Does anyone know how to work around this? I'm getting this error outside tests in a plain storybook story so I don't think the jest.mock solution is applicable.

Here's the pattern:

import { Context } from 'somewhere'
const withContext = Component => 
  React.forwardRef((props, ref) => (
    <Context.Consumer>
      {
        ({ contextValue1, contextValue2 }) =>
          <Component
            {...props}
            contextValue1={contextValue1}
            contextValue2={contextValue2}
            ref={ref}
          />
      }
    </Context.Consumer>
  ))
amacleay commented 6 years ago

The only workaround I've found is to turn off prop table generation:

withInfo({
  propTables: false
  text: 'Here it is',
})(...)

FWIW, I've pushed a demo repo demonstrating this failure with a basic React.Consumer example;

  .add(
    'with info with a consumer',
    withInfo('Is it Friday?')(() => (
      <IsFridayContext.Consumer>
        {isFriday =>
          isFriday ? (
            <a href="https://www.youtube.com/watch?v=kfVsfOSbJY0">It's friday</a>
          ) : (
            "It's a miserable day"
          )
        }
      </IsFridayContext.Consumer>
    ))
  );
stale[bot] commented 6 years ago

Hi everyone! Seems like there hasn't been much going on in this issue lately. If there are still questions, comments, or bugs, please feel free to continue the discussion. Unfortunately, we don't have time to get to every issue. We are always open to contributions so please send us a pull request if you would like to help. Inactive issues will be closed after 30 days. Thanks!

stale[bot] commented 6 years ago

Hey there, it's me again! I am going close this issue to help our maintainers focus on the current development roadmap instead. If the issue mentioned is still a concern, please open a new ticket and mention this old one. Cheers and thanks for using Storybook!

nickserv commented 5 years ago

This is still broken, please reopen.

ferrybrouwer commented 5 years ago

+1 ⇢ Even with the new major release @storybook/addon-info 4.0.2 it's still broken. Please reopen.

DarkPurple141 commented 5 years ago

Still broken.

catamphetamine commented 5 years ago

The bots are broken

Sawtaytoes commented 5 years ago

Still receiving the error in 4.1.4 for me.

camflan commented 5 years ago

same here, latest storybook and addons

snakeUni commented 5 years ago

Still broken.

romans12 commented 5 years ago

This is still broken in 4.1.11. Can we reopen this?

guiyep commented 5 years ago

this is still broken in 4.1.11

stale[bot] commented 5 years ago

Hi everyone! Seems like there hasn't been much going on in this issue lately. If there are still questions, comments, or bugs, please feel free to continue the discussion. Unfortunately, we don't have time to get to every issue. We are always open to contributions so please send us a pull request if you would like to help. Inactive issues will be closed after 30 days. Thanks!

dgana commented 5 years ago
// Function Component
Component.diplayName = "Component Name"

// Class Component
static displayName = "Component Name"
guiyep commented 5 years ago

This is still broken in latest version. Any idea? I’m just ignoring it...

dgana commented 5 years ago

Its working fine with me try check dependencies below

"dependencies": {
    "chroma-js": "1.3.7",
    "classnames": "2.2.6",
    "leaflet": "^1.4.0",
    "leaflet-routing-machine": "^3.2.12",
    "leaflet.markercluster": "^1.4.1",
    "prop-types": "15.6.2",
    "react": "^16.8.1",
    "react-dom": "^16.8.1",
    "react-leaflet": "^2.2.0",
    "react-leaflet-markercluster": "^2.0.0-rc3",
    "react-leaflet-search": "^0.6.0",
    "react-pose": "3.2.1",
    "react-select": "2.0.0",
    "react-table": "6.8.6",
    "recharts": "1.1.0",
    "superagent": "3.8.3",
    "tachyons": "4.11.1",
    "tachyons-cli": "1.3.2"
  },
"devDependencies": {
    "@babel/core": "^7.3.3",
    "@babel/plugin-proposal-class-properties": "^7.3.3",
    "@babel/preset-env": "^7.3.1",
    "@babel/preset-react": "^7.0.0",
    "@dump247/storybook-state": "^1.5.2",
    "@sambego/storybook-state": "^1.3.2",
    "@storybook/addon-a11y": "^4.1.11",
    "@storybook/addon-actions": "^4.1.11",
    "@storybook/addon-centered": "^4.1.11",
    "@storybook/addon-google-analytics": "^4.1.11",
    "@storybook/addon-info": "^4.1.12",
    "@storybook/addon-knobs": "^4.1.11",
    "@storybook/addon-links": "^4.1.11",
    "@storybook/addon-notes": "^4.1.11",
    "@storybook/addon-options": "^4.1.11",
    "@storybook/addons": "^4.1.11",
    "@storybook/react": "^4.1.11",
    "autoprefixer": "^9.4.7",
    "babel-loader": "^8.0.5",
    "babel-polyfill": "^6.26.0",
    "babelrc-rollup": "^3.0.0",
    "css-mqpacker": "^7.0.0",
    "enzyme": "^3.9.0",
    "enzyme-adapter-react-16": "^1.9.1",
    "enzyme-to-json": "^3.3.5",
    "jest": "^24.1.0",
    "postcss-calc": "^7.0.1",
    "postcss-class-repeat": "^0.1.1",
    "postcss-conditionals": "^2.1.0",
    "postcss-css-variables": "^0.11.0",
    "postcss-custom-media": "^7.0.7",
    "postcss-discard-comments": "^4.0.2",
    "postcss-extend-rule": "^2.0.0",
    "postcss-import": "^12.0.1",
    "postcss-modules": "^1.4.1",
    "rimraf": "^2.6.3",
    "rollup": "^1.2.3",
    "rollup-plugin-babel": "^4.3.2",
    "rollup-plugin-commonjs": "^9.2.1",
    "rollup-plugin-copy-assets-to": "^1.0.0",
    "rollup-plugin-node-resolve": "^4.0.1",
    "rollup-plugin-postcss": "^2.0.3",
    "storybook-host": "^5.0.3",
    "storybook-readme": "^4.0.5"
  }
stale[bot] commented 5 years ago

Hi everyone! Seems like there hasn't been much going on in this issue lately. If there are still questions, comments, or bugs, please feel free to continue the discussion. Unfortunately, we don't have time to get to every issue. We are always open to contributions so please send us a pull request if you would like to help. Inactive issues will be closed after 30 days. Thanks!

shilman commented 5 years ago

Solved in 4.1.x by https://github.com/storybooks/storybook/pull/4961

And maybe re-solved (?!) in 5.0.x by https://github.com/storybooks/storybook/pull/6222

stale[bot] commented 5 years ago

Hi everyone! Seems like there hasn't been much going on in this issue lately. If there are still questions, comments, or bugs, please feel free to continue the discussion. Unfortunately, we don't have time to get to every issue. We are always open to contributions so please send us a pull request if you would like to help. Inactive issues will be closed after 30 days. Thanks!

stale[bot] commented 5 years ago

Hi everyone! Seems like there hasn't been much going on in this issue lately. If there are still questions, comments, or bugs, please feel free to continue the discussion. Unfortunately, we don't have time to get to every issue. We are always open to contributions so please send us a pull request if you would like to help. Inactive issues will be closed after 30 days. Thanks!

damusnet commented 5 years ago

Looks like #6222 was included in storybook@5.1.0-alpha.19 but the stable version of 5.1.0 has yet to come out.

shilman commented 5 years ago

@damusnet we're in beta now and hope to do a full release in a few weeks

ndelangen commented 5 years ago

This should be fixed in 5.1

iwarner commented 4 years ago

I am still getting this issue in 5.2.8

shilman commented 4 years ago

Addon-info is being superceded by addon-docs, which fixes a bunch of bugs and is easier to maintain. Please give it a try! https://medium.com/storybookjs/storybook-docspage-e185bc3622bf

JohnGrisham commented 4 years ago

I had this same issue and solved it by using jest.mock to remove storybook-info from this process. This has the added benefit of not including the markup that info creates into the snapshots.

I added the following code to Storyshots.test.js:

jest.mock('@storybook/addon-info', () => ({
  withInfo: () => storyFn => storyFn,
  setDefaults: () => {},
}))

Hope this helps someone

@danieltott This is great but in my case I also received this console error when implementing.

Warning: Functions are not valid as a React child. This may happen if you return a Component instead of from render. Or maybe you meant to call this function rather than return it. in ThemeProvider (created by Theme) in Theme (at config.js:39)

To fix this I had to call the storyFn instead of just returning it. My complete example below.

jest.mock('@storybook/addon-info', () => ({
  withInfo: () => (storyFn: any) => storyFn(),
  setDefaults: () => {},
}));