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

Documentation: Dealing with HOCs #12576

Closed
shilman opened this issue Sep 25, 2020 · 13 comments
Closed

Documentation: Dealing with HOCs #12576

shilman opened this issue Sep 25, 2020 · 13 comments

Comments

@shilman
Copy link
Member

shilman commented Sep 25, 2020

Originally posted by @thclark in #4143 (comment)

[complaints redacted]

I've worked around the two aspects of this:

  • documentation not getting generated
  • code examples generated for the wrapped component

The workaround requires that the unwrapped component must also be exported, so I've adopted a pattern of exporting the unwrapped component as a named export, and the wrapped as the default component.

Here's a complete working example (for a custom MUI component), for which the use of the react-controllables HOC is causing this issue.

TwoWaySwitch.jsx

import React from 'react'
import PropTypes from 'prop-types'
import FormGroup from '@material-ui/core/FormGroup'
import Switch from '@material-ui/core/Switch'
import Grid from '@material-ui/core/Grid'
import Typography from '@material-ui/core/Typography'
import controllable from 'react-controllables'


/**
 * A two-way switch to swap between two valid options
 *
 * Essentially a boolean switch like a checkbox (where 'checked'
 * places it in hte right hand position) but with
 * better UX where both options are valid.
 *
 * Can be used as a controlled or uncontrolled component. To use
 * in a controlled mode, pass both `checked` bool and `onCheckedChange`
 * function.
 *
 */
function TwoWaySwitch({
  leftLabel, rightLabel, checked, onCheckedChange,
}) {
  const handleChange = (event) => {
    if (onCheckedChange) {
      onCheckedChange(event.target.checked)
    }
  }

  const color = checked ? 'primary' : 'secondary'

  return (
    <FormGroup>
      <Typography component="div">
        <Grid component="label" container alignItems="center" spacing={1}>
          {leftLabel ? <Grid item>{leftLabel}</Grid> : null}
          <Grid item>
            <Switch
              checked={checked}
              onChange={handleChange}
              value="checked"
              color={color}
            />
          </Grid>
          {rightLabel ? <Grid item>{rightLabel}</Grid> : null}
        </Grid>
      </Typography>
    </FormGroup>
  )
}


TwoWaySwitch.defaultProps = {
  leftLabel: '',
  rightLabel: '',
}

TwoWaySwitch.propTypes = {
  /**
   * Label positioned to the left. Can be text or node.
   */
  leftLabel: PropTypes.node,
  /**
   * Label positioned to the right. Can be text or node.
   */
  rightLabel: PropTypes.node,
  /**
   * Control state of switch, `checked=True` places it
   * in the right hand position.
   */
  // eslint-disable-next-line react/require-default-props
  checked: PropTypes.bool,
  /**
   * Provide a callback function when the switch changes.
   * Function will receive the new `checked` value.
   */
  // eslint-disable-next-line react/require-default-props
  onCheckedChange: PropTypes.func,
}

// Working around https://github.com/storybookjs/storybook/issues/4143
export { TwoWaySwitch }
const exportComponent = controllable(TwoWaySwitch, ['checked'])
exportComponent.displayName = 'TwoWaySwitch'
export default exportComponent

TwoWaySwitch.stories.js

As per your normal story file, except:

// NOTE - SEE I'M IMPORTING THE UNWRAPPED COMPONENT
// If you want to use is as if it were wrapped, you'll need to wrap it yourself here...
import { TwoWaySwitch } from './TwoWaySwitch'

End result

This produces documentation as expected:
Screenshot 2020-09-22 at 10 46 38

@wztech0192
Copy link

wztech0192 commented Dec 6, 2020

Another way to fix the snippet without messing with our source code is to transform the snippet string using parameters/transformSource.

For example

