Skip to content
This repository has been archived by the owner on Nov 20, 2020. It is now read-only.

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

Open
yannbf opened this issue Sep 1, 2020 · 4 comments

Comments

@yannbf
Copy link

yannbf commented Sep 1, 2020

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

yannbf commented Sep 11, 2020

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

@shilman
Copy link

shilman commented Oct 1, 2020

@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

@yannbf
Copy link
Author

yannbf commented Oct 1, 2020

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

@flessard
Copy link

Any update ?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants