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

Error in addon-docs prop table when using HOC #9023

Open
Zenoo opened this issue Dec 2, 2019 · 14 comments
Open

Error in addon-docs prop table when using HOC #9023

Zenoo opened this issue Dec 2, 2019 · 14 comments

Comments

@Zenoo
Copy link

Zenoo commented Dec 2, 2019

Describe the bug
When using HOCs, the PropTypes aren't correctly displayed in the addon-docs prop table.
Instead, the following error is displayed:

Cannot read property 'classes' of undefined

To Reproduce
Steps to reproduce the behavior:

  1. Create an HOC.
  2. Create a story for this component

Expected behavior
The prop table should be filled with the component's proptypes.

Screenshots
Error

Code snippets

class Alert extends React.Component { ... }
Alert.propTypes = {
  variant: PropTypes.string,
  dismissible: PropTypes.bool,
  icon: PropTypes.elementType,
  classes: PropTypes.object.isRequired
};
Alert.defaultProps = {
  variant: 'primary',
  dismissible: false,
};

export default withStyles(theme => ({
  alert: props => ({
    backgroundColor: theme.palette[props.variant].main
  }),
  message: {
    display: 'flex',
    alignItems: 'center',
  },
  icon: {
    marginRight: theme.spacing(2),
  }
}))(Alert);

System:

System:
    OS: Windows 10 10.0.18362
    CPU: (8) x64 Intel(R) Core(TM) i7-7700HQ CPU @ 2.80GHz
  Binaries:
    Node: 10.16.0 - C:\Program Files\nodejs\node.EXE      
    npm: 6.10.1 - C:\Program Files\nodejs\npm.CMD
  Browsers:
    Edge: 44.18362.449.0
@shilman
Copy link
Member

shilman commented Dec 2, 2019

Workaround: export the pure component & use that instead

@Zenoo
Copy link
Author

Zenoo commented Dec 2, 2019

It seems to work fine, thanks !
In my example, I only added the export for the base class (export class Alert extends React.Component { ... }) and imported like so in my stories:

import Alert, { Alert as AlertComponent } from '......';

@gforrest-bw
Copy link

Is there a plan to address this? I'm working on a component library that themes and re-exports components from Material UI, so I don't have control over all of the components I'm documenting.

@stale
Copy link

stale bot commented Dec 23, 2019

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 stale bot added the inactive label Dec 23, 2019
@shilman shilman added the todo label Dec 24, 2019
@stale stale bot removed the inactive label Dec 24, 2019
@shilman
Copy link
Member

shilman commented Dec 24, 2019

@gforrest-bw absolutely. hopefully we can address this in january as a high-priority bug.

shilman added a commit that referenced this issue Feb 14, 2020
@shilman
Copy link
Member

shilman commented Feb 14, 2020

Repro: e3ab757

@shilman shilman added the react label May 23, 2020
@gaurav5430
Copy link

For now I ended up using the same approach as suggested by @Zenoo

exporting both the wrapped component and the unwrapped component
and then using the unwrapped component only for the component property.

import Alert, { Alert as AlertComponent } from '......';
export default {
    component: AlertComponent
}

@gforrest-bw
Copy link

The HOC we use changes the prop contract, so this isn't the most effective workaround for us.

@gaurav5430
Copy link

The HOC we use changes the prop contract, so this isn't the most effective workaround for us.

what do you mean that the HOC changes the props contract ?
@gforrest-bw

@gforrest-bw
Copy link

We use TypeScript, so a higher-order component can modify the expected props of the final component by removing or adding props. This is all captured and evaluated by TypeScript. The end user of the library will only see the final, post-hoc (no pun intended) props.

@gaurav5430
Copy link

@gforrest-bw

umm, not sure if i understand,
so if the HOC adds/removes some props,
wouldn't these props still be documented as optional in whatever is the PropTypes equivalent in typescript, possibly the interface for the component props ?

@gforrest-bw
Copy link

function exampleHoc(Component) {
  return ({ foo }) => (
    <Component bar={foo} />
  );
}

Is a trivial (and not very useful) example, but in this case the HOC returns a component with props { foo }, but the wrapped component has { bar }. Documenting the wrapped component would give the wrong idea about the usage of the actual library export.

In real life, we tend to use HOCs that inject a few props into the inner component which aren't present at all on the final outer props, so the documentation would show props which don't 'exist,' in theory.

This isn't a showstopper, just noting that the remedy isn't perfect.

@gitnarqt
Copy link

Any update on this issue? It just doesn't make sense to add unnecessary exports to the code just to document it properly 🤷🏻‍♂️

@patriklarssson
Copy link

Any news on this issue? The extra export works for the moment, but it would be awesome to have it work without it to minimize bugs with faulty import on components with HOCs

@shilman shilman removed the todo label Jun 20, 2023
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

6 participants