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

addon-docs: No props for some components #9592

Closed
thany opened this issue Jan 22, 2020 · 26 comments
Closed

addon-docs: No props for some components #9592

thany opened this issue Jan 22, 2020 · 26 comments

Comments

@thany
Copy link

thany commented Jan 22, 2020

Describe the bug
Some components storybook cannot find/display props for.
I've scraped a couple of issues concerning this, but found no solution. It seems I've found another situation yet, where storybook is unable to show props.

To Reproduce
Steps to reproduce the behavior:

  1. Make a component
  2. Make it complex or something maybe?
  3. Hope that it doesn't show props
  4. ...?

Sorry, I really have no idea what's "wrong" with my component to make it not show props. Otherwise I would've fixed, wouldn't I? So I also don't yet know what you need to do exactly, to reproduce it.

It works absolutely fine in intellisense, and typescript doesn't complain about implicit any types (which I disallow).

Expected behavior
Should just show the props I've built.

Code snippets
In my doc, I'm just doing this:

<Props of={FormInputUpload} />

System:

  System:
    OS: Windows 10 10.0.18363
    CPU: (12) x64 Intel(R) Core(TM) i9-8950HK CPU @ 2.90GHz
  Binaries:
    Node: 10.16.3 - C:\Program Files\nodejs\node.EXE
    Yarn: 1.16.0 - C:\Program Files (x86)\Yarn\bin\yarn.CMD
    npm: 6.9.0 - C:\Program Files\nodejs\npm.CMD
  Browsers:
    Edge: 44.18362.449.0