parameters:{
 transformSource: src=>{
  src = src.replace(/WithStyles\(ForwardRef\((.*)\){2}/g, "$1");
  return src;
 }
}

this turn MUI <WithStyles(ForwardRef(Typography))/> into <Typography/>

@thclark
Copy link

thclark commented Dec 6, 2020

Very elegant, keeping it out of the code. I'll start using that. Thanks @wztech0192 :)

@wztech0192
Copy link

Very elegant, keeping it out of the code. I'll start using that. Thanks @wztech0192 :)

np, I really hope they can post something that describe all the properties of parameters somewhere. Right now I have to trace through their code to know that property exist...

@wztech0192
Copy link

wztech0192 commented Dec 8, 2020

The transformSource solution breaks in production build due to uglify/minify... However, after some more headache I came up with a more robust fix by using a resolveDisplayName function.

const resolveDisplayName = comps =>
 Object.entries(comps).forEach(([key, entry])=>{
  entiry.displayName = key;
 })

for each stories

//index.stories.jsx
import {Typography, Paper} from '@material-ui/core'
import Foo from '..'
resolveDisplayName({Typography,  Paper, RenamedFoo: Foo})

or globally

//preview.js
import * as muiComps from '@material-ui/core`
//this will map the key of components as displayName regard their order/level.
resolveDisplayName(muiComps)

@nitesh3539
Copy link

Storybook did not show the argsTable for my HOC propTypes

is there any other approach to do this apart from above approach?

Marker.jsx -->

/**
 * A decorative element which can be used to highlight the most important information in a card.
 * It can be used by attaching itself on the any side of the card or in the middle of the card.
 */
export const Marker = withScales(styled(Base)`
  border-radius: ${borderRadiusStyles};
  background: ${appearanceStyles(markerAppearances)};
`);
Marker.defaultProps = {
  componentName: 'Marker',
};
Marker.propTypes = {
  direction: PropTypes.oneOf(directions).isRequired,
  corner: PropTypes.oneOf(corners).isRequired,
  appearance: PropTypes.oneOf(appearances).isRequired,
  height: PropTypes.string.isRequired,
  width: PropTypes.string.isRequired,
};

Marker.stories.js --->

export default {
  title: 'Components/Atoms/Marker',
  component: Marker,
};

System:
OS: macOS 11.2.3
Binaries:
Node: 12.7.0
npm: 6.10.0
Browsers:
Chrome: 77.0.3865.120
npmPackages:
"@storybook/addon-links": "^6.0.22",
"@storybook/addon-storysource": "^6.0.22",
"@storybook/addons": "^6.0.22",
"@storybook/react": "^6.0.22",
077FD762-9F25-49B4-A708-47F15FF00987_1_201_a

@ghost
Copy link

ghost commented Jun 10, 2021

@wztech0192 Where does this params object need to go? Have tried adding it to .storybook/main under addons/addon-storysource/options/parameters

Seems to be the wrong spot.

@wztech0192
Copy link

@wztech0192 Where does this params object need to go? Have tried adding it to .storybook/main under addons/addon-storysource/options/parameters

Seems to be the wrong spot.

Under .storybook/preview.js, we will do export const parameters =.... to set global parameters

@GreLI
Copy link

GreLI commented Oct 27, 2021

I have issue with forwardRef also. None of the recipes worked. This declaration doesn't work:

export default forwardRef(Component)

However, I've discovered that this one works:

const Component = forwardRef((props: Props, ref: Ref) => /* implementation */)

export default Component

(Setting displayName didn't help either.)

@CiprianaM
Copy link

CiprianaM commented Dec 20, 2021

In case you are wrapping your component with some properties (say, styles), you will need to import in your story the default component also.

For example, in my component Avatar.jsx:

export { Avatar };
const exportComponent = withStyles(styles, { withTheme: true })(Avatar);
exportComponent.displayName = 'Avatar';
export default exportComponent;

In Avatar.stories.jsx:

import React from 'react';

import Default, { Avatar } from '../Avatar/Avatar';

export default {
  component: Avatar,
  title: 'Demos/Avatar',
};

export const TestStory1 = args => (
  <Default
    {...args}
  />
);

TestStory1.story = {
  name: 'with documentation',
};

I have imported both the wrapped component (Default) and the original component (Avatar), because:

  • Avatar is the one holding all the props, except the styles
  • Default is the one which will actually render my story, where I need to have access to all the styles and classes.

Result:

Screenshot 2021-12-20 at 16 27 57

@jonniebigodes
Copy link
Contributor

Closing this for now as this requires a bit more investigation on the integration. Later on, the documentation will be updated for CSS libraries and once it's worked on this issue will be factored in as well.

Hope you all have a great week and thank you for raising the issue and your input.

Stay safe

@JuhG
Copy link

JuhG commented Oct 21, 2022

The interesting thing is that Styleguidist is able to figure out the props with a HOC. For the exact same component, where the component is not even exported, just a single default export with the wrapped component. And they use the same react-docgen-typescript package. It doesn't work by default, only if we pass the our tsconfig as withCustomConfig:

module.exports = {
  propsParser: (filePath) => {
    return require("react-docgen-typescript")
      .withCustomConfig("./tsconfig.json", {
        shouldExtractLiteralValuesFromEnum: true,
      })
      .parse(filePath);
  },
// ... other config
}

// styleguide.config.js

Here's the docs for that: https://react-styleguidist.js.org/docs/configuration/#propsparser

Tried to do the same directly with react-docgen-typescript, passing the file path and it's working fine. So it should be possible.

@shilman Is there any Storybook limitation why it's not working? Is there any way I can help with this issue?

Here's a quick codesandbox with a HOC wrapper and no named export and react-docgen-typescript is still able to figure the props out. In this example tsconfig isn't even specified. (run start script)

Copy link
Member Author

shilman commented Oct 24, 2022

@JuhG responded in the other issue 👍

@tada-s
Copy link

tada-s commented Feb 16, 2023

This is another workaround to avoid exporting twice:

Original component code (where withDiv is the HOC):

export const MyComponent = withDiv(({ ...props }) => {
  // ...
  return <>...</>;
});

Workaround:

export const MyComponent = (props) => <>{_MyComponent(props)}</>;
const _MyComponent = withDiv(({ ...props }) => {
  // ...
  return <>...</>;
});

With this, I can see properly my component documentation and props documentation on Storybook 7.

Complete minimal working example with docs:

import React from "react";
import PropTypes from "prop-types";

/* HOC */
function withDiv(Component) {
  return (props) => (
    <div>
      <Component {...props} />
    </div>
  );
}

/**
 * MyComponent Docstring
 */
export const MyComponent = (props) => <>{_MyComponent(props)}</>;
const _MyComponent = withDiv(({ ...props }) => {
  // ...
  return <>...</>;
});
MyComponent.propTypes = {
  /**
   * Docstring for propA
   */
  propA: PropTypes.string,
  /**
   * Docstring for propB
   */
  propB: PropTypes.number,
};

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

9 participants