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

transform JSX.Element to React$Node or React.Node #76

Closed
villesau opened this issue Mar 9, 2019 · 9 comments
Closed

transform JSX.Element to React$Node or React.Node #76

villesau opened this issue Mar 9, 2019 · 9 comments

Comments

@villesau
Copy link

villesau commented Mar 9, 2019

Hi, great work with Flowgen!

I tried to convert some React component definitions yesterday that has JSX.Element types. I think proper way to handle these types is to transform them to React$Node or React.Node automatically instead of global$JSX$Element. I think it's safe assumption in Flow since Flow assumes all the JSX to be React anyways, which means only JSX you can have in Flow codebase is React. Where this kind of logic would belong in the code base if I would want to give it a try?

Here is an example input and output of current behaviour:

Input:

/// <reference types="react" />
import * as React from 'react';
declare class Button extends React.PureComponent<React.ButtonHTMLAttributes<HTMLButtonElement> & {
    active?: boolean;
    children?: string | JSX.Element;
    fullWidth?: boolean;
    icon?: JSX.Element;
    small?: boolean;
    large?: boolean;
    primary?: boolean;
    secondary?: boolean;
    destructive?: boolean;
    linkDefault?: boolean;
    link?: boolean;
    loading?: boolean;
}> {
    render(): JSX.Element;
}
export default Button;

Output:

import * as React from "react";
declare class Button
  mixins React.PureComponent<
      React.ButtonHTMLAttributes<HTMLButtonElement> & {
        active?: boolean,
        children?: string | global$JSX$Element,
        fullWidth?: boolean,
        icon?: global$JSX$Element,
        small?: boolean,
        large?: boolean,
        primary?: boolean,
        secondary?: boolean,
        destructive?: boolean,
        linkDefault?: boolean,
        link?: boolean,
        loading?: boolean
      }
    > 
  render(): global$JSX$Element;
}
declare export default typeof Button;
@orta
Copy link
Collaborator

orta commented Mar 9, 2019

Yeah, I agree 👍

@villesau
Copy link
Author

villesau commented Mar 9, 2019

The current output is pretty broken in many ways. Desired output for above would be roughly something like this:

declare class Button extends React$PureComponent<
HTMLButtonElement &
{
  active?: boolean,
  children?: string | React$Node,
  fullWidth?: boolean,
  icon?: React$Node,
  small?: boolean,
  large?: boolean,
  primary?: boolean,
  secondary?: boolean,
  destructive?: boolean,
  linkDefault?: boolean,
  link?: boolean,
  loading?: boolean
}> {
  render(): React$Node; // Even this is not perhaps needed as React$Component have it by default
}

So there are actually quite a bunch what needs to be mapped from TS to Flow when transforming component definitions. React.ButtonHTMLAttributes<HTMLButtonElement> is also something that might not have 1:1 equivalent.

How would you approach implementing better React support?

@goodmind
Copy link
Contributor

goodmind commented Mar 9, 2019

@villesau
Copy link
Author

villesau commented Mar 9, 2019

@goodmind
Copy link
Contributor

goodmind commented Mar 9, 2019

@villesau its not jsx props, they should be camelCase. Someone should pick up this PR facebook/flow#6727 to typecheck native elements first

@villesau
Copy link
Author

villesau commented Mar 9, 2019

On the other hand in the mean time this should be sufficient: ReactDOM$HTMLElementJSXIntrinsic<HTMLButtonElement, ReactDOM$HTMLElementProps>

https://github.com/facebook/flow/pull/6727/files#diff-ce626ccb6b8eb922c17388db8d33cbabR281

@goodmind
Copy link
Contributor

goodmind commented Mar 9, 2019

Fixed in #77

@goodmind
Copy link
Contributor

goodmind commented Mar 9, 2019

@villesau but this is types from PR :)
Now button is completely untyped
button: {instance: HTMLButtonElement, props: {children?: React$Node, [key: string]: any}},

@villesau
Copy link
Author

villesau commented Mar 9, 2019

Yeah exactly. If it's not yet part of Flow builtin libdefs, it's still possible to type without.

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