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

Add converter for TSTypeOperator #59

Open
petegleeson opened this issue Apr 9, 2019 · 3 comments
Open

Add converter for TSTypeOperator #59

petegleeson opened this issue Apr 9, 2019 · 3 comments

Comments

@petegleeson
Copy link
Contributor

We have a complicated type that looks like:

// HtmlAttributes = AllHTMLAttributes - OnlyButtonProps
// We do this so onClick, and other props that overlap with html attributes,
// have the type defined by OnlyButtonProps.
type HtmlAttributes = Pick<
  React.AllHTMLAttributes<HTMLElement>,
  Exclude<keyof React.AllHTMLAttributes<HTMLElement>, keyof OnlyButtonProps>
>;

// This ends up being the Button prop API
type ButtonProps = HtmlAttributes & OnlyButtonProps

Running extract-react-types on this type throws the following error:

../packages/core/button/src/components/Button.tsx (../node_modules/extract-react-types-loader!../packages/core/button/src/components/Button.tsx)
Module build failed (from ../node_modules/extract-react-types-loader/index.js):
Error: Missing converter for: TSTypeOperator

It's pretty understandable something like this would not be supported by extract-react-types. This issue is really to discuss how type system specific features should (or shouldn't) be supported. My workaround at the moment is just creating a new file with a dummy component with the prop types I want extracted.

Maybe this highlights a need for a magic comment that extract-react-types will look for and extract if it exists.

@Noviny
Copy link
Contributor

Noviny commented Apr 9, 2019

In general, we’ve put magic into pretty-proptypes (you can override all type definitions using its API).

Since it’s an AST type, we should support it. What part is actually the TSTypeOperator that is causing the fail? Once we have a generic converter for that, we can look at supporting specific custom cases.

Precedent for this is how we use Array<>, which is technically not a type, but we specifically look for it and treat it differently.

There’s also things such as how we resolve type combinations or spread operators, where we do logic to resolve these to the intended type.

If you don’t want to start work on solving this, a link to ast explorer showing the simplest possible instance of TSTypeOperator and it’s AST, so we can look at what hits would need to be preserved to be able to move forwards.

(I assume the complexity here is not the type operator, but the fact you may want to resolve the pick, right?)

@Noviny
Copy link
Contributor

Noviny commented Apr 9, 2019

Oh, Pick and Exclude.

@alexreardon
Copy link

And Omit

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

No branches or pull requests

3 participants