react-docgen-typescript-loader icon indicating copy to clipboard operation
react-docgen-typescript-loader copied to clipboard

Code fails when using displayName that contains invalid characters or reserved words

Open yannbf opened this issue 5 years ago • 4 comments
trafficstars

TLDR

If a component has a displayName like one of the following examples:

Button.displayName = "UI/Button"; // invalid characters
Button.displayName = "Some-Kebab-Name"; // invalid characters
Toggle.displayName = "switch"; // reserved word
Basic.displayName = "default"; // reserved word

It will result in a code like this: image

Which will end up breaking the app when the code is parsed.

Explanation

There are a few issues opened in Storybook like this and this which made me investigate why they were happening, and after some thorough investigation, I've noticed that this loader has two pieces of code which will break the code in case we have unsupported names: here and here. There was a possible workaround mentioned here to disable the setDisplayName from happening, but even if that was the case, the code would still break in the line mentioned previously, which will run regardless of the setDisplayName property set to true or false.

Essentially, the root of the problem is using typescript.createIdentifier with a string that results in an invalid identifier. If there was a way to get the component name other than just from displayName, then this would certainly fix the problem:

ts.createPropertyAccess(
-  ts.createIdentifier(d.displayName),
+  ts.createIdentifier(d.functionName), // <== supposedly new variable with function name written from the raw source
  ts.createIdentifier("displayName"),
)

But I know things are not that simple. I don't have a solution but I'm willing to help fix this but I definitely need some guidance. For now I'd say it's good to at least validate the displayName and throw a reasonable error if it's not valid.

I propose to close the related issues and make this as a centralised place to take care of this problem. What do you think?

Related issues are #102 #24 #84

yannbf avatar Sep 01 '20 18:09 yannbf

@hipstersmoothie I heard you might have some insights about this, care to take a look? Thanks!

yannbf avatar Sep 11 '20 13:09 yannbf

@yannbf you might have more luck filing this on https://github.com/hipstersmoothie/react-docgen-typescript-plugin, which is what we currently use in storybook by default

shilman avatar Oct 01 '20 05:10 shilman

Hey @shilman although that's true, react-docgen-typescript-plugin still uses this lib, and funny enough the only time react-docgen-typescript-loader is used is for the functionality which I am describing.

So I guess it's important to think either if there should be a fix in this lib, which in turn will also fix the one used in Storybook, or if there should be a patch in typescript-plugin instead. In any way would be nice to know what you guys think @strothj @hipstersmoothie

yannbf avatar Oct 01 '20 09:10 yannbf

Any update ?

flessard avatar Nov 19 '20 20:11 flessard