storybookjs / storybook

Storybook is the industry standard workshop for building, documenting, and testing UI components in isolation
https://storybook.js.org
MIT License
84.84k stars 9.34k forks source link

Description and Props slots not shown when using TS and Component.displayName #9493

Closed eriktoyra closed 4 years ago

eriktoyra commented 4 years ago

Describe the bug @storybook/addon-docs fails to populate the "Description" and "Props" slots in the following scenario;

To Reproduce Here's a sample repo which reproduces this bug: https://github.com/eriktoyra/storybook-component-displayname-bug

  1. npm run storybook
  2. Open up the EmpireAlert story in the storybook and go to the Docs tab. Note that both the "Description" and "Props" slots are populated.
  3. Open up ./src/components/EmpireAlert.tsx and uncomment //EmpireAlert.displayName = "RebelAlliance.EmpireAlert" on line 47. (You might have to restart Storybook to see the effect)
  4. Go back to the Storybook and note that the "Description" and "Props" slots have lost their content.

Expected behavior I expect Component.displayName to have no effect at all on "Description" and "Props" in the docs. The displayName string is meant for debugging purposes.

Screenshots

Actual result

Skärmavbild 2020-01-16 kl  13 52 54

Expected result

Skärmavbild 2020-01-16 kl  13 53 23

Code snippets

./.storybook/main.js

module.exports = {
  stories: ["../src/**/*.stories.(js|tsx)"],
  addons: [
    "@storybook/preset-typescript",
    {
      name: "@storybook/preset-create-react-app",
      options: {
        tsDocgenLoaderOptions: {}
      }
    },
    "@storybook/addon-actions",
    {
      name: "@storybook/addon-docs",
      options: {
        configureJSX: true
      }
    },
    "@storybook/addon-links"
  ]
};

./src/components/EmpireAlert.tsx

import styled from "@emotion/styled";
import React from "react";

const Wrapper = styled("div")<{}>(({ theme }) => ({
  backgroundColor: "tomato",
  color: "white",
  padding: 10
}));

type AlertCode = "Code Red" | "Code Yellow" | "Code Green";

export interface EmpireAlertProps {
  /**
   * A title that brings attention to the alert.
   */
  title: AlertCode;
  /**
   * A message alerting about Empire activities.
   */
  message: string;
}

/**
 * This message should show up in the Docs panel if everything works fine.
 */
export const EmpireAlert: React.FC<EmpireAlertProps> = ({
  title = "Code Yellow",
  message
}) => (
  <Wrapper>
    <h1>{title}</h1>
    <p>{message}</p>
  </Wrapper>
);

/**
 * Specifying a displayName with a different name for the component will
 * cause DocsPage to fail to populate the Description and Props slots
 *
 * Works:
 *  - Using a displayName that _matches_ the component name exactly.
 *  - Not using a displayName at all.
 *
 * Fails:
 *  - Using a displayName that _does not match_ the component name exactly.
 */
 // EmpireAlert.displayName = "RebelAlliance.EmpireAlert"; // Uncomment to trigger bug

./src/components/EmpireAlert.stories.tsx

import React from "react";
import { EmpireAlert } from "./EmpireAlert";

export default {
  title: "EmpireAlert",
  component: EmpireAlert
};

export const _default = () => (
  <EmpireAlert
    title="Code Red"
    message="The Empire has been sighted on Hoth. Man the battle stations!"
  />
);

System: Storybook 5.3.3 is also affected.

Environment Info:

  System:
    OS: macOS Mojave 10.14.6
    CPU: (12) x64 Intel(R) Core(TM) i7-8750H CPU @ 2.20GHz
  Binaries:
    Node: 10.15.0 - ~/.nvm/versions/node/v10.15.0/bin/node
    npm: 6.13.4 - ~/.nvm/versions/node/v10.15.0/bin/npm
  Browsers:
    Chrome: 79.0.3945.117
    Firefox: 70.0.1
    Safari: 13.0.4
  npmPackages:
    @storybook/addon-actions: 5.3.4 => 5.3.4 
    @storybook/addon-docs: 5.3.4 => 5.3.4 
    @storybook/addon-links: 5.3.4 => 5.3.4 
    @storybook/addons: 5.3.4 => 5.3.4 
    @storybook/preset-create-react-app: ^1.5.1 => 1.5.1 
    @storybook/preset-typescript: ^1.2.0 => 1.2.0 
    @storybook/react: 5.3.4 => 5.3.4 

Additional context Yes, I will be able to assist in pushing a PR for this given som guidance.

eriktoyra commented 4 years ago

I poked around in the code base and found line 34 in ./addons/docs/src/blocks/utils.ts that might be the cause of this problem.

https://github.com/storybookjs/storybook/blob/66c524f194e14a6ba1328ba19bd6c71ecfc9e2da/addons/docs/src/blocks/utils.ts#L23-L39

shilman commented 4 years ago

@patricklafrance mind taking a look?

atanasster commented 4 years ago

@eriktoyra - i think the displayName is used by react-docgen-typescript to match the parsed typescript prop tables and and docgen info (ie the description). In this respect, it makes sense that the dislayName will affect those features, since the component prop table will not be "found", no?

eriktoyra commented 4 years ago

@eriktoyra - i think the displayName is used by react-docgen-typescript to match the parsed typescript prop tables and and docgen info (ie the description). In this respect, it makes sense that the dislayName will affect those features, since the component prop table will not be "found", no?

@atanasster Thank's for looking that up! It seems you are correct. But the purpose of displayName is not to be used as a "component lookup" alternative to the actual component name. As per the official documentation its purpose is to be used for displaying an alternative component name when debugging.

