strothj / babel-plugin-react-docgen-typescript

Babel Plugin to generate docgen data from React components written in TypeScript.
12 stars 4 forks source link

Add shouldExtractValueFromUnion and fix default value bug #21

Closed DSil closed 4 years ago

DSil commented 4 years ago

This PR includes two commits:

Screenshots Before: ![Screen Shot 2020-09-09 at 12 36 26 AM](https://user-images.githubusercontent.com/6265045/92541863-12c54100-f23f-11ea-8145-070c2bc0a0e8.png) Now: ![Screen Shot 2020-09-09 at 12 15 03 AM](https://user-images.githubusercontent.com/6265045/92541870-1789f500-f23f-11ea-806d-a56b1ac07e9c.png)

Note 1:

shouldExtractValueFromUnion allows a more complete parsing than shouldExtractLiteralValuesFromEnum. It supports union types with multiple types (not only strings). I believe the latter is no longer needed, since the first covers the same scenarios, but I prefer to avoid a breaking change and keep supporting both, since react-docgen-typescript also supports both.

Note 2:

When using a default value that is not a string, the babel parser simply could not compile, as it was always expecting a string as default value. This is now fixed by transforming the value into a string. (See Problem with tests)

Problem with tests:

Running the tests locally was not a problem (using node 12). However, the pre-commit also runs the tests and it constantly failed because of a known bug with node and the toString method (implicitly called in the template string transformation). I was not able to enforce this to run on node 11.12.0 or above (the version that fixes this issue). Are you able to add this? Or is this not an issue, since the tests run locally with no problem?

Changelog and README:

I updated them as I thought it should be but you're free to update them as it best suits you.

Thanks.

DSil commented 4 years ago

Hi @strothj are you still maintaining this project?

strothj commented 4 years ago

Unfortunately I’ve had no time as I’ve been stuck in a mix of travel and work. I don’t use this project myself personally. I can pass the reigns to you if you’re interested.

DSil commented 4 years ago

I would really appreciate if I could just use this with these features, so that I don’t need to build something similar on my side. I guess I can take the reigns if you’re open to it. There doesn’t seem to be much going around anyway :)

strothj commented 4 years ago

@DSil I've invited you as a collaborator to this repository. I couldn't transfer you the repository yet because you have an existing fork on your account. I'm guessing the pull requests will need to be taken care of first. What is your NPM username?

DSil commented 4 years ago

I do not have an NPM account. I’ll take a moment to do that later today

DSil commented 4 years ago

My npm username is the same as github: dsil

strothj commented 4 years ago

I invited you in npm. You should now be able to handle the repo and npm package.

DSil commented 4 years ago

Thank you @strothj, I'll handle it.