Open yannbf opened 3 years ago
@hipstersmoothie I heard you might have some insights about this, care to take a look? Thanks!
@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
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
Any update ?
TLDR
If a component has a displayName like one of the following examples:
It will result in a code like this:
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 fromdisplayName
, then this would certainly fix the problem: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