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

Parse and check type arguments on JSX opening and self-closing tags #22415

Merged
merged 2 commits into from
Mar 22, 2018

Conversation

weswigham
Copy link
Member

Allows, eg,

<MyComponent<number> data={12} />

in .tsx files.

Fixes #6395.

Was there a list of things we were concerned wouldn't parse well? AFAIK, this is actually easy for us to unambiguously parse.

min = Math.min(min, getMinTypeArgumentCount(sig.typeParameters));
max = Math.max(max, length(sig.typeParameters));
}
const paramCount = min < max ? min + "-" + max : min;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would find min === max ? min : min + "-" + max clearer

@@ -17337,6 +17380,17 @@ namespace ts {
}
}

function getTypeArgumentArityError(node: Node, signatures: Signature[], typeArguments: NodeArray<TypeNode>) {
let min = Number.POSITIVE_INFINITY;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure why the original author didn't just use Infinity / -Infinity ?

Copy link
Member

@RyanCavanaugh RyanCavanaugh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have nitty comments in moved code that can be ignored 👍

@weswigham weswigham merged commit a909000 into microsoft:master Mar 22, 2018
@weswigham weswigham deleted the jsx-type-arguments branch March 22, 2018 22:07
@aaronjensen
Copy link

This is great! Is there a related issue on Babel? What is the process for ensuring they stay up to date?

@weswigham
Copy link
Member Author

I don't think we have a formal process right now, but pinging @andy-ms seems like a decent way to check. 😝

@mvestergaard
Copy link

@weswigham Really great this finally made it into the language!

Question: When tsconfig is set to "tsx": "preserve", is the generic removed?

@weswigham
Copy link
Member Author

It aught to be. If not, it's a bug.

@andrewbranch
Copy link
Member

Is there any sense of in what release this will land?

@weswigham
Copy link
Member Author

2.9 which will be released inside two months, given our bimonthly release cadence 😄

@marty-wang
Copy link

It seems that one has to explicitly specify the type, as oppose to that the compiler can infer the type from the value.

What I mean is that in JSX, say I have the component below

interface IProps<T> {
    value: T;
    children: (value: T) => React.ReactNode;
}

export class MyComp<T extends {}> extends React.Component<IProps<T>> {
    public render() {
        return this.props.children(this.props.value);
    }
}

when I use MyComp, in the code below, specify the type number, otherwise the val variable will not have type of number.

<MyComp<number> value={4}>
    {val => {
       // the type of val is number
        return "";
    }}
</MyComp>

However, If I omit specifying the type number, the val variable will be type of {}.

<MyComp value={4}>
    {val => {
       // the type of val is {}
        return "";
    }}
</MyComp>

This seems to be a bug, because compare to Generics in regular class, like below., the type of number does not have to be explicitly specified.

class A<T> {
    constructor(public val: T) {
    }
}

const a = new A(5)
a.val // val is type of number

@andrewbranch
Copy link
Member

@marty-wang IIUC, I think what you're seeing is #22636

@marty-wang
Copy link

marty-wang commented May 4, 2018

@andrewbranch yes, it is the same issue. In #22636 @mhegazy seems saying it would be addressed in 2.9. However, at least it is not yet in 2.9.0-dev.20180503.
Really hope that it will be fixed by the official release of 2.9

@weswigham
Copy link
Member Author

#23492 has yet to be merged

@marty-wang
Copy link

@weswigham sweet! thanks!

@jorySsense
Copy link

jorySsense commented Jun 11, 2018

Question,

What would be best way to pass a generic type to a child component?

interface GenericProps<T> {
  isFetching: boolean;
  items: T[];
  removeItem: () => Promise<any>;
  history: History;
}

class ListPage<T> extends React.Component<GenericProps<T>> {
  render() {
    return <LayoutCard {...this.props} />
      <ListWrapper<T> items={this.props.items}>
        {this.props.children}
      </ListWrapper>
    </LayoutCard>
  }
};

@weswigham
Copy link
Member Author

What you have there aught to be correct.

@jorySsense
Copy link

@weswigham i was getting a

Type '{}' is not assignable to type 'IntrinsicAttributes & IntrinsicClassAttributes<ListWithModalConsumer<{}>> & Readonly<{ children?:...'.   Type '{}' is not assignable to type 'Readonly<ListPageProps<{}>>'

@jorySsense
Copy link

jorySsense commented Jun 11, 2018

Oh i guess i should of mentioned that List accepts a generic type

export class ListWrapper<T> extends React.Component<ListWrapperProps<T>> {

  render() {
    const { items, children } = this.props;
    return <ModalConsumer>
      {({ showModal, hideModal }) =>
        <List style={{ width: '100%' }} items={items}
              render={(item: T) => children({ item, hideModal, showModal })}>
        </List>
      }
    </ModalConsumer>
  }
}

@weswigham
Copy link
Member Author

weswigham commented Jun 11, 2018

You need to constrained your T to at least have the members required by the component you're invoking. In your example, T has no constraint, so it is overriding the type implied by your addition of the items prop and checking against the {} implied by the unconstrained T. (Or intersect the desired props type with T instead of just passing T)

@jameslaneconkling
Copy link

is there a non-jsx equivalent for this? e.g.

given this usage w/ jsx

<MyGenericComponent<SomeType> {...props} />

what is the equivalent using createElement?

createElement(StringInput, props)

// [ts] Type '{}' is not assignable to type 'SomeType'.

@nickserv
Copy link
Contributor

I believe the first generic parameter of createElement is equivalent:

createElement<SomeType>(MyGenericComponent, props)

@jameslaneconkling
Copy link

The type signature of createElement is

    function createElement<P>(
        type: SFC<P> | ComponentClass<P> | string,
        props?: Attributes & P | null,
        ...children: ReactNode[]): ReactElement<P>;

so looks like the first parameter is Props

Copy link

@zero-t4 zero-t4 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice

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 this pull request may close these issues.

10 participants