Edit: We're likely not doing that in 17 but 18 to make the 17 upgrade as easy as possible. See https://github.com/DefinitelyTyped/DefinitelyTyped/issues/46691#issuecomment-672840456 for more context.
Among active contributors we talked informally about making some breaking changes to @types/react in an upcoming major release of React that we think will improve the typings long-term. Now that a release candidate for React 17 is available we should formalize what breaking changes we want to make.
GitHub issues is not the best place for discussing umbrella topics (no threading) but hopefully this will do.
@deprecated typestslintReactTypeSFCElementSFCFactorySFCStatelessComponentRefForwardingComponentPropschildren from React.FCany with unknown which is probably the change with the biggest impact.event.persist()?~@gaearon Should we hold off with removing deprecated React features? We currently flag the following features as deprecated:
UNSAFE_ prefixed lifecycles)this.refs i.e. string refsenterKeyHintdisableRemotePlaybackSee #46690 where we can test the approaches. Ideally @types would understand pre-releases but I couldn't find documentation about it so we have to be practical about the approach chosen.
@types/react authors@johnnyreilly, @bbenezech, @pzavolinsky, @digiguru, @ericanderson, @DovydasNavickas, @theruther4d, @guilhermehubner, @ferdaber, @jrakotoharisoa, @pascaloliv, @hotell, @franklixuefei, @Jessidhia, @saranshkataria, @lukyth, @eps1lon, @zieka, @dancerphil, @dimitropoulos, @disjukr, @vhfmag, @hellatan
I think one change we should consider is finally moving the JSX namespace under React.createElement. This will finally allow consumers to use multiple JSX compilers in the same codebase without the intrinsic elements colliding with each other.
From the TS docs:
The factory chosen will also affect where the JSX namespace is looked up (for type checking information) before falling back to the global one. If the factory is defined as React.createElement (the default), the compiler will check for React.JSX before checking for a global JSX. If the factory is defined as h, it will check for h.JSX before a global JSX.
Remove event.persist()?
It hasn’t been deleted, just doesn’t do anything now. So I don’t think it should be removed.
One important note about this release that might not be obvious from the blog post.
It is very important that React 17 is easy to upgrade to because for legacy screens in huge apps it might be the last release they will upgrade to.
This is why even already announced deprecations (like removing UNSAFE versions, legacy context, renderSubtree) are not included in this release. They will be in 18 instead.
The goal of 17 is to fix event delegation so that Gradual Upgrades become a feasible strategy for legacy apps when React 18 comes out. So any extra breaking changes in React 17 defeat the purpose.
Can we postpone any major breakages to 18 in typings as well?
Can we postpone any major breakages to 18 in typings as well?
Works for me. Though at some point the typings have to include some breaking changes exclusive to types.
The way TypeScript works might also prevent having multiple major versions of react (with their respective typings) in a codebase anyway (since they affect the global namespace). So that is definitely something we should investigate in the coming months. We might have to release a breaking change during the v17 lifetime in order to support multiple react versions with their typings in a codebase.
Edit: though your particular demo should work as far as I can tell. It might get problematic if people use package aliases from yarn though. But these could cause other problems as well.
Seems like porting https://github.com/reactjs/react-gradual-upgrade-demo to TS and figuring out what the issues are is going to be the next step.
Regardless of when we release this (with 17 vs with 18), we should probably start work on this fork anyway, so we can be better prepared when the time comes. We unfortunately cannot add runtime warnings (only deprecation annotations which aren't always possible to make obvious in an IDE) so we don't have the advantage of being able to warn users effectively.
Could we also consider fixing https://github.com/DefinitelyTyped/DefinitelyTyped/issues/35622, whenever we make the other breaking changes mentioned here?
Could we also consider fixing #35622, whenever we make the other breaking changes mentioned here?
I think that would be fixed by removing implicit children. That one is already included in the list.
Hmm, but what if you still wanted to allow any ReactNode as a child, e.g. you specified children: ReactNode explicitly?
Hmm, but what if you still wanted to allow any
ReactNodeas a child, e.g. you specifiedchildren: ReactNodeexplicitly?
You could still do that which we usually recommend. By removing implicit children any children type that is too wide is responsibility of the typings author and no longer ours.
We would still need a type that represents a valid React child node. ReactNode should be that but currently it includes {} which makes it useless, since that type is assignable to lots of things. As a breaking change I would like to see {} removed so we can actually rely on ReactNode when we want our components to accept children of type ReactElement, fragment, boolean, string, number or null.
As per the discussion in the original issue, I think we would also need to do this to fix child type checking of host components.
but currently it includes {}
Right, forgot about that. Let's discuss a possible path forward in https://github.com/DefinitelyTyped/DefinitelyTyped/issues/35622#issuecomment-673378489 and then include it here.
I would like to see adjustments for the FC/FunctionComponent and class component render return type as well.
In their current form, these types can either lead to run-time errors or are too restrictive compared to the possible vanilla React return values (the link is about render in classes, though the same applies for function components).
FC type only accepts JSX.Element | null. boolean, string, number and Array are currently missing.render accepts ReactNode, which is too wide.ReactNode accepts a wide range of types (due to {}). So if you return undefined or arbitrary object values (which is possible with types) by accident, run-time errors are triggered later on.
I would consider an explicit RenderReturnType type useful and practicable, like:
type RenderReturnType = ReactChild | boolean | null | Array<RenderReturnType>;
class Component<P, S> {
render(): RenderReturnType;
// ...
}
interface FunctionComponent<P = {}> {
(props: PropsWithChildren<P>, context?: any): RenderReturnType;
// ...
}
// common existing types
type ReactText = string | number;
type ReactChild = ReactElement | ReactText;
interface ReactNodeArray extends Array<ReactNode> {}
type ReactFragment = {} | ReactNodeArray;
Maybe there is also potential to combine RenderReturnType with ReactNode, if {} is excluded, as proposed by @OliverJAsh . But I am unsure about the original intent to introduce ReactNode. Also: ReactNode includes undefined, which is invalid.
Hence I propose an explicit type for function and class component return values. What do you think?
@bela53 We can't do anything about the return type of components. TypeScript restricts these. We need https://github.com/microsoft/TypeScript/issues/21699 first.
@eps1lon thanks for the link - certainly helps to understand that the issue is more compiler related. The FC type definition seems to be hardwired into the compiler itself (e.g. changing the type definitions of the react .d.ts file has no effect). I mistakenly thought, this had changed since TS 2.8 local JSX namespaces.
Though microsoft/TypeScript#21699 has a bigger scope than my suggestion, as it intends to implement custom JSX types looked up dynamically by inferring from the JSX factory function.
Since the issue is stale since long time, I would be just happy, if they change static render return types in accordance with vanilla React like mentioned above. I wonder why this already hasn't been done with a breaking change.
I'm not sure what you mean by "static" though, the allowed return types are dictated by the factory function itself— React allows some subset of types, and maybe something like Vue (where fragments are not yet supported) does not allow returning arrays yet.
Here is a simple example to illustrate what I mean by static:
import React from "react"
const MyComp = () => "foo" // valid React comp
const jsx = <MyComp /> // errors, function component return type is statically fixed to JSX.Element | null
The default jsx factory is React.createElement, whose overload for above function is looked up in React namespace:
function createElement<P extends {}>(
type: FunctionComponent<P>,
props?: Attributes & P | null,
...children: ReactNode[]): FunctionComponentElement<P>;
Now, even if I change the FunctionComponent definition inside index.d.ts of my local @types/react to something like:
interface FunctionComponent<P = {}> {
(props: PropsWithChildren<P>, context?: any): ReactElement<any, any> | null | string; // add string
}
, it won't be respected and the error still occurs.
This let me assume, there is one static, hard wired place, containing the FC return type JSX.Element | null. If I remember correctly, other issues mentioned this as well.
Yes, the technical procedure is that the compiler covariantly checks that the return type of a specific JSX constructor is assignable to the JSX.Element type (which can be defined differently for a specific factory function).
The issue is that JSX.Element _has_ to be black boxed to an object since you need to sometimes access the VDOM object, because the return value of a JSX expression is deferred through that VDOM object.
const Foo = () => 'hello world!'
const foo = <Foo /> // this type HAS to be an object in the React world, because:
const fooProps = foo.props
const fooKey = foo.key
I see, thank you. So basically the crux is, there is no distinction between a JSX expression type and the component return type, which leads to microsoft/TypeScript#21699 again.
The compiler looks up the jsx factory function React.createElement, sees FunctionComponent as type parameter and enforces its return type as JSX.Element, which is defined in the global JSX namespace. There is no room for other values like string, number or Array, JSX.Element must be an object, as you say. If I understand correctly, the RFC strives to read the FunctionComponent type at hand from the createElement overload parameter, so we are able to keep it more specific.
Fun fact: With following definitions in @types/react/index.d.ts, string is possible with current types (but doesn't feel right):
declare global {
namespace JSX {
type Element = React.ReactElement<any, any> | string
// ...
}
Sorry for somewhat having hijacked the issue :-). Let's root for the TS one to be (hopefully) resolved soon TM.
Yup! That's correct, and what you did would be the correct choice in "fixing" this (you /have/ to change the type of JSX.Element instead of the return type of function components), but the issue would be that now you can no longer access the VDOM properties of a generated JSX element (which admittedly is a far rarer use case but must still be valid)
I would like to see default type parameters for react HTML elements. I don't believe this is a breaking change.
All the React specific HTML element types take a T as their type parameter with no default types. This causes any library users to provide the default HTML element type at every usage. These should likely default to their respective HTML Element types.
For example why should you have to provide the HTML element type:
type In = React.InputHTMLAttributes<HTMLInputElement>
When you already know you are dealing with an Input HTML element?
type In = React.InputHTMLAttributes
Here is an example of the change: https://github.com/jleider/DefinitelyTyped/pull/9
Most helpful comment
I would like to see adjustments for the
FC/FunctionComponentand class componentrenderreturn type as well.In their current form, these types can either lead to run-time errors or are too restrictive compared to the possible vanilla React return values (the link is about
renderin classes, though the same applies for function components).Status quo:
FCtype only acceptsJSX.Element | null.boolean,string,numberandArrayare currently missing.renderacceptsReactNode, which is too wide.ReactNodeaccepts a wide range of types (due to{}). So if you returnundefinedor arbitrary object values (which is possible with types) by accident, run-time errors are triggered later on.Possible solution
I would consider an explicit
RenderReturnTypetype useful and practicable, like:Maybe there is also potential to combine
RenderReturnTypewithReactNode, if{}is excluded, as proposed by @OliverJAsh . But I am unsure about the original intent to introduceReactNode. Also:ReactNodeincludesundefined, which is invalid.Hence I propose an explicit type for function and class component return values. What do you think?