Support for importing types

[devongovett]

Closes #33.

This adds support for importing propTypes, flow, and typescript from other modules. Various people have tried to do this through external handlers in the past, but this has been more challenging because they have needed to reimplement or copy many of the builtin helper functions available in react-docgen. This PR adds builtin support for importing, and should be much simpler than external solutions.

Current limitations:

• ES6 imports only. Could add CommonJS support, but it's a bit more complicated and I'm not sure it's worth it when most apps should be using ES6 now anyway.
• Resolving from node_modules is supported, but flow libdefs (flow-typed folder, .js.flow files, and .d.ts files) are unsupported. Would need to reimplement a custom resolver for those.

Some of these could be resolved, but we could also decide to leave some for future enhancements in order to start somewhere.

• Allow different resolvers
• Allow to disable resolving imports

[danez]

Unrelated to this PR: The Typescript change will be in a new major, because we also changed the license, so if you want to rename some internal helpers to better reflect their real purpose feel free to do so and create a PR. If you have the time.

[danez]

Thanks looking forward to see this in action.

I haven't looked at the code yet, but two things I would like to mention:

• We need to make sure that react-docgen works in environments were fs is not available. Not sure whats the best solution, but I've seen in other projects where filersystem related functionality is put in a single file and then "overwritten" by specifying the browser field in package.json which loads a mock. There are probably better solutions though.

• Could you rebase on master? I removed recast and instead use ast-types directly, so might have some merge conflicts and the new files would need adaption.

[devongovett]

Yep, working on the rebase now. For the fs thing, I think we could use the browser override trick, and point to a file that just returns null. If fs is unavailable, importing will just be unsupported.

[devongovett]

Ready for review! I added support for namespace imports, and re-exports. Leaving CommonJS and libdefs for future work. Also added a browser override for resolveImportedValue which does nothing.

[nicholaai]

I know a lot of the community has been dying for this, myself included. thank you! Hope this can get in after proper review 👍

[devongovett]

Hey sorry to be that guy (I know how it is), but is there anything I can do to help move this PR along?

[mjhayes412]

I second this ^^

[danez]

Sorry I was working on other stuff and Felix is also not available. I hope I will try to find some time soon.

[danez]

Really nice work.

It would be nice to add more unit tests, especially for resolveToValue and resolveImportedValue. We could use https://github.com/tschaub/mock-fs to create the tests.

Will this also work for type/typeof imports in flow? Didn't see any tests. If not I'm happy to do that in a separate PR.

I think only doing ES6 imports is fine.

[danez]

Suggested change

	resolveImports: boolean = true,
	resolveImports?: boolean = true,

I wonder if it makes more sense to have false as default? That would be less breaking for outside resolvers/handlers. But we are also doing a major anyway...
Are we using more cases with true in our codebase?

[danez]

Why overwriting the parserOptions?

[fkling]

I haven't looked at it yet, but I wanted to note two things:

• At Facebook we use CommonJS like syntax
• We also use a different module loader (we have a global name space)

If we are going to support dependency resolution, then we need to support multiple syntax and module loaders at some point, or at least make it easy to plug those in.

Short term we need to be able to either enable or disable dependency resolution (depending on what the default is), via CLI parameters and/or config, so that it still runs in environments where resolution does not work.

[okonet]

Any ETA on this to get merged?

[devongovett]

I need to find time to make the requested changes. Namely make resolvers pluggable as @fkling requested, and add more tests as @danez requested. If someone has time and wants to help, that would be appreciated.

[lifeiscontent]

@devongovett do you have time to finish this up?

[sami616]

Vote for getting this merged! 👍🏼

[connor-baer]

@jdevo23, you might wanna exclude GitHub from your vacation autoresponder 😄

[sami616]

Does this PR also solve the issue of using react types from the namespace (instead of directly importing them) resulting in any too?

ie: React.ReactNode instead of ReactNode

[hipstersmoothie]

Is react-docgen-typescript a fork of react-docgen? What's the relationship between the two?

@Vanuan I do not believe so. react-docgen and react-docgen-typescript both implement the standard for react docgen. (I think this was defined for styleguidist) They are both separate implementation that approach the problem in different ways.

react-docgen-typescript basically just runs your code through a typescript compiler and uses the AST typescript produces to construct the prop-type information (combining interfaces, types, and jsdoc). I am unsure of how this package works though.

[blockracers]

so close...

[michaelbayday]

Very interested in the PR.
Could we please get some traction on this PR?

Thank you!

[ethanshar]

Any plans to merge this PR any time soon ?

[agjs]

Thanks for opening the PR and putting in the work. I believe many of us would appreciate if this can get merged as soon as possible.

[haffmaestro]

ITT: A lot of people asking if the work can be done for them. (Me included)
Not present: Anyone willing to do the work.

[shilman]

@haffmaestro not quite.. check out @phated's efforts to get this over the line: #464

and give him a big cheer!! 🙌

[phated]

Fwiw, that work ☝️ is now ready for review. It's a beefy PR (lots and lots of tests) and I'd love for some people to look at it!

[kubilaykiymaci]

hi, when will this pr will be merged? does anybody care about this? i have to decide about my usage.

[haffmaestro]

@kubilaykiymaci Looks like a lot of progress was made by @phated in #464 which has been merged 👏.

I don't think this pr will ever be merged, but looks like v5.4.0-alpha.0 includes #464 so that might solve your problems.