microsoft / TypeScript

TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
https://www.typescriptlang.org
Apache License 2.0
100.33k stars 12.4k forks source link

Type JSX elements based on createElement function #14729

Open ivogabe opened 7 years ago

ivogabe commented 7 years ago

TypeScript currently uses a JSX namespace to type the props of JSX elements. This is customisable by modifying JSX.IntrinsicElements, JSX.ElementClass. However, the type of an element is always JSX.Element.

Of course more interfaces could be added to the JSX namespace. However, I think that with less effort, the compiler could be made more flexible. I'd propose to check JSX elements based on a (virtual) call to JSX.createElement. For instance, the current behaviour can approximately be written like this:

namespace JSX {
  // intrinsic elements
  function createElement<K extends keyof JSX.IntrinsicElements>(tag: K, props: JSX.IntrinsicElements[K], children: JSX.Element[]): JSX.Element;
  // class and functional components
  function createElement<P>(component: (JSX.ElementClass & { new(): { props: P } }) | ((props: P) => JSX.Element), props: P, children: JSX.Element[]): JSX.Element;
}

Given that function signatures are well customisable with the use of generics for instance, most requests can be implemented this way. For instance, #13890 and #13618 would benefit from this change. Libraries that use JSX, but not on the React-way, will benefit from this too.

Proposed semantics

For a JSX element, a virtual call to JSX.createElement is constructed. It is passed three arguments:

  1. Tag name (string) in case of an intrinsic argument (<div />). Otherwise, the identifier is passed (Foo for <Foo />).
  2. An object containing the props.
  3. An array containing the children.

This should be roughly the same as the JSX transform used in the emitter phase. One notable difference is the following: in case of no properties or no children, an empty object or an empty array should be passed, instead ignoring the argument. This makes it easier to write JSX.createElement for library authors.

Backwards compatibility

For backwards compatibility, one of the following approaches could be taken:

Error reporting

When type checking the generated function call, the checker can give error messages like "Argument of type .. " or "Supplied parameters do not match any signature of call target". These messages should be replaced when checking JSX elements.

RyanCavanaugh commented 7 years ago

I think this is doable and could potentially cut out a lot of code from the compiler. When we added JSX support we didn't have spread types or keyof / T[k] so this was impossible. Behavior-wise I think this approach would allow all current behavior to be represented with the exception of data- and aria-, which would could special case to not present excess property errors.

The remaining difficulty is just technical - the overload resolution code is hardcoded in a lot of places with the assumption that it's operating over an actual argument list syntax tree rather than some abstract set of parameters.

niieani commented 7 years ago

Having this would allow one to utilize the concept of generalized JSX with TS. Currently it seems impossible, because the return type of any JSX expression is always a non-generic JSX.Element.

In fact, given this it would make sense to ditch the whole "JSX" global namespace and just use the declared type of the locally scoped jsxFactory (set in the tsconfig.json), so that the return type of a JSX expression could be different in every file, based on the implementation of the jsxFactory.

ivogabe commented 7 years ago

I think this is doable and could potentially cut out a lot of code from the compiler. When we added JSX support we didn't have spread types or keyof / T[k] so this was impossible.

That's exactly what I thought!

The remaining difficulty is just technical - the overload resolution code is hardcoded in a lot of places with the assumption that it's operating over an actual argument list syntax tree rather than some abstract set of parameters.

The compiler already handles different kinds of function calls. resolveSignature in checker.ts works with normal call expressions, but also tagged templates and decorators. Those last two don't have an explicit argument list in the source code, so I think that JSX components could be integrated there too? Actually, that function already handles JSX elements for the language service, when used with SFCs.

@niieani That's an interesting use case. I think that it would be useful in a lot of situations, other than the DOM, where some domain specific language is desired.

niieani commented 7 years ago

@ivogabe yeah, I especially like the example with ASTs, represented by JSX. So much more readable than declaring plain objects for AST.

ceymard commented 7 years ago

I'd like to ping this issue ; could it be possible that the whole jsx mechanic be simply a transform to a function call with its implied checking ?

This would allow for so much flexibility in library creation.

RyanCavanaugh commented 7 years ago

could it be possible that the whole jsx mechanic be simply a transform to a function call with its implied checking ?

No. Two reasons and there may be more:

[1]

interface Point {
    x: number;
    y: number;
}

var m: Point;
var j: Point = { z: 10, ...m }; // Not an error due to spread
niieani commented 7 years ago

@RyanCavanaugh The first could be fixed via other types of constructs that could be added to the language:

