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

Broken jsx element with not static type with ts 3.2.1 #28768

Closed
dfilatov opened this issue Nov 30, 2018 · 9 comments
Closed

Broken jsx element with not static type with ts 3.2.1 #28768

dfilatov opened this issue Nov 30, 2018 · 9 comments
Assignees
Labels
Bug A bug in TypeScript Domain: JSX/TSX Relates to the JSX parser and emitter Fixed A PR has been merged for this issue

Comments

@dfilatov
Copy link

dfilatov commented Nov 30, 2018

TypeScript Version: 3.2.1

Search Terms:
jsx

Code

import * as React from 'react';

function render(url?: string): React.ReactNode {
    const Tag = url? 'a' : 'button';
    return <Tag>test</Tag>;
}

Expected behavior:
Everything is ok. TypeScript 3.1.6 compiles it properly.

Actual behavior:
error: JSX element type 'Tag' does not have any construct or call signatures. [2604]

@devrelm
Copy link

devrelm commented Nov 30, 2018

Related: #28631

@weswigham weswigham added the Domain: JSX/TSX Relates to the JSX parser and emitter label Nov 30, 2018
@weswigham
Copy link
Member

weswigham commented Nov 30, 2018

It's because we started strongly checking tag calls such as these, and the 'a' tag's properties and 'button' tag's properties are heavily dissimilar (one isn't a strict subset of the other) - they're all optional, so a call like this can still succeed at runtime, but in the typesystem at present the two signatures are irreconcilable. Needs a full fix of #7294 to work well.

@rhys-vdw
Copy link
Contributor

rhys-vdw commented Dec 2, 2018

Please consider this now broken example:

import React, {
  ReactType,
  StatelessComponent,
} from "react"
import classNames from "classnames"

interface Props {
  className?: string
}

export function createStyledComponent<P extends { className: string }>(
  Component: ReactType<P>,
  className: string
): StatelessComponent<Props> {
  return props => (
    <Component className={classNames(className, props.className)}>
      {props.children}
    </Component>
  )
}
ERROR in [at-loader] ./app/assets/javascripts/utilities/component-factory.tsx:19:6
    TS2604: JSX element type 'Component' does not have any construct or call signatures.

Is this something that should be addressed in @types/react?

@Jessidhia
Copy link

Jessidhia commented Dec 3, 2018

Nothing that can be done on the @types/react side, or rather, what can be done is already done -- ReactType<P> will work more-or-less correctly with React.createElement.

image

Note that your P extends { className: string } will actually reject all DOM elements because their className includes undefined which is not assignable to string.

image

@rhys-vdw
Copy link
Contributor

rhys-vdw commented Dec 3, 2018

@Kovensky Oops, the existing (working) code I had was:

export function createStyledComponent(
  Component: ReactType,
  className: string
): StatelessComponent<Props> {
  return props => (
    <Component className={classNames(className, props.className)}>
      {props.children}
    </Component>
  )
}

That generic change was an attempt to get it working under TS 3.2.1

sqs added a commit to sourcegraph/sourcegraph that referenced this issue Dec 4, 2018
TypeScript 3.2 has better support for generic-typed object spreads, which helped in another commit.

Compile errors were due to:

- microsoft/TypeScript#28768
@azizhk
Copy link
Contributor

azizhk commented Dec 6, 2018

Just a small reproduction:
https://repl.it/repls/InconsequentialBreakableCategories
Go to package.json and change typescript version to 3.2.1 to see how this breaks.

How I had to fix it:
https://github.com/ritz078/pebble/pull/151/files#diff-288d98c45cab569122e391278589e7dbR123
But still, that does not work as something fails in the union of types. Check how InputProps union type is defined. Now I can't do something like:

const inp = (
  <Input
    textArea={abc as boolean}
  />
)

even though my union type is

type InputProps = {
  textArea?: false
  // more types where textArea would be undefined or false
} & {
  textArea: true
  // more types where textArea would be true
}

@RyanCavanaugh
Copy link
Member

@weswigham can we get a fresh read on this?

@weswigham
Copy link
Member

weswigham commented Mar 7, 2019

The OP's example should have been fixed by #29011 (and testing shows it is). @rhys-vdw's example here also works. @azizhk's example probably typechecks better than he wants, because we correctly reject the spread of a React.InputHTMLAttributes<HTMLTextAreaElement> as A's arguments, because a "input" element won't be happy getting a "textarea"'s onChange handler (or vice-versa).

@weswigham weswigham added Bug A bug in TypeScript Fixed A PR has been merged for this issue and removed Needs Investigation This issue needs a team member to investigate its status. labels Mar 7, 2019
@azizhk
Copy link
Contributor

azizhk commented Mar 11, 2019

Hi, I just upgraded to

"@types/react": "^16.8.7",
"@types/react-dom": "^16.8.2",
"typescript": "^3.3.3333",

I can use this for string components now const A = condition ? "input" : "textarea",

But I cannot use this for components which have generics

class OptionGroupCheckBox<OptionType> extends React.PureComponent<OptionGroupProps<OptionType>> {
  // methods
}

class OptionGroupRadio<OptionType> extends React.PureComponent<OptionGroupProps<OptionType>> {
  // methods
}

const OptionGroup = condition ? OptionGroupCheckBox : OptionGroupRadio;
const vdom = (
  <OptionGroup
    // OptionGroupProps<OptionType>
  />
)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript Domain: JSX/TSX Relates to the JSX parser and emitter Fixed A PR has been merged for this issue
Projects
None yet
Development

No branches or pull requests

7 participants