[react] Ensure forwards compatible pattern for typing ref is used (D…

…efinitelyTyped#68720) Co-authored-by: Matt Pocock <mattpocockvoice@gmail.com>
1EDExg0ffyXfTEqdIUAYNZGnCeajIxMWd2vaQeP · Feb 29, 2024 · 2bcc7e7 · 2bcc7e7
1 parent df158f2
commit 2bcc7e7
Show file tree

Hide file tree

Showing 9 changed files with 117 additions and 37 deletions.
diff --git a/types/react-blessed/react-blessed-tests.tsx b/types/react-blessed/react-blessed-tests.tsx
@@ -211,11 +211,9 @@ const boxRef = React.createRef<ReactBlessed.BoxElement>();
 
 <ForwardRef ref={boxFnRef} />;
 <ForwardRef ref={boxRef} />;
-// @ts-expect-error
 <ForwardRef ref="string" />;
 <ForwardRef2 ref={boxFnRef} />;
 <ForwardRef2 ref={boxRef} />;
-// @ts-expect-error
 <ForwardRef2 ref="string" />;
 
 const newContextRef = React.createRef<NewContext>();
@@ -225,7 +223,6 @@ const newContextRef = React.createRef<NewContext>();
 
 const ForwardNewContext = React.forwardRef((_props: {}, ref?: React.Ref<NewContext>) => <NewContext ref={ref} />);
 <ForwardNewContext ref={newContextRef} />;
-// @ts-expect-error
 <ForwardNewContext ref="string" />;
 
 const ForwardRef3 = React.forwardRef(

diff --git a/types/react-redux/react-redux-tests.tsx b/types/react-redux/react-redux-tests.tsx
@@ -1729,8 +1729,7 @@ function testRef() {
     // Should be able to pass modern refs to a ForwardRefExoticComponent
     const modernRef: React.Ref<string> | undefined = undefined;
     <ConnectedForwardedFunctionalComponent ref={modernRef}></ConnectedForwardedFunctionalComponent>;
-    // Should not be able to use legacy string refs
-    // @ts-expect-error
+    // Should be able to use legacy string refs
     <ConnectedForwardedFunctionalComponent ref={""}></ConnectedForwardedFunctionalComponent>;
     // ref type should agree with type of the forwarded ref
     // @ts-expect-error

diff --git a/types/react/index.d.ts b/types/react/index.d.ts
@@ -189,6 +189,7 @@ declare namespace React {
      * <div ref="myRef" />
      * ```
      */
+    // TODO: Remove the string ref special case from `PropsWithRef` once we remove LegacyRef
     type LegacyRef<T> = string | Ref<T>;
 
     /**
@@ -217,9 +218,10 @@ declare namespace React {
     > =
         // need to check first if `ref` is a valid prop for ts@3.0
         // otherwise it will infer `{}` instead of `never`
-        "ref" extends keyof ComponentPropsWithRef<C> ? NonNullable<ComponentPropsWithRef<C>["ref"]> extends Ref<
+        "ref" extends keyof ComponentPropsWithRef<C>
+            ? NonNullable<ComponentPropsWithRef<C>["ref"]> extends RefAttributes<
                 infer Instance
-            > ? Instance
+            >["ref"] ? Instance
             : never
             : never;
 
@@ -232,9 +234,55 @@ declare namespace React {
      */
     type Key = string | number | bigint;
 
+    /**
+     * @internal The props any component can receive.
+     * You don't have to add this type. All components automatically accept these props.
+     * ```tsx
+     * const Component = () => <div />;
+     * <Component key="one" />
+     * ```
+     *
+     * WARNING: The implementation of a component will never have access to these attributes.
+     * The following example would be incorrect usage because {@link Component} would never have access to `key`:
+     * ```tsx
+     * const Component = (props: React.Attributes) => props.key;
+     * ```
+     */
     interface Attributes {
         key?: Key | null | undefined;
     }
+    /**
+     * The props any component accepting refs can receive.
+     * Class components, built-in browser components (e.g. `div`) and forwardRef components can receive refs and automatically accept these props.
+     * ```tsx
+     * const Component = forwardRef(() => <div />);
+     * <Component ref={(current) => console.log(current)} />
+     * ```
+     *
+     * You only need this type if you manually author the types of props that need to be compatible with legacy refs.
+     * ```tsx
+     * interface Props extends React.RefAttributes<HTMLDivElement> {}
+     * declare const Component: React.FunctionComponent<Props>;
+     * ```
+     *
+     * Otherwise it's simpler to directly use {@link Ref} since you can safely use the
+     * props type to describe to props that a consumer can pass to the component
+     * as well as describing the props the implementation of a component "sees".
+     * {@link RefAttributes} is generally not safe to describe both consumer and seen props.
+     *
+     * ```tsx
+     * interface Props extends {
+     *   ref?: React.Ref<HTMLDivElement> | undefined;
+     * }
+     * declare const Component: React.FunctionComponent<Props>;
+     * ```
+     *
+     * WARNING: The implementation of a component will not have access to the same type in versions of React supporting string refs.
+     * The following example would be incorrect usage because {@link Component} would never have access to a `ref` with type `string`
+     * ```tsx
+     * const Component = (props: React.RefAttributes) => props.ref;
+     * ```
+     */
     interface RefAttributes<T> extends Attributes {
         /**
          * Allows getting a ref to the component instance.
@@ -243,21 +291,13 @@ declare namespace React {
          *
          * @see {@link https://react.dev/learn/referencing-values-with-refs#refs-and-the-dom React Docs}
          */
-        ref?: Ref<T> | undefined;
+        ref?: LegacyRef<T> | undefined;
     }
 
     /**
      * Represents the built-in attributes available to class components.
      */
-    interface ClassAttributes<T> extends Attributes {
-        /**
-         * Allows getting a ref to the component instance.
-         * Once the component unmounts, React will set `ref.current` to `null`
-         * (or call the ref with `null` if you passed a callback ref).
-         *
-         * @see {@link https://react.dev/learn/referencing-values-with-refs#refs-and-the-dom React Docs}
-         */
-        ref?: LegacyRef<T> | undefined;
+    interface ClassAttributes<T> extends RefAttributes<T> {
     }
 
     /**
@@ -1522,6 +1562,7 @@ declare namespace React {
         P extends any ? ("ref" extends keyof P ? Omit<P, "ref"> : P) : P;
     /** Ensures that the props do not include string ref, which cannot be forwarded */
     type PropsWithRef<P> =
+        // Note: String refs can be forwarded. We can't fix this bug without breaking a bunch of libraries now though.
         // Just "P extends { ref?: infer R }" looks sufficient, but R will infer as {} if P is {}.
         "ref" extends keyof P
             ? P extends { ref?: infer R | undefined }

diff --git a/types/react/package.json b/types/react/package.json
@@ -180,6 +180,10 @@
         {
             "name": "Dmitry Semigradsky",
             "githubUsername": "Semigradsky"
+        },
+        {
+            "name": "Matt Pocock",
+            "githubUsername": "mattpocock"
         }
     ]
 }
diff --git a/types/react/test/index.ts b/types/react/test/index.ts
@@ -446,8 +446,11 @@ const ForwardingRefComponent2 = React.forwardRef<HTMLElement>((props, ref) => {
         ref(e: HTMLDivElement) {
             if (typeof ref === "function") {
                 ref(e);
-            } else if (ref) {
+            } else if (typeof ref === "object" && ref !== null) {
                 ref.current = e;
+            } else {
+                // $ExpectType null
+                ref;
             }
         },
     });

diff --git a/types/react/test/tsx.tsx b/types/react/test/tsx.tsx
@@ -454,11 +454,9 @@ const badlyAuthoredRef: React.RefObject<HTMLDivElement | null | undefined> = { c
 
 <ForwardRef ref={divFnRef} />;
 <ForwardRef ref={divRef} />;
-// @ts-expect-error
 <ForwardRef ref="string" />;
 <ForwardRef2 ref={divFnRef} />;
 <ForwardRef2 ref={divRef} />;
-// @ts-expect-error
 <ForwardRef2 ref="string" />;
 // @ts-expect-error Undesired behavior
 <ForwardRef2 ref={badlyAuthoredRef} />;
@@ -482,7 +480,6 @@ const newContextRef = React.createRef<NewContext>();
 
 const ForwardNewContext = React.forwardRef((_props: {}, ref?: React.Ref<NewContext>) => <NewContext ref={ref} />);
 <ForwardNewContext ref={newContextRef} />;
-// @ts-expect-error
 <ForwardNewContext ref="string" />;
 
 const ForwardRef3 = React.forwardRef(

diff --git a/types/react/ts5.0/index.d.ts b/types/react/ts5.0/index.d.ts
@@ -186,6 +186,7 @@ declare namespace React {
      * <div ref="myRef" />
      * ```
      */
+    // TODO: Remove the string ref special case from `PropsWithRef` once we remove LegacyRef
     type LegacyRef<T> = string | Ref<T>;
 
     /**
@@ -214,9 +215,10 @@ declare namespace React {
     > =
         // need to check first if `ref` is a valid prop for ts@3.0
         // otherwise it will infer `{}` instead of `never`
-        "ref" extends keyof ComponentPropsWithRef<C> ? NonNullable<ComponentPropsWithRef<C>["ref"]> extends Ref<
+        "ref" extends keyof ComponentPropsWithRef<C>
+            ? NonNullable<ComponentPropsWithRef<C>["ref"]> extends RefAttributes<
                 infer Instance
-            > ? Instance
+            >["ref"] ? Instance
             : never
             : never;
 
@@ -230,30 +232,66 @@ declare namespace React {
     type Key = string | number | bigint;
 
     /**
-     * @internal You shouldn't need to use this type since you never see these attributes
-     * inside your component or have to validate them.
+     * @internal The props any component can receive.
+     * You don't have to add this type. All components automatically accept these props.
+     * ```tsx
+     * const Component = () => <div />;
+     * <Component key="one" />
+     * ```
+     *
+     * WARNING: The implementation of a component will never have access to these attributes.
+     * The following example would be incorrect usage because {@link Component} would never have access to `key`:
+     * ```tsx
+     * const Component = (props: React.Attributes) => props.key;
+     * ```
      */
     interface Attributes {
         key?: Key | null | undefined;
     }
+    /**
+     * The props any component accepting refs can receive.
+     * Class components, built-in browser components (e.g. `div`) and forwardRef components can receive refs and automatically accept these props.
+     * ```tsx
+     * const Component = forwardRef(() => <div />);
+     * <Component ref={(current) => console.log(current)} />
+     * ```
+     *
+     * You only need this type if you manually author the types of props that need to be compatible with legacy refs.
+     * ```tsx
+     * interface Props extends React.RefAttributes<HTMLDivElement> {}
+     * declare const Component: React.FunctionComponent<Props>;
+     * ```
+     *
+     * Otherwise it's simpler to directly use {@link Ref} since you can safely use the
+     * props type to describe to props that a consumer can pass to the component
+     * as well as describing the props the implementation of a component "sees".
+     * {@link RefAttributes} is generally not safe to describe both consumer and seen props.
+     *
+     * ```tsx
+     * interface Props extends {
+     *   ref?: React.Ref<HTMLDivElement> | undefined;
+     * }
+     * declare const Component: React.FunctionComponent<Props>;
+     * ```
+     *
+     * WARNING: The implementation of a component will not have access to the same type in versions of React supporting string refs.
+     * The following example would be incorrect usage because {@link Component} would never have access to a `ref` with type `string`
+     * ```tsx
+     * const Component = (props: React.RefAttributes) => props.ref;
+     * ```
+     */
     interface RefAttributes<T> extends Attributes {
         /**
          * Allows getting a ref to the component instance.
          * Once the component unmounts, React will set `ref.current` to `null` (or call the ref with `null` if you passed a callback ref).
          * @see {@link https://react.dev/learn/referencing-values-with-refs#refs-and-the-dom}
          */
-        ref?: Ref<T> | undefined;
+        ref?: LegacyRef<T> | undefined;
     }
     /**
      * Represents the built-in attributes available to class components.
      */
-    interface ClassAttributes<T> extends Attributes {
-        /**
-         * Allows getting a ref to the component instance.
-         * Once the component unmounts, React will set `ref.current` to `null` (or call the ref with `null` if you passed a callback ref).
-         * @see {@link https://react.dev/learn/referencing-values-with-refs#refs-and-the-dom}
-         */
-        ref?: LegacyRef<T> | undefined;
+    interface ClassAttributes<T> extends RefAttributes<T> {
     }
 
     /**
@@ -1520,6 +1558,7 @@ declare namespace React {
         P extends any ? ("ref" extends keyof P ? Omit<P, "ref"> : P) : P;
     /** Ensures that the props do not include string ref, which cannot be forwarded */
     type PropsWithRef<P> =
+        // Note: String refs can be forwarded. We can't fix this bug without breaking a bunch of libraries now though.
         // Just "P extends { ref?: infer R }" looks sufficient, but R will infer as {} if P is {}.
         "ref" extends keyof P
             ? P extends { ref?: infer R | undefined }

diff --git a/types/react/ts5.0/test/index.ts b/types/react/ts5.0/test/index.ts
@@ -443,8 +443,11 @@ const ForwardingRefComponent2 = React.forwardRef<HTMLElement>((props, ref) => {
         ref(e: HTMLDivElement) {
             if (typeof ref === "function") {
                 ref(e);
-            } else if (ref) {
+            } else if (typeof ref === "object" && ref !== null) {
                 ref.current = e;
+            } else {
+                // $ExpectType null
+                ref;
             }
         },
     });

diff --git a/types/react/ts5.0/test/tsx.tsx b/types/react/ts5.0/test/tsx.tsx
@@ -454,11 +454,9 @@ const badlyAuthoredRef: React.RefObject<HTMLDivElement | null | undefined> = { c
 
 <ForwardRef ref={divFnRef} />;
 <ForwardRef ref={divRef} />;
-// @ts-expect-error
 <ForwardRef ref="string" />;
 <ForwardRef2 ref={divFnRef} />;
 <ForwardRef2 ref={divRef} />;
-// @ts-expect-error
 <ForwardRef2 ref="string" />;
 // @ts-expect-error Undesired behavior
 <ForwardRef2 ref={badlyAuthoredRef} />;
@@ -482,7 +480,6 @@ const newContextRef = React.createRef<NewContext>();
 
 const ForwardNewContext = React.forwardRef((_props: {}, ref?: React.Ref<NewContext>) => <NewContext ref={ref} />);
 <ForwardNewContext ref={newContextRef} />;
-// @ts-expect-error
 <ForwardNewContext ref="string" />;
 
 const ForwardRef3 = React.forwardRef(