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

displayName appears to be ignored #82

Closed
argshook opened this issue Apr 19, 2018 · 6 comments · Fixed by #87
Closed

displayName appears to be ignored #82

argshook opened this issue Apr 19, 2018 · 6 comments · Fixed by #87

Comments

@argshook
Copy link
Contributor

argshook commented Apr 19, 2018

react-docgen-typescript seems to not consider displayName property. moreover, it uses filename as displayName, which (for me) usually is wrong.

here's function in question:

https://github.com/styleguidist/react-docgen-typescript/blob/master/src/parser.ts#L555

i would expect Component.displayName = 'hello' or static displayName = 'hello' to be used when available.

related to #72

@pvasek
Copy link
Collaborator

pvasek commented Apr 25, 2018

I am not sure if this is wrong thing. Does styleguidist (with js only) use display name instead? I see display name more as the friendly name for debugging. Am I wrong?

@argshook
Copy link
Contributor Author

i consider this a bug since displayName value returned is wrong. don't know how styleguidist works in this regard, as i have not used it.

i am using react-docgen-typescript as a standalone tool. my components do have displayName set, however, react-docgen-typsecript returns me displayName: 'index' or something alike (filename).

another library that i use together with react-docgen-typescript is react-docgen, it is used to parse components written in good old js. this library does handle displayName and returns me same displayName that is set in component. i can later display that in documentation.

if i have js component and document it with react-docgen, all is good. now let's say i decide to rewrite js component to ts and use react-docgen-typescript this time. everything is ok, except from displayName. I used to get corret value, but now i get filename. it rarely is the same as displayName. thus, my documentation becomes falsy.

i think current behaviour is not correct and fixing it should do no harm.

tried to PR this but didn't finish yet, don't know typescript compiler well enough so stumbling around. ill try to find more time to tackle this

@dosentmatter
Copy link

dosentmatter commented May 3, 2018

I'm having the same problem. Styleguidist uses react-docgen to parse props by default.

I'm using Styleguidist and for .js files, I'm able to set the displayName multiple ways. Here is the order of precedence from the methods I've tried:

  1. Set displayName directly eg. Component.displayName = 'name1'

image

  1. Function name eg. const Component = function name2() {}

image

  1. variable name (for unamed functions such as arrow functions) eg. const name3 = () => {}

image

react-docgen-typescript doesn't use any of the three methods to get displayName. Below, I am setting displayName to 'Button' directly, the function name to AButton, and the variable name to Button, but the displayName gets set to lowercase button:
image

It's because displayName gets overriden here:

displayName: componentName,

const componentName = computeComponentName(exp, source);

exportName === '__function' ||
exportName === 'StatelessComponent'
) {
// Default export for a file: named after file
return path.basename(source.fileName, path.extname(source.fileName));

See how it gets set to the filename, path.basename(source.fileName, path.extname(source.fileName));. In my case, I was using 'StatelessComponent', but it would have been '__function' if I just used a regular fat arrow function. I don't know what 'default' is for.

At the minimum, we should be able to set displayName ourselves. It would be nice to be able to grab it from the function name and variable name. I prefer variable name because I use anonymous functions more than function declarations.

Styleguidist also has handlers option, but I don't really understand how to use it and I'm not sure if the example works with typescript or react-docgen-typescript.

erlendev added a commit to navikt/aksel that referenced this issue May 9, 2018
Due to a bug / strange behaviour from one of our dependencies (react-docgen-typescript)
used to generate docgenInfo for components, in how it assigns displayName to components,
components used directly by the Guideline-app needs to be exported as classes instead of
as function constants / StatelessComponents after updating outdated package-lock.json
file.

Ref open issue in react-docgen-typescript:
styleguidist/react-docgen-typescript#82
@argshook
Copy link
Contributor Author

@dosentmatter hello, thanks for thorough explanation

so the order should be like so:

  1. displayName
  2. function or class name
  3. function or class reference name

in other words, displayName is the king, if you set it manually, it must be important.
currently im not so interested in number 2 and 3, i think number 1 is most important. it should fix a lot of documentation for many.

im going to work on this today, hopefully with some results :)

@dosentmatter
Copy link

Hey @argshook. Yeah, I think displayName is the easiest to implement with the most flexibility. Actually, I did find another workaround. If you do export const, the name would show up correctly:
image
image

export default doesn't change the name for some reason.

@pvasek
Copy link
Collaborator

pvasek commented May 10, 2018

This should be fixed in v1.4.0 release thanks to @argshook and his PR #87.

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

Successfully merging a pull request may close this issue.

3 participants