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

React + Storybook - Props are missing / types of props are missing in docs when running with Nx #7544

Closed
danr-za opened this issue Oct 27, 2021 · 23 comments
Assignees
Labels
outdated scope: storybook Issues related to Storybook support in Nx type: bug

Comments

@danr-za
Copy link
Contributor

danr-za commented Oct 27, 2021

Current Behavior

  • Running nx run lib:storybook - Not all component props are displayed in Docs, both with CSF and MDX
  • Running start-storybook - Works as expected

Expected Behavior

Should be as same as Storybook's result.

Steps to Reproduce

  • Clone the following repo
  • Run nx run ui:storybook - Go to Button docs and see that not all props are displayed, and the ones that are displayed, don't have types.
  • Run cd libs/ui && start-storybook - Go to Button docs and see all props are displayed with types as expected.

Environment

  Node : 14.16.1
  OS   : darwin x64
  npm  : 7.19.0
  
  nx : 13.1.2
  @nrwl/angular : Not Found
  @nrwl/cli : 13.1.2
  @nrwl/cypress : 13.1.2
  @nrwl/devkit : 13.1.2
  @nrwl/eslint-plugin-nx : 13.1.2
  @nrwl/express : Not Found
  @nrwl/jest : 13.1.2
  @nrwl/linter : 13.1.2
  @nrwl/nest : Not Found
  @nrwl/next : Not Found
  @nrwl/node : Not Found
  @nrwl/nx-cloud : Not Found
  @nrwl/react : 13.1.2
  @nrwl/schematics : Not Found
  @nrwl/tao : 13.1.2
  @nrwl/web : 13.1.2
  @nrwl/workspace : 13.1.2
  @nrwl/storybook : 13.1.2
  @nrwl/gatsby : Not Found
  typescript : 4.3.5
@mandarini mandarini added the scope: storybook Issues related to Storybook support in Nx label Oct 28, 2021
@mandarini mandarini self-assigned this Nov 22, 2021
@mandarini
Copy link
Member

Hi there @danr-za ! Thanks for filing an issue and also for providing a reproduction repo! This is what I see when I run

nx run ui:storybook 

what would the expected behavior be?

Screenshot 2021-11-22 at 4 58 30 PM

@danr-za
Copy link
Contributor Author

danr-za commented Nov 22, 2021

Hey @mandarini, thanks for taking a look.
Try running it with cd libs/ui && start-storybook

This is the result you'll see, which differs from the one you run with nx (all button props with their types are displayed):
image

@danr-za
Copy link
Contributor Author

danr-za commented Jan 11, 2022

@mandarini any news regarding this one?

@mandarini
Copy link
Member

Hi @danr-za ! Will get into it soon, sorry about that!!

@mandarini
Copy link
Member

Hi there @danr-za ! First of all, sorry for taking this long to get to this. Since it was not a blocking issue, and other blocking issues came up, I left this behind :( Thanks for understanding.

So, of course, in order to fix this and see all the props, you can add the missing props manually on the story, and they will show up.
eg.:

Basic.args = {
  disabled: false,
  children: 'Hello',
  primary: false,
  onClick: () => {},
  className: '',
};

However, you are totally correct, it's weird that the pure Storybook server assumes the props correctly from the component, and our wrapper does not. So it's an omission, and we'll find a way to fix this.

Another question (not related to above bug) would be: do you believe that it would make sense for Nx to read all props and add all of them in the generated story automatically?

@danr-za
Copy link
Contributor Author

danr-za commented Feb 4, 2022

Hey @mandarini
Thanks for getting back to this.
Indeed, it seems that something is missing during the type reading or it might be that Storybook uses different tools for it because you can see it's not only the props that are missing, their type is not correct too. For example, you can add onClick in args, and it would show it in the props table as any or unknown while "pure" Storybook will show it as an event handler from React types.

Regarding your question - totally. In our repo, I have created a generator that creates a component together with a story with all the "common" props already populated in args. 😄

@mandarini
Copy link
Member

@danr-za Yes yes, I see what you're saying. We'll get to it, eventually, I promise, hehe! :)

Ha, it's nice you created your own generator to get around this, ha! :) Let me know if you'd like to help enhancing the Nx code to read the missing props, btw. We're open to contributions!

@pataroka
Copy link

Hi, barging in here to give the issue a bump and confirm that the build executor has the same behaviour.

@mandarini
Copy link
Member

Hi there @pataroka ! Yes, we are aware. I'm looking into it to understand why. You can try to debug as well, if you want to help... Something in the options that are passed gets written over or something.

@pataroka
Copy link

pataroka commented Apr 1, 2022

@mandarini I compared the webpack config of both builds and it seems that the nx is missing a plugin. Cant't tell which one as converting this to JSON removes the plugin names. Don't know if it's relevant, but it's worth a try. Attached are the two config exports.
webpack-config-comparison.zip

@mandarini
Copy link
Member

Thank you!!!!

@mandarini
Copy link
Member

Hi @pataroka @danr-za !!

So, something magic happened, and it now works. Can you please try with latest version of Nx?

React app with Storybook, react component has props, no args are declared in story, controls show up!

@mandarini
Copy link
Member

