nrwl / nx

Smart Monorepos · Fast CI
https://nx.dev
MIT License
23.62k stars 2.36k forks source link

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

Closed ghost closed 2 years ago

ghost commented 3 years ago

Current Behavior

Expected Behavior

Should be as same as Storybook's result.

Steps to Reproduce

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 commented 2 years ago

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

ghost commented 2 years ago

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

ghost commented 2 years ago

@mandarini any news regarding this one?

mandarini commented 2 years ago

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

mandarini commented 2 years ago

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?

ghost commented 2 years ago

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 commented 2 years ago

@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 commented 2 years ago

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

mandarini commented 2 years ago

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 commented 2 years ago

@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 commented 2 years ago

Thank you!!!!

mandarini commented 2 years ago

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 commented 2 years ago

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

pataroka commented 2 years ago

@mandarini Latest 13 or latest 14 version?

mandarini commented 2 years ago

Oh! Latest 14!

pataroka commented 2 years ago

@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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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

pataroka commented 2 years ago

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

github-actions[bot] commented 1 year ago

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.