Given the fact that this bug does not occur when using displayName in a JavaScript based component I would say that the TypeScript version is doing it wrong.

stale[bot] commented 4 years ago

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!

shilman commented 4 years ago

Repro: https://github.com/storybookjs/storybook/commit/ed714ba365dcb449810464f33d12590fda1c8b1d (and https://github.com/storybookjs/storybook/commit/57e571cada30114ffc57bfaef3c7f4045dc0a23a oops!!)

sidhuko commented 4 years ago

OP could you try changing your code to use import * as React from 'react';?

eriktoyra commented 4 years ago

OP could you try changing your code to use import * as React from 'react';?

@sidhuko Tried that just now. It had no effect unfortunately. Even tried to update Storybook to 5.3.13 (latest as of now) but still no effect.

shilman commented 4 years ago

@eriktoyra can you try disabling typescript-react-docgen-loader and using the built-in babel-plugin-react-docgen instead? i'll be writing a guide about this as part of 6.0, but in the meantime maybe this tip can get you on the right track.

eriktoyra commented 4 years ago

@shilman It had no effect I'm afraid. Or maybe I dit it the wrong way? Did the following:

  1. Uninstalled react-docgen-typescript-loader.
  2. Installed babel-plugin-react-docgen.
  3. Added .babelrc containing the following snippet:
    {
    "plugins": ["react-docgen"]
    }

Let me know if I can assist in some other way with this issue.

shilman commented 4 years ago

@eriktoyra one more thing 😄

Try changing:

export const EmpireAlert: React.FC<EmpireAlertProps> = ({
  title = "Code Yellow",
  message
}) => (

To:

export const EmpireAlert: React.FC<EmpireAlertProps> = ({
  title = "Code Yellow",
  message
}: EmpireAlertProps) => (

Workaround for https://github.com/reactjs/react-docgen/issues/387

eriktoyra commented 4 years ago

@shilman Still no success. The only way I can make it work is as in my code comment below. It seems very much related to the displayName property.

/**
 * Specifying a displayName with a different name for the component will
 * cause DocsPage to fail to populate the Description and Props slots
 *
 * Works:
 *  - Using a displayName that _matches_ the component name exactly.
 *  - Not using a displayName at all.
 *
 * Fails:
 *  - Using a displayName that _does not match_ the component name exactly.
 */
EmpireAlert.displayName = "RebelAlliance.EmpireAlert";

I'm still having a theory that using a displayName not matching the actual component's name sends the component doc parser looking for a component that does not exists. See function getComponentName here.

pachuka commented 4 years ago

Looks like the issue specifically has to do with the filename of the component. For example if I do product-section-header.tsx the generated docgeninfo is attached to productsectionheader:

This one fails to find props:

/***/ "./src/molecules/quote/product-section-header/product-section-header.tsx":
/*!*******************************************************************************!*\
  !*** ./src/molecules/quote/product-section-header/product-section-header.tsx ***!
  \*******************************************************************************/
 /*component implementation removed*/
try {
    // @ts-ignore
    productsectionheader.displayName = "productsectionheader";
    // @ts-ignore
    productsectionheader.__docgenInfo = { /* data removed */ };
    // @ts-ignore
    if (typeof STORYBOOK_REACT_CLASSES !== "undefined")
        // @ts-ignore
        STORYBOOK_REACT_CLASSES["src/molecules/quote/product-section-header/product-section-header.tsx#productsectionheader"] = { docgenInfo: productsectionheader.__docgenInfo, name: "productsectionheader", path: "src/molecules/quote/product-section-header/product-section-header.tsx#productsectionheader" };
}
catch (__react_docgen_typescript_loader_error) { }
/***/ }),

vs. if I name the file ProductSectionHeader.tsx this one the props map properly

/***/ "./src/molecules/quote/product-section-header/ProductSectionHeader.tsx":
/*!*****************************************************************************!*\
  !*** ./src/molecules/quote/product-section-header/ProductSectionHeader.tsx ***!
  \*****************************************************************************/
 /*component implementation removed*/
exports.ProductSectionHeader = ProductSectionHeader;
try {
    // @ts-ignore
    ProductSectionHeader.displayName = "ProductSectionHeader";
    // @ts-ignore
    ProductSectionHeader.__docgenInfo = {  /* data removed */};
    // @ts-ignore
    if (typeof STORYBOOK_REACT_CLASSES !== "undefined")
        // @ts-ignore
        STORYBOOK_REACT_CLASSES["src/molecules/quote/product-section-header/ProductSectionHeader.tsx#ProductSectionHeader"] = { docgenInfo: ProductSectionHeader.__docgenInfo, name: "ProductSectionHeader", path: "src/molecules/quote/product-section-header/ProductSectionHeader.tsx#ProductSectionHeader" };
}
catch (__react_docgen_typescript_loader_error) { }
/***/ }),

In my scenario I am just using @storybook/preset-typescript - 1.2.0, which internally uses react-docgen-typescript-loader AFAIK. Though I just noticed there have been updates to their repo + migration strategy away from that, so I'll try that! https://github.com/storybookjs/presets/blob/master/packages/preset-typescript/CHANGELOG.md

stale[bot] commented 4 years ago

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[bot] commented 4 years ago

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!

dzwillia commented 4 years ago

I ran into this issue just a bit ago as well. We wanted to preface our components with the library name for better specificity in React DevTools. Have not had any luck with any of the workaround or suggestions above.

HarishSha commented 2 years ago

Facing same issue

phocks commented 1 year ago

+1 still having this issue. Svelte + TS

valentinpalkovic commented 6 months ago

@phocks I just tried it, and I couldn't reproduce it with the latest Storybook version. Can you provide a reproduction with the latest version of Storybook?