You can clone this repository and run nx storybook re-nx-other and see the Button showing the props even without args specified.

@pataroka
Copy link

@mandarini Latest 13 or latest 14 version?

@mandarini
Copy link
Member

Oh! Latest 14!

@pataroka
Copy link

pataroka commented Jun 3, 2022

@mandarini This still does not work for me. First, after migration, without updating the storybook dependencies to ^6.5.6 the sotorybook dev server fails to build
Screenshot 2022-06-03 at 9 32 58
Then, after manually updating the storybook dependencies, it runs, but inherited props are still not inferred.

import React from 'react';
import classnames from 'classnames';
import { makeStyles } from '@material-ui/core';
import Chip, { ChipProps } from '@material-ui/core/Chip';

import styles from './Tag.module.scss';

const useStyles = makeStyles(() => {
  return {
    background: {
      backgroundColor: (props: ITagColorProps) => props.backgroundColor,
    },
    text: {
      color: (props: ITagColorProps) => props.textColor,
    },
  };
});

export interface ITagColorProps {
  backgroundColor?: string;
  textColor?: string;
}

export interface ITagProps extends ChipProps, ITagColorProps {
  showTooltip?: boolean;
}

export const Tag: React.FC<ITagProps> = (props) => {
  const { backgroundColor, textColor, className, label, showTooltip, ...rest } = props;
  const classes = useStyles({ textColor, backgroundColor });

  return (
    <Chip
      data-test='tag'
      color='primary'
      size='small'
      {...rest}
      label={showTooltip ? <span title={label as string}>{label}</span> : label}
      className={classnames('rnd-tag', styles.tag, classes.text, classes.background, className)}
    />
  );
};

Screenshot 2022-06-03 at 9 36 49

When starting with storybook directly:
Screenshot 2022-06-03 at 9 46 28

@mandarini
Copy link
Member

Hmm very interesting. I will look into it. In the meantime, can you share a repository which I can clone and play around? It would be super helpful. Also, can you run nx report and post here the result? Thank you so much for this @pataroka !

@mandarini
Copy link
Member

mandarini commented Jun 3, 2022

Hi there @pataroka ! If you remove the first line of your component, import React from 'react';, it will work! :)

Can you try this out and let me know? I just tried it locally and it seems to work. I don't know why yet, I will investigate eventually. Until then, you can remove that import and the magic will happen. Here's my repository that shows it works. Here is your file, this repo has a bunch of other things I'm testing, too. That's with the latest version of Nx.

@pataroka
Copy link

pataroka commented Jun 3, 2022

Hey, @mandarini, thanks for looking at this so quickly. Here's the report:

   Node : 14.19.1
   OS   : darwin x64
   yarn : 3.1.1
   
   nx : 14.1.9
   @nrwl/angular : Not Found
   @nrwl/cypress : 14.1.9
   @nrwl/detox : Not Found
   @nrwl/devkit : 14.1.9
   @nrwl/eslint-plugin-nx : 14.1.9
   @nrwl/express : Not Found
   @nrwl/jest : 14.1.9
   @nrwl/js : 14.1.9
   @nrwl/linter : 14.1.9
   @nrwl/nest : Not Found
   @nrwl/next : Not Found
   @nrwl/node : Not Found
   @nrwl/nx-cloud : Not Found
   @nrwl/nx-plugin : Not Found
   @nrwl/react : 14.1.9
   @nrwl/react-native : Not Found
   @nrwl/schematics : Not Found
   @nrwl/storybook : 14.1.9
   @nrwl/web : 14.1.9
   @nrwl/workspace : 14.1.9
   typescript : 4.4.4
   ---------------------------------------
   Community plugins:

Removing the import works, but I still need to push the storybook dependencies up to 6.5.6 - the 6.4.22 which comes with nx 14.1.9 fails to build.
I tried to create something similar to our setup, but I could not reproduce the issue in the demo repo. I guess it could be down to 3rd party dependency misalignment as we're using yarn workspaces to be able to keep asynchronous versioning of some of our libraries. This in turn, would make it quite hard to create a repo clone that mimics everything that's going on. For now I think it's safe to say that it is down to some path resolutions in our own setup, and I'm on my own to discover what and where.
Thanks again for the help!

@mandarini
Copy link
Member

mandarini commented Jun 3, 2022

In my repo, which I shared above, I still use Storybook 6.4.22. Are you sure it's still breaking for you? In any case, is the 6.5.6 upgrade causing any issues for you? Or are you pointing it out that we should add the upgrade to Nx?

In any case, the next version of Nx will install Storybook 6.5. It already works, as you verified yourself!!

Let me know if your issue remains unsolved!

@mandarini
Copy link
Member

Oh, and please look at the repo I posted, maybe it helps!

@pataroka
Copy link

pataroka commented Jun 3, 2022

I think our build problem is related to this issue storybookjs/storybook#15336
But if upgrading storybook fixes it and it works with the latest version of nx I'm fine :)

@github-actions
Copy link

This issue has been closed for more than 30 days. If this issue is still occuring, please open a new issue with more recent context.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
outdated scope: storybook Issues related to Storybook support in Nx type: bug
Projects
None yet
Development

No branches or pull requests

3 participants