(Browsers is wrong, I'm actually on Firefox 72)

I'm on Storybook 5.3.8, and all plugins are, too.
I've just upgraded from 5.2.4 which also had this exact problem. It's almost like the upgrade didn't go through, but it did, because SB has decided to go darkmode, which is probably a new thing. I'm also on Typescript 3.6.4.

Additional context
I've tried these possible problems:

@eriktoyra
Copy link

@thany: I would try the following:

  1. Move the props interface to the same file as FormInputUpload. If that doesn't work, you may rule out No props appearing for named function export with Addon-docs #9205.
  2. Make sure that you haven't set a displayName for your component. Using a displayName will cause the problem I have in Description and Props slots not shown when using TS and Component.displayName #9493.
  3. If possible, post the code from FormFieldUpload in this issue and I will try to help you from there.

@shilman
Copy link
Member

shilman commented Jan 23, 2020

@thany sorry for the hassle on this. turns out that reliably displaying props across 1000 different configurations and language features is not an easy thing. fortunately react-docgen@5 supports typescript, so we can halve that number in 6.0 (and clean up/document most of these cases). thanks for your patience!

@thany
Copy link
Author

thany commented Jan 23, 2020

@eriktoyra I have tried those things.

  1. The interface in the same file or not, makes no difference. It's actually kind of weird if it would, I feel. I would think the docgen for typescript "just" takes whatever typescript also provides to intellisense, but I'm sure it's more complicated than that.
  2. We don't use displayName. They get generated pretty well, usually 😎
  3. Due to intellectual property restrictions, I can't post most of it. I fear if I'd reduce it to something I could post, it would start working, because I would have to remove a LOT of supporting code around it. I'm sorry I can't be too helpful there.

@shilman Not a problem 👍🏼
Soon(-ish)™ we'll start working on upgrading a lot of other stuff, not just Storybook, and then we'll pick on your suggestion, too. Hopefully that'll help. Since we're on 5.2.4 still (I did the upgrade to 5.3.8 locally just to test it out) we've got our work cut out. We'll probably delete addon-info as well, which might interfere in some way maybe? It's deprecated (right?) for a reason, I'd think.

@shilman
Copy link
Member

shilman commented Jan 23, 2020

@thany actually addon-info is not technically deprecated yet. but it will be soon, and definitely an evolutionary dead end at this point, so i definitely recommend migrating off at your earliest convenience!

@garand
Copy link

garand commented Jan 29, 2020

I'm having this issue as well. In my component I'm returning some JSX stored in an object based on a prop. If I wrap the return value in JSX it works however.

// Doesn't show props
return svg[variant]

// Shows props
return <>{svg[variant]}</>

@thany
Copy link
Author

thany commented Jan 31, 2020

In my case, I'm not returning anything like that. It's a React.FC<Props> and just simply returns some react elements. It might be worth mentioning I'm also using styled-components, but for almost every other component that's not a problem, and I don't think I'm doing anything different from those.

So @garand maybe open a separate issue because your situation is kind of different from what I think most people do in their components.

@taze-fullstack
Copy link

taze-fullstack commented Feb 13, 2020

@thany I'm experiencing this with styled-components as well! [My addon-docs is at 5.3.13]

Works:

const Hello = ({title}: HelloProps) => {
  return <div className="hello">Hello Component {title}</div>;
};

DOESN'T Work: (no props on the table)

import styled from 'styled-components';

const StyledHello = styled.div`
  color: red;
`;

const Hello = ({title}: HelloProps) => {
  return <StyledHello className="hello">Hello Component {title}</StyledHello>;
};

Update: It seems that just importing styled from styled-components breaks it!?!?!? What's more baffling is that after importing -> breaking, commenting the import does not fix it back!???!!

@garand
Copy link

garand commented Feb 13, 2020

@taze-fullstack What about if you wrap the <StyledHello ...></StyledHello> with a React Fragment?

@taze-fullstack
Copy link

@garand I've removed my whole node_modules folder and added a .yarnclean that contains @types/react-native in an attempt to fix another issue over at DefinitelyTyped/#33311 re-ran install, and it's unbelievably working now.

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

shilman commented Feb 14, 2020

Repro for @taze-fullstack 's case. c8c19a2

@sidhuko
Copy link

sidhuko commented Feb 20, 2020

@shilman I found another interesting resolution and point of investigation for you.

I had three components:

  • Exported Styled Component; works fine out of the box.
  • Search React.FC component; didn't work at all hence me trawling around here
  • Select React.FC component; which worked fine too so not a setup issue.

After a short while I just sat back and compared the two React.FC components what I noticed is that the search which I had bought over from ES used:

import React from 'react';

Whereas the Select I had rewritten was using:

import * as React from 'react';

When I used the import * syntax it all worked! Phew...

@shilman
Copy link
Member

shilman commented Feb 21, 2020

@sidhuko thanks for this! FYI we're going to switch form react-docgen-typescript-loader to babel-plugin-react-docgen in 6.0, which uses react-docgen directly (more maintainers, fewer moving parts, doesn't have as many edge cases like this

@thany
Copy link
Author

thany commented Feb 28, 2020

Importing react like that seems to fix some situations for me. Not all.

Here are a few more situation where it magically doesn't work.

Union interfaces

No props found for this component:

const Modal: React.FC<ISpacingComponent<IModal>>

If I change this to:

const Modal: React.FC<IModal>

Then the props from IModal are displayed. ISpacingComponent is defined like so:

export interface IPaddings {
  // Just some regular props here
}
export interface IMargins {
  // Just some regular props here
}
type ISpacer = IPaddings & IMargins & {
  /**
   * Determines which element to render as.
   */
  as?: keyof JSX.IntrinsicElements;
};
export type ISpacingComponent<Props> = Props & Partial<ISpacer>;

So there's nothing too special about it. Typescript loves this, and intellisense works beautifully. I would imagine the typescript webpack loader thingy (I follow the documentation to the letter on this!) uses actual typescript, not its own little broken parser?

forwardRef components

const FormInputCheckbox = React.forwardRef<Ref, React.PropsWithChildren<IFormInputCheckRadio>>

This produces a different error:

_a is undefined

Which is displayed in place of the "No props found for this component" message.
I absolutely need my component to be written like that. And again, intellisense works fine, so it's not like typescript would choke on it.

Interface with extends

const FormInputCheckboxList: React.FC<IFormInputCheckboxList>

If I skim too quickly over the generated props table, it appears to work fine. But it's omitting a LOT of props actually. That might be because the webpack typescripter loader thingy chokes on this:

interface IFormInputCheckboxList extends Omit<IFormInputCheckRadioList, 'onChange' | 'value'> {
  value?: Array<string>;
  onChange: (values: Array<string>) => void;
}

Only value and onChange are listed (the correct ones at least, not the ones being omitted using the Omit type - that'd be even more hilarious). Again, intellisense works fine (this is getting repetative 😄).

Export order

It seems that the typescript loader thingy naively assumes the last export in a module is the component we're after. Even if that export is a totally different one than the one being imported in the MDX doc:

export default FormInputSelect;
export { OuterWrapper as Base };

Here, it blindly assumes OuterWrapper is the component exported from this module. And as a result, the props table lists every HTML attribute (because it's a styled-component over a div), instead of the props for FormInputSelect. Something like this is just frustrating, to be honest. This kind of problem really doesn't need to exist.
To prove that this is a real, actual, genuine, verifyable bug, the MDX imports:

import FormInputSelect from './controls/FormInputSelect';

Which is the DEFAULT import, clearly. So taking the last import what happens to be listed in a module, is just plain wrong.

@thany
Copy link
Author

thany commented Feb 28, 2020

As for the "Union interfaces" situation, it seems to purely be the fact that I'm pulling intefaces from another module. Again, why does that make any difference at all? Who cares where my interfaces live? Should I repeat them in each and every component where I use them? Surely that's not the most productive solution...

Just throwing this into the mix, not sure if it adds anything valuable to diagnosing the problem 😀

@danielbayerlein
Copy link

Is there already a solution for pure styled-components components?

import styled from 'styled-components'
import PropTypes from 'prop-types'

const Card = styled.div`
  background-color: ${props => props.theme.palette.primary.white};
  box-shadow: 0 2px 3px 0 ${props => props.theme.palette.shadowGray};
  width: 100%;
`

Card.propTypes = {
  children: PropTypes.node.isRequired
}

export default Card

@stale
Copy link

stale bot commented Apr 2, 2020

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 Apr 2, 2020
@EduardAkhmetshin
Copy link

EduardAkhmetshin commented Apr 16, 2020

@danielbayerlein I have the same problem. import-statement at the beginning of the file didn't help.

@stale stale bot removed the inactive label Apr 16, 2020
@EduardAkhmetshin
Copy link

It seems like babel-plugin-react-docgen can't parse styled components and set __docgenInfo.

@SarahEW1206
Copy link

SarahEW1206 commented Apr 22, 2020

I'm not sure if this is the right place for this but I've also noticed that if a type is defined as:

/**
   * Button type
   */
  buttonType?: 'button' | 'reset' | 'submit' | undefined;

The docs props table will simply show the type of the type, i.e union. Like this:
Screen Shot 2020-04-22 at 10 39 05 AM

is there a way to make it display the the details and not just union, etc?!

@shilman
Copy link
Member

shilman commented Apr 22, 2020

@SarahEW1206 see #10311

@jonasantonelli
Copy link

Debugging a lot I identified that babel was somehow removing the __docgenInfo property.

So I made it work by installing this plugin: https://github.com/storybookjs/babel-plugin-react-docgen

@shilman
Copy link
Member

shilman commented May 1, 2020

@jonasantonelli that's strange. babel-plugin-react-docgen should be automatically applied in the default babel setup. are you using a custom babel config?

@thany
Copy link
Author

thany commented May 11, 2020

It feel to me like everyone is writing their own little typescript parser to get this kind of information from the sourcecode.

Why is it not possible to just to whatever VScode (or any other editor) does in order to get proper intellisense information? Certainly, code editors don't need these crappy unstable babel plugins to get intellisense information. They (or at least VScode) use the actual proper typescript engine, whichever version the editor or project comes with.

@jonasantonelli
Copy link

@jonasantonelli that's strange. babel-plugin-react-docgen should be automatically applied in the default babel setup. are you using a custom babel config?

Oww I understand now
I'm using a custom configuration for Storybook, then I needed to import it to solve. Thanks for explanation

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

stale bot commented Jun 15, 2020

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 Jun 15, 2020
@stale
Copy link

stale bot commented Jul 18, 2020

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!

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

10 participants