Skip to content

Commit

Permalink
🤖 Merge PR #65135 [react] Allow returning ReactNode from function com…
Browse files Browse the repository at this point in the history 
…ponents by @eps1lon

* [react] Allow returning ReactNode from function components

* [react] Ignore statics from element type checking

would require a lot of work to fix the issues in the consuming libraries
(e.g. rc-easyui, moxy__next-router-scroll etc)
.propTypes assignablity is ultimately not important here.

* Add JSX.ElementType to scoped namespace

* Add JSX.ElementType to automatic runtime
  • Loading branch information
Sebastian Silbermann committed Jun 1, 2023
1 parent 748cf88 commit 443451c
Show file tree
Hide file tree
Showing 6 changed files with 42 additions and 36 deletions.
27 changes: 21 additions & 6 deletions types/react/index.d.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ declare namespace React {
* @deprecated https://legacy.reactjs.org/docs/legacy-context.html#referencing-context-in-stateless-function-components
*/
deprecatedLegacyContext?: any,
) => ReactElement<any, any> | null)
) => ReactNode)
| (new (props: P) => Component<any, any>);

interface RefObject<T> {
Expand Down Expand Up @@ -113,7 +113,7 @@ declare namespace React {
C extends
| ForwardRefExoticComponent<any>
| { new (props: any): Component<any> }
| ((props: any, context?: any) => ReactElement | null)
| ((props: any, context?: any) => ReactNode)
| keyof JSX.IntrinsicElements
> =
// need to check first if `ref` is a valid prop for ts@3.0
Expand Down Expand Up @@ -380,7 +380,7 @@ declare namespace React {
/**
* **NOTE**: Exotic components are not callable.
*/
(props: P): (ReactElement|null);
(props: P): ReactNode;
readonly $$typeof: symbol;
}

Expand Down Expand Up @@ -550,7 +550,7 @@ declare namespace React {
type FC<P = {}> = FunctionComponent<P>;

interface FunctionComponent<P = {}> {
(props: P, context?: any): ReactElement<any, any> | null;
(props: P, context?: any): ReactNode;
propTypes?: WeakValidationMap<P> | undefined;
contextTypes?: ValidationMap<any> | undefined;
defaultProps?: Partial<P> | undefined;
Expand All @@ -566,7 +566,7 @@ declare namespace React {
* @deprecated - Equivalent with `React.FunctionComponent`.
*/
interface VoidFunctionComponent<P = {}> {
(props: P, context?: any): ReactElement<any, any> | null;
(props: P, context?: any): ReactNode;
propTypes?: WeakValidationMap<P> | undefined;
contextTypes?: ValidationMap<any> | undefined;
defaultProps?: Partial<P> | undefined;
Expand All @@ -576,7 +576,7 @@ declare namespace React {
type ForwardedRef<T> = ((instance: T | null) => void) | MutableRefObject<T | null> | null;

interface ForwardRefRenderFunction<T, P = {}> {
(props: P, ref: ForwardedRef<T>): ReactElement | null;
(props: P, ref: ForwardedRef<T>): ReactNode;
displayName?: string | undefined;
// explicit rejected with `never` required due to
// https://github.com/microsoft/TypeScript/issues/36826
Expand Down Expand Up @@ -3125,7 +3125,9 @@ declare namespace React {
componentStack: string;
}

// Keep in sync with JSX namespace in ./jsx-runtime.d.ts and ./jsx-dev-runtime.d.ts
namespace JSX {
type ElementType = GlobalJSXElementType;
interface Element extends GlobalJSXElement {}
interface ElementClass extends GlobalJSXElementClass {}
interface ElementAttributesProperty extends GlobalJSXElementAttributesProperty {}
Expand Down Expand Up @@ -3188,6 +3190,18 @@ declare global {
* @deprecated Use `React.JSX` instead of the global `JSX` namespace.
*/
namespace JSX {
// We don't just alias React.ElementType because React.ElementType
// historically does more than we need it to.
// E.g. it also contains .propTypes and so TS also verifies the declared
// props type does match the declared .propTypes.
// But if libraries declared their .propTypes but not props type,
// or they mismatch, you won't be able to use the class component
// as a JSX.ElementType.
// We could fix this everywhere but we're ultimately not interested in
// .propTypes assignability so we might as well drop it entirely here to
// reduce the work of the type-checker.
// TODO: Check impact of making React.ElementType<P = any> = React.JSXElementConstructor<P>
type ElementType = string | React.JSXElementConstructor<any>;
interface Element extends React.ReactElement<any, any> { }
interface ElementClass extends React.Component<any> {
render(): React.ReactNode;
Expand Down Expand Up @@ -3394,6 +3408,7 @@ declare global {
// React.JSX needs to point to global.JSX to keep global module augmentations intact.
// But we can't access global.JSX so we need to create these aliases instead.
// Once the global JSX namespace will be removed we replace React.JSX with the contents of global.JSX
type GlobalJSXElementType = JSX.ElementType;
interface GlobalJSXElement extends JSX.Element {}
interface GlobalJSXElementClass extends JSX.ElementClass {}
interface GlobalJSXElementAttributesProperty extends JSX.ElementAttributesProperty {}
Expand Down
1 change: 1 addition & 0 deletions types/react/jsx-dev-runtime.d.ts
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import * as React from './';

export namespace JSX {
type ElementType = React.JSX.ElementType;
interface Element extends React.JSX.Element {}
interface ElementClass extends React.JSX.ElementClass {}
interface ElementAttributesProperty extends React.JSX.ElementAttributesProperty {}
Expand Down
1 change: 1 addition & 0 deletions types/react/jsx-runtime.d.ts
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import * as React from './';

export namespace JSX {
type ElementType = React.JSX.ElementType;
interface Element extends React.JSX.Element {}
interface ElementClass extends React.JSX.ElementClass {}
interface ElementAttributesProperty extends React.JSX.ElementAttributesProperty {}
Expand Down
3 changes: 0 additions & 3 deletions types/react/test/experimental.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -176,17 +176,14 @@ function Optimistic() {

function elementTypeTests() {
const ReturnPromise = () => Promise.resolve('React');
// @ts-expect-error Needs https://github.com/DefinitelyTyped/DefinitelyTyped/pull/65135
const FCPromise: React.FC = ReturnPromise;
class RenderPromise extends React.Component {
render() {
return Promise.resolve('React');
}
}

// @ts-expect-error Needs https://github.com/DefinitelyTyped/DefinitelyTyped/pull/65135
<ReturnPromise />;
// @ts-expect-error Needs https://github.com/DefinitelyTyped/DefinitelyTyped/pull/65135
React.createElement(ReturnPromise);
<RenderPromise />;
React.createElement(RenderPromise);
Expand Down
10 changes: 1 addition & 9 deletions types/react/test/index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -218,14 +218,6 @@ FunctionComponent2.defaultProps = {
foo: 42
};

// allows null as props
const FunctionComponent4: React.FunctionComponent = props => null;

// undesired: Rejects `false` because of https://github.com/DefinitelyTyped/DefinitelyTyped/issues/18051
// leaving here to document limitation and inspect error message
// @ts-expect-error
const FunctionComponent5: React.FunctionComponent = () => false;

// React.createFactory
const factory: React.CFactory<Props, ModernComponent> =
React.createFactory(ModernComponent);
Expand All @@ -250,7 +242,7 @@ const element: React.CElement<Props, ModernComponent> = React.createElement(Mode
const elementNoState: React.CElement<Props, ModernComponentNoState> = React.createElement(ModernComponentNoState, props);
const elementNullProps: React.CElement<{}, ModernComponentNoPropsAndState> = React.createElement(ModernComponentNoPropsAndState, null);
const functionComponentElement: React.FunctionComponentElement<SCProps> = React.createElement(FunctionComponent, scProps);
const functionComponentElementNullProps: React.FunctionComponentElement<SCProps> = React.createElement(FunctionComponent4, null);
const functionComponentElementNullProps: React.FunctionComponentElement<SCProps> = React.createElement(FunctionComponent2, null);
const domElement: React.DOMElement<React.HTMLAttributes<HTMLDivElement>, HTMLDivElement> = React.createElement("div");
const domElementNullProps = React.createElement("div", null);
const htmlElement = React.createElement("input", { type: "text" });
Expand Down
36 changes: 18 additions & 18 deletions types/react/test/tsx.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -612,40 +612,48 @@ function reactNodeTests() {

function elementTypeTests() {
const ReturnVoid = () => {};
// @ts-expect-error
const FCVoid: React.FC = ReturnVoid;
class RenderVoid extends React.Component {
// @ts-expect-error
render() {}
}

const ReturnUndefined = () => undefined;
const FCUndefined: React.FC = ReturnUndefined;
class RenderUndefined extends React.Component {
render() {
return undefined;
}
}

const ReturnNull = () => null;
const FCNull: React.FC = ReturnNull;
class RenderNull extends React.Component {
render() {
return null;
}
}

const ReturnNumber = () => 0xeac1;
const FCNumber: React.FC = ReturnNumber;
class RenderNumber extends React.Component {
render() {
return 0xeac1;
}
}

const ReturnString = () => 'Hello, Dave!';
const FCString: React.FC = ReturnString;
class RenderString extends React.Component {
render() {
return 'Hello, Dave!';
}
}

const ReturnSymbol = () => Symbol.for('react');
// @ts-expect-error
const FCSymbol: React.FC = ReturnSymbol;
class RenderSymbol extends React.Component {
// @ts-expect-error
render() {
Expand All @@ -654,28 +662,31 @@ function elementTypeTests() {
}

const ReturnArray = () => [<div key="one" />];
const FCVArray: React.FC = ReturnArray;
class RenderArray extends React.Component {
render() {
return [<div key="one" />];
}
}

const ReturnElement = () => <div />;
const FCElement: React.FC = ReturnElement;
class RenderElement extends React.Component {
render() {
return <div />;
}
}

const ReturnReactNode = ({children}: {children?: React.ReactNode}) => children;
const FCReactNode: React.FC = ReturnReactNode;
class RenderReactNode extends React.Component<{children?: React.ReactNode}> {
render() {
return this.props.children;
}
}

const ReturnPromise = () => Promise.resolve('React');
// @ts-expect-error experimental release channel only
// Will not type-check in a real project but accepted in DT tests since experimental.d.ts is part of compilation.
const FCPromise: React.FC = ReturnPromise;
class RenderPromise extends React.Component {
// Will not type-check in a real project but accepted in DT tests since experimental.d.ts is part of compilation.
Expand Down Expand Up @@ -703,10 +714,8 @@ function elementTypeTests() {
// @ts-expect-error
React.createElement(RenderVoid);

// Undesired behavior. Returning `undefined` should be accepted in all forms.
// @ts-expect-error
// Desired behavior.
<ReturnUndefined />;
// @ts-expect-error
React.createElement(ReturnUndefined);
<RenderUndefined />;
React.createElement(RenderUndefined);
Expand All @@ -717,18 +726,14 @@ function elementTypeTests() {
<RenderNull />;
React.createElement(RenderNull);

// Undesired behavior. Returning `number` should be accepted in all forms.
// @ts-expect-error
// Desired behavior.
<ReturnNumber />;
// @ts-expect-error
React.createElement(ReturnNumber);
<RenderNumber />;
React.createElement(RenderNumber);

// Undesired behavior. Returning `string` should be accepted in all forms.
// @ts-expect-error
// Desired behavior.
<ReturnString />;
// @ts-expect-error
React.createElement(ReturnString);
<RenderString />;
React.createElement(RenderString);
Expand All @@ -743,10 +748,7 @@ function elementTypeTests() {
// @ts-expect-error
React.createElement(RenderSymbol);

// Undesired behavior. Returning `Array` should be accepted in all forms.
// @ts-expect-error
<ReturnArray />;
// @ts-expect-error
React.createElement(ReturnArray);
<RenderArray />;
React.createElement(RenderArray);
Expand All @@ -757,17 +759,15 @@ function elementTypeTests() {
<RenderElement />;
React.createElement(RenderElement);

// Undesired behavior. Returning `ReactNode` should be accepted in all forms.
// @ts-expect-error
// Desired behavior.
<ReturnReactNode />;
// @ts-expect-error
React.createElement(ReturnReactNode);
<RenderReactNode />;
React.createElement(RenderReactNode);

// @ts-expect-error Only available in experimental release channel
// Will not type-check in a real project but accepted in DT tests since experimental.d.ts is part of compilation.
<ReturnPromise />;
// @ts-expect-error Only available in experimental release channel
// Will not type-check in a real project but accepted in DT tests since experimental.d.ts is part of compilation.
React.createElement(ReturnPromise);
// Will not type-check in a real project but accepted in DT tests since experimental.d.ts is part of compilation.
<RenderPromise />;
Expand Down

7 comments on commit 443451c

@httpete
Copy link

@httpete httpete commented on 443451c Jun 2, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Wow, this broke my entire ts:check step. Is anyone else seeing a spray of errors like this after updating to 18.2.8? This is with the latest TS 5.1.3. These components are declared with the regular React.FC typing.

renderEntryComponent.tsx:46:8 - error TS2786: 'EntryComponent' cannot be used as a JSX component.
  Its return type 'ReactNode' is not a valid JSX element.

@eps1lon
Copy link
Collaborator

@eps1lon eps1lon commented on 443451c Jun 3, 2023

Choose a reason for hiding this comment

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

Are you seeing these when running tsc or in your IDE? Could you share a minimal, cloneable repro to investigate?

@httpete
Copy link

@httpete httpete commented on 443451c Jun 3, 2023

Choose a reason for hiding this comment

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

After looking more closely, @types/react: "18.2.8" works fine with TS 5.0.4. When you switch to 5.1.3, it throws the error.

18.2.7 works fine with TS 5.1.3. The incompatibility is the combo of 18.2.8 and TS 5.1.3 (both latest).

@leozhao0709
Copy link
Contributor

Choose a reason for hiding this comment

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

When I use 18.2.8 and TS 5.1.3,

When use StrictModel, it shows error: 'StrictMode' cannot be used as a JSX component.
When use React.FC, it shows error: Its return type 'ReactNode' is not a valid JSX element. Type 'undefined' is not assignable to type 'Element | null'.

But when switch to the combo of 18.2.8 and TS 5.0.4, then above error disappears.

@axel-planfox
Copy link

Choose a reason for hiding this comment

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

Unfortunately, this change breaks using renderToString and renderToStaticMarkup, which still expect ReactElement instead of ReactNode.

@MikkCZ
Copy link

@MikkCZ MikkCZ commented on 443451c Jun 5, 2023

Choose a reason for hiding this comment

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

Wow, this broke my entire ts:check step. Is anyone else seeing a spray of errors like this after updating to 18.2.8? This is with the latest TS 5.1.3. These components are declared with the regular React.FC typing.

renderEntryComponent.tsx:46:8 - error TS2786: 'EntryComponent' cannot be used as a JSX component.
  Its return type 'ReactNode' is not a valid JSX element.

I am seeing this in MikkCZ/pontoon-addon#685, using TypeScript 5.1.3. Seems like rendering a component like this makes tsc unhappy.

const Child: React.FC<React.ComponentProps<'button'>> = (props) => (
  <button {...props} />
);

const RenderedParent: React.FC<React.ComponentProps<typeof Child>> = (props) => (
  <Link {...props} />
);

createRoot(document.getElementById('root')).render(
  <React.StrictMode><RenderedParent /></React.StrictMode>,
);

Maybe even simpler example can be made. I haven't actually tried.

@bclements
Copy link

@bclements bclements commented on 443451c Jun 5, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

+1 Having same issue after upgrading to 18.2.8 and TS 5.1.3

Please sign in to comment.