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

TypeScript reports an error when trying to use a variable of type string as JSX element identifier #28806

Closed
MadaraUchiha opened this issue Dec 2, 2018 · 15 comments
Labels
Duplicate An existing issue was already created

Comments

@MadaraUchiha
Copy link

MadaraUchiha commented Dec 2, 2018

TypeScript Version: 3.2.1, 3.3.0-dev.20181201

Search Terms: TypeScript JSX string type, TypeScript JSX string element

Code

let El = "div"; // El is of type string, not 'div'

const x = <El onClick={() => 42} />;

Expected behavior:
Before (and not including) 3.2.0, this code emitted no errors.

Actual behavior:
TypeScript emits error:

[ts]
Type '{ onClick: () => number; }' is not assignable to type '{}'.
  Property 'onClick' does not exist on type '{}'. [2322]

Playground Link: N/A - The playground doesn't support JSX.

Related Issues:
Not on GitHub, but the release announcement specifically mentioned this as a possible regression, and asked to report it.


It seems like it infers a type of either {} or React.IntrinsicAttributes now, instead of what used to be any.

I'm not sure what's the correct course of action here, but upgrading to 3.2 made a previously compiling code to not compile due to this, so I'm currently forced to revert to 3.1.6.

Ideas?

cc @aizikdahan @Linkgoron @Moshiko1989

@ajafff
Copy link
Contributor

ajafff commented Dec 2, 2018

Duplicate of #28768?

@MadaraUchiha
Copy link
Author

@ajafff Indeed seems like it. Could you mark it as a dupe?

@weswigham
Copy link
Member

weswigham commented Dec 4, 2018

No, I doubt this is a dupe - you're not dealing with a union of tags here.

@weswigham weswigham added Bug A bug in TypeScript Domain: JSX/TSX Relates to the JSX parser and emitter labels Dec 4, 2018
@weswigham
Copy link
Member

@MadaraUchiha To clarify the issue, do you expect

declare var Element: string;
const tag = <Element onClick={() => 42} />;

to work?

@MadaraUchiha
Copy link
Author

Yes.

My specific usecase is a <Restricter> component which, among other things, accepts the type of element it should render as, defaulting to 'div'.

I'd expect either string to work, or some workaround (ReactPossibleHTMLElements or whatever) to be available.

@weswigham
Copy link
Member

React's intrinsic elements don't have an indexer - I don't see how an arbitrary string is supposed to work.

@MadaraUchiha
Copy link
Author

MadaraUchiha commented Dec 4, 2018

@weswigham The code that I wrote definitely compiles and passes typechecking before 3.2. I honestly don't know how or what it infers (I'm guessing any because gibberish props pass too).

It's worth noting that it passes before 3.2 with or without @types/react, and doesn't pass in 3.2 with or without @types/react.

@weswigham
Copy link
Member

Goes to check old code It should have at least been a Property_0_does_not_exist_on_type_1 error. Hm. You're correct though, it definitely wasn't an error before (and was quietly just being checked as any)! It should have been, though; the intent was there!

@weswigham
Copy link
Member

As you said yourself - it allowed passing arbitrary garbage props, so was patently unsafe. You can explicitly opt-in to an checked tag by casting the tag in question any, eg:

let El: any = "div"; // El is unchecked and will allow any props to be passed to it

const x = <El onClick={() => 42} />;

@MadaraUchiha
Copy link
Author

@weswigham This is an acceptable workaround for me for now. However, taking it one step further, would it be possible to make it actually safe? onClick is something that all HTML elements share, ultimately I aim to bind onClickCapture, onTouchStartCapture etc on my element, those should be available on all elements, or at the very least, I'm only interested in HTML elements that have those props.

Is it possible to making it safe after all? I can already foresee problems with the different kind of SyntheticEvents that React has.

@weswigham
Copy link
Member

@MadaraUchiha A combination of limiting your input to keyof JSX.IntrinsicElements instead of string should handle the "only valid html elements" bit from you, and #7294 should track being able to make such a union of tags actually usable from us.

@manucorporat
Copy link

manucorporat commented Dec 5, 2018

@weswigham so if i understand correctly, it does accept a string inside keyof JSX.IntrinsicElements but not union types of strings inside keyof JSX.IntrinsicElements?

We are having the same error with:

const TagType = condition ? 'a' : 'button';
return <TagType class="hello"></TagType>;

How is that code unsafe? we just use "common" properties of the union type. ie common properties like onClick or class.

@weswigham
Copy link
Member

Yeah, that's the part where I said we needed to do better - the props of an 'a' tag and a 'button' tag aren't a subset of one another, so our current signature resolution doesn't handle it well. #7294 tracks that.

@sandersn sandersn added this to the TypeScript 3.4.0 milestone Dec 11, 2018
@sandersn
Copy link
Member

@weswigham That makes it sounds like a dupe of #7294. Is that correct?

@weswigham
Copy link
Member

Yes - how react is typed and used has seriously increased the urgency of fully implementing #7294 😄

@RyanCavanaugh RyanCavanaugh added Duplicate An existing issue was already created and removed Bug A bug in TypeScript Domain: JSX/TSX Relates to the JSX parser and emitter labels Mar 13, 2019
@RyanCavanaugh RyanCavanaugh removed this from the TypeScript 3.4.0 milestone Mar 13, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Duplicate An existing issue was already created
Projects
None yet
Development

No branches or pull requests

6 participants