WithDomProperties<Point> -- adds typing data-* and aria-* to the type

Second we could think about. Maybe the spreads in JSX would behave differently?

There's probably something we could do. In any case, this would massively simplify the API and allow for custom JSX applications as I've mentioned in the comment above.

amir-arad commented 7 years ago

AFAIK it's not just data-* and area-*, it's *-*.

class Foo extends React.Component<{a:string}>{}

//This works:
<Foo b-3="bar" a="ggg"/>
joewood commented 6 years ago

@RyanCavanaugh now that the second point has been fixed, can this be revisited?

interface Point {
    x: number;
    y: number;
}

var m: Point;
var j: Point = { z: 10, ...m }; // NOW ERRORS
jchitel commented 6 years ago

Another problem to consider here is how to deal with backwards compatibility, particularly around the DefinitelyTyped definitions for React, which would have to be updated to support this.

Existing types that depend on the JSX namespace would no longer work because the compiler wouldn't look there anymore. The most graceful way I can see to handle that would be to modify the React definitions so that the JSX namespace definitions are dependent on (or at least structurally equivalent to) whatever types are used for React.createElement, and deprecate the JSX namespace.

For non-React use cases, this will probably just have to be a breaking change, with a suggested migration pattern. If someone is already using the JSX namespace for typing JSX, all they need to do is add a jsxFactory function (which they probably already have) and tie their JSX types to it.

I agree that having this would improve the flexibility of JSX-in-TS considerably, but the migration for existing code will probably be quite painful.

mhegazy commented 6 years ago

should be covered by https://github.com/Microsoft/TypeScript/issues/21699

Jessidhia commented 5 years ago

This proposal I just posted is probably useless if this is actually implemented -- but one thing I have convinced myself of is that the center will not hold.

I don't think it's possible to get any further improvement of React type checking, JSX included, without breaking backwards compatibility. There is way too much accumulated garbage in the types that existed because the compiler did not have enough features to represent them correctly, and now that the compiler does (or at least nearly does), fixing them would break compatibility with existing code.

I think that now that we have typesVersion we can do this. I'd be up to write new @types/react specifically for ts >= 3.3 but that'd still have to be a clean break. 3.2 would still use the current types, 3.3 would use new types, new names, new definitions, and a lot of garbage thrown away.


I've been considering that rewrite for a few days now, actually, but it'd just be an "arbitrary-looking" break from a certain point of view. If we get the compiler to use the factory function for deriving types that certainly makes a stronger case for the rewrite.

weswigham commented 5 years ago

The main typesystem feature we're missing to be able to fully type most react/jsx components without namespace shenanigans would be "call types" which can resolve calls in higher order/in types (respecting generics, inference, and overloads appropriately). Following that, expanded call resolution rules to allow more calls of unions/intersections with signatures within them is also required (see the current reports on where jsx is right now - this is probably the immediate priority.

That's really the only thing stopping me from adding the factory-function-resolver mode, since without it you can't actually accurately type the factory function, which is kind of a big deal.

m93a commented 5 years ago

Could this be added to the roadmap again? Compiler API and JS synergy are both extremely cool, and definitely more important, but this could provide some kick-ass inference with JSX children in much more than just React applications. And it's a little frustrating to see the dust falling on the PR that should have fixed this a year ago…

trusktr commented 4 years ago

Just adding my (end-user) thoughts, so the bot doesn't lock the issue:

I hope this will land sometime, so we can have code like

const div: HTMLDivElement = <div>This is a div!</div>

which after compiled would result in the div variable referencing an instance of an HTMLDivElement. The compiled output could be, for example:

const div = document.createElement('div')
div.textContent = 'This is a div!'

As a workaround, I currently do

const div = (<div>This is a div!</div>) as any as HTMLDivElement

but gotta remember to update the type when updating the tag name.

Some helper functions could make it terser, for now:

const div = div(<div>This is a div!</div>) // return type is HTMLDivElement
const el = div(<p>This is a p!</p>) // don't make this mistake!

(Is there any better workaround?)

sbussard commented 4 years ago

I'm glad to see this issue has not been closed. Sad that it hasn't been implemented though.

osdiab commented 4 years ago

Another thought on this:

Some analogs in other spaces:

My company's current use case is libraries that require specific types of children or else error at runtime, like mjml-react; having these checks happen at compile time by making instances of react components expressible by the type system would significantly tighten our development experience.

entropitor commented 3 years ago

Would it be easier to get a solution in by adding another "phantom" type to the JSX namespace so that we don't break any existing flows but the many people that need it, can type it better themselves?

