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

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

Closed
eriktoyra opened this issue Jan 16, 2020 · 19 comments
Closed

Comments

@eriktoyra
Copy link

eriktoyra commented Jan 16, 2020

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

  • TypeScript is used.
  • The component to document has a Component.displayName and the display name is assigned a string that doesn't match the component's name exactly.

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
Copy link
Author

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.

export const getComponentName = (component: Component): string => {
if (!component) {
return undefined;
}
if (typeof component === 'string') {
if (component.includes('-')) {
return titleCase(component);
}
return component;
}
if (component.__docgenInfo && component.__docgenInfo.displayName) {
return component.__docgenInfo.displayName;
}
return component.name;
};

@shilman
Copy link
Member

shilman commented Jan 16, 2020

@patricklafrance mind taking a look?

@atanasster
Copy link
Member

@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
Copy link
Author

@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
Copy link

stale bot commented Feb 11, 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 Feb 11, 2020
shilman added a commit that referenced this issue Feb 14, 2020
shilman added a commit that referenced this issue Feb 14, 2020
@shilman
Copy link
Member

shilman commented Feb 14, 2020

Repro: ed714ba (and 57e571c oops!!)

@stale stale bot removed the inactive label Feb 14, 2020
@sidhuko
Copy link

sidhuko commented Feb 20, 2020

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

@eriktoyra
Copy link
Author

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
Copy link
Member

shilman commented Feb 26, 2020

@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
Copy link
Author

@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
Copy link
Member

shilman commented Feb 27, 2020

@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 reactjs/react-docgen#387

@eriktoyra
Copy link
Author

@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
Copy link

pachuka commented Mar 18, 2020

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
Copy link

stale bot commented Apr 8, 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 8, 2020
@stale
Copy link

stale bot commented May 9, 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!

@stale stale bot closed this as completed May 9, 2020
@dzwillia
Copy link

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
Copy link

Facing same issue

@phocks
Copy link

phocks commented Nov 8, 2023

+1 still having this issue. Svelte + TS

@valentinpalkovic
Copy link
Contributor

@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?

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