devinrhode2 commented 3 years ago

So what is the recommended way to type jsx components in react? I'm not a fan of literally everything being JSX.Element, so am looking for anything that is more specific, even if it's just cosmetic.

danielo515 commented 3 years ago

Hey. I really need to be able to specify which component types are valid, so I can ensure at compile time that my components are being used as I intend them to be used. Is this really impossible with TS?

trusktr commented 3 years ago

It is not possible yet.

danielo515 commented 3 years ago

So how do you type them when, for example, you need to iterate the children array and access the props? TS will not understand that the children has that props. This is key to produce high quality components that are type safe, is it at least on the roadmap?

WilliamPeralta commented 2 years ago
interface InputProps {
    name: string
    value: any
}

interface SelectProps extends InputProps{
    items: any[]
}

export const Input =  (props : InputProps) => {
    return <div>Sono un input</div>
}

export const Select =  (props : SelectProps) => {
    return <div>Sono un Select</div>
}

type PropsOf<T> = T extends React.ComponentType<infer Props> ? Props : object

// type BPropsOf<T> = PropsOf<BaseFieldProps<T>>

type BaseFieldProps<T> = {
    field1: string
    field2: boolean
    component: React.ComponentType<PropsOf<T>>
} & {
    [Property in keyof PropsOf<T>]: PropsOf<T>[Property]
}

// type FieldProps = BaseFieldProps<InputProps>  // & PropsOf<BaseFieldProps<InputProps>['component']>

//  & PropsOf<FieldProps['component']> 
function Field < C >(props : BaseFieldProps<C>){
    return <div>Sono un field</div>
}

export const Home =  () => {
    return (
        <div>
            <h2>I am body</h2>
            <Field<typeof Select>
                field1="field1"
                field2    
                component={Select}
               {/* Here all the properties of the component */}
            />
        </div>
    )
}

There is a way to avoid having to write the generic "typeof Select" near "Field" and extract it from the prop "component"??

trusktr commented 2 years ago

@WilliamPeralta This is not a help channel for JSX component generics. This topic is specifically about the return type of JSX expressions.

I would kindly recommend to delete the post so it isn't distracting, and ask in the appropriate channels (StackOverflow, Discord, etc). You'll get way better help that way.

reverofevil commented 2 years ago

Joe, it's directly related. If JSX was typed as all other function calls, TS would be able to infer generic parameters. It's not only a return type problem, it's all about JSX being typed incorrectly.

On Sat, Dec 4, 2021, 05:17 Joe Pea @.***> wrote:

@WilliamPeralta https://github.com/WilliamPeralta This is not a help channel for JSX component generics. This topic is specifically about the return type of JSX expressions.

I would kindly recommend to delete the post so it isn't distracting, and ask in the appropriate channels (StackOverflow, Discord, etc). You'll get way better help that way.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/microsoft/TypeScript/issues/14729#issuecomment-985949340, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAWYQIIMDDKEXRQUUFQTIVLUPF22DANCNFSM4DEG63DQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

trusktr commented 2 years ago

I'm all for proper types everywhere, but this issue is about the return types. Once those are in place, this opens room for the generics to be improved.

@RyanCavanaugh As you fear the performance of JSX return types, what about a compromise? Either:

At least allow us to try the feature in a nightly release and see if it does as poorly as you say. Let us measure it!

weswigham commented 2 years ago

We kinda did with experimental builds, that's why we know the perf is so bad.

trusktr commented 2 years ago

Is it worse than nested function calls like h('div', h('p'))?

weswigham commented 2 years ago

No but yes. No, it's pretty much that exactly (slight note: class/function components are essentially two calls of cost per tag, rather than one), yes, because people don't chain generic functions 100 levels deep in practice (convention simply dictates you use a temporary variable after a few levels of nesting), but do chain jsx tags 100 levels deep.

niieani commented 2 years ago

@weswigham would it much of a burden for your team if this were to land behind a flag? Most uses might not need generic JSX, but there are plenty of niche uses that would greatly benefit from this, and 100-level deep nesting of any kind can (and should) be avoided in well crafted code anyway.

trusktr commented 2 years ago

What about return types, without inference of generics?

ConnorJamesLow commented 2 years ago

@RyanCavanaugh As you fear the performance of JSX return types, what about a compromise? Either:

  • enable with an option that defaults to disabled?
  • allow return types only for top level result of a JSX expression so at least something like const div = <div>...</div> regardless of what children the div has.

I'm voting for that second option.

flying-sheep commented 2 years ago

Any idea how to get JSX typing per file when using jsx: "react-jsx"?

Before I could just use #22207:

things.ts

namespace mything {
    export namespace JSX {
        type Element = ...
    }
}
export function mything(...) {...}

app.ts

/** @jsx mything */
import { mything } from './things'

const a = <a/>

What can I do now?

/edit: figured it out:

jsx-runtime.ts

export namespace JSX {
    type Element = ...
}
export function jsx(...) {...}

app.ts

/** @jsxImportSource . */

const a = <a/>
eps1lon commented 1 year ago

Has anybody tried to write a declaration of a JSX factory that would cover all the type-checking features JSX type-checking currently implements?

Specifically, I don't see how we can enable generic props if we construct a synthetic function call:

import * as React from "react";

interface ComponentProps<T> {
  items: T[];
  render: (item: T) => string;
}
function Component<T>(props: ComponentProps<T>) {
  return null;
}

function jsxA<Props>(elementType: (props: Props) => any, props: Props) {}
function jsxB<ElementType extends (props: any) => any>(
  elementType: ElementType,
  props: Parameters<ElementType>[0]
) {}

// props are inferred as unknown
jsxA(Component, {
  items: ["one", "tow"],
  // item is implicitly any
  render: (item) => String(item.length),
});

jsxB(Component, {
  items: ["one", "tow"],
  // item is unknown
  render: (item) => String(item.length),
});

// Works as expected
<Component
  items={["one", "two"]}
  render={(item) => {
    return String(item.length);
  }}
/>;

Playground Link

reverofevil commented 1 year ago

@eps1lon That's because TS has a buggy intentionally different implementation of unification: it generates an unknown whenever you ask to unify monotype to polytype. Verbatim translation of your code to any language that implements it correctly works.

data Elem = Elem String {- | ... -} deriving (Eq, Show)

data Props t = Props { items :: [t], render :: t -> Elem }

component :: Props t -> Elem
component _ = Elem ""

type FC p = p -> Elem

jsx :: FC p -> p -> Elem
jsx f p = f p

main = print $ jsx component (Props ["one", "two"] (\item -> Elem (show (length item))))

Playground link

TylorS commented 1 year ago

I've been following various JSX threads over the years all boiling down to this, I really truly hope this lands in TS at some point even if not 5.1 directly.

As you fear the performance of JSX return types, what about a compromise? Either:

  • enable with an option that defaults to disabled?
    • allow return types only for top level result of a JSX expression so at least something like const div =
      ...
      regardless of what children the div has.

In response to this question, I sincerely hope for it to be 1. The type-safety of these functions seems most important over performance issues. The Effect community (formerly fp-ts) and I are building libraries today for html rendering and are today forced to use tagged template literals to be able to track typed resources and typed errors of children to be able to aggregate the resources required to run them and the errors they might produce as unions.

If JSX uses createElement as any other function for type-checking, we'd be able to utilize JSX's AST to further optimize them using compiler techniques and provide much more type-safe APIs for properties which is a lot more cumbersome to do with template literal strings and requires language service/server plugins to work as expected.

TylorS commented 1 year ago

@RyanCavanaugh You closed this with #51328, but it doesn't at all lookup the types by the signature of createElement. I think this should remain open

yairopro commented 1 year ago

I just add here a pure sample code of the problem:

https://codesandbox.io/s/jsx-element-loses-types-6nmh3m?file=/src/App.tsx

ghost commented 1 year ago

Maybe there's something I don't get here. Why is it not possible to allow JSX.Element to be generic? Simply passing it the JSX type (the literal string in the case of intrinsic elements and the type of the symbol in the case of value-based elements) would solve a lot of issues). Something like the following could be done:

I'm new here, so maybe I'm missing something. I do know that this approach might have some performance issues similar to the one discussed here.

Also, does Typescript always pass the key as the third parameter to the factory function when compiled? I'm not sure.

ghost commented 1 year ago

It could be an opt-in feature behind a TSConfig option since it'll break existing code.

ghost commented 1 year ago

So, @MartinJohns and @xxaff, do you think it's a bad idea? I'll like to learn. I'm new to the TS world.

arthurfiorette commented 1 year ago

This was planned for milestone 5.1. Any updates here?

frzi commented 5 months ago

Is there still any interest or plans for this? This seems like an incredible feature that’ll make libraries utilizing JSX far more versatile. 😄

robbiespeed commented 5 months ago

Here's a couple TS Playground examples of how we could use this feature to enable similar features to flow's "renders" types and limiting allowed children or other props that receive jsx: