preactjs / preact

⚛️ Fast 3kB React alternative with the same modern API. Components & Virtual DOM.
https://preactjs.com
MIT License
36.35k stars 1.93k forks source link

Type errors involving `forwardRef` and `Ref` in the compatibility layer. #4402

Closed maxbrieiev closed 1 month ago

maxbrieiev commented 1 month ago

Describe the bug

I am having some issue with preact's compatibility layer. I use Deno and Headlessui, but I don't think it is Deno specific.

There are unexpected type errors, when using forwardRef, ForwardedRef and Ref. When doing import { type Ref } from "react", the preact's compatibility layer appears to resolve the type from preact/hooks/src, but possibly it should be resolved from preact/src instead. This seems to cause type errors.

To Reproduce

mkdir deno-preact-test
cd deno-preact-test
deno init

Add the following to deno.json:

  "imports": {
    "@headlessui/react": "https://esm.sh/@headlessui/react@2.0.4?external=react,react-dom,@types/react",
    "preact": "npm:preact@10.22.0",
    "react": "npm:/preact@10.22.0/compat",
    "react-dom": "npm:/preact@10.22.0/compat",
    "@types/react": "npm:/preact@10.22.0/compat"
  },
  "compilerOptions": {
    "jsx": "react-jsx",
    "jsxImportSource": "preact"
  }

Create index.tsx file with the following content:

import * as Headless from "@headlessui/react";
import { type ForwardedRef, forwardRef } from "react";

export const MyInput = forwardRef(
  (props, ref: ForwardedRef<HTMLInputElement>) => {
    return (
      <span>
        <Headless.Input ref={ref} {...props} />
      </span>
    );
  },
);

Run deno check index.tsx and observe the output:

error: TS2322 [ERROR]: Type 'ForwardedRef<HTMLInputElement>' is not assignable to type 'Ref<HTMLElement> | undefined'.
  Type 'null' is not assignable to type 'Ref<HTMLElement> | undefined'.
        <Headless.Input ref={ref} {...props} />
                        ~~~
    at file:///Users/max/code/deno-preact-test/index.tsx:8:25

    The expected type comes from property 'ref' which is declared here on type 'IntrinsicAttributes & CleanProps<"input", InputPropsWeControl | "disabled" | "invalid" | "autoFocus"> & OurProps<"input", InputRenderPropArg> & { ...; } & { ...; } & { ...; }'
        ref?: Ref<RefType>;
        ~~~
        at https://esm.sh/v135/@headlessui/react@2.0.4/X-ZS9AdHlwZXMvcmVhY3QscmVhY3QscmVhY3QtZG9t/dist/utils/render.d.ts:62:5

Screenshot

Screenshot 2024-05-30 at 13 06 30
maxbrieiev commented 1 month ago

FYI, I get this type error also with Nodejs:

npm init preact
cd my-preact-app
npm install @headlessui/react

Create a file with the content as in the previous message and observe the error.

rschristian commented 1 month ago

When you say

When doing import { type Ref } from "react", preact's compatibility layer appears to resolve the type from preact/hooks/src, but possibly it should be resolved from preact/src instead. This seems to cause type errors.

Do you mean this type error goes away if you switch it to the type from core, rather than hooks?

It's certainly possible that was just a typo when it was written, standardizing the Ref interface (where possible, I think everything should mostly align w/out issue) sounds good to me if you wanted to make a PR.

maxbrieiev commented 1 month ago

Do you mean this type error goes away if you switch it to the type from core, rather than hooks?

Correct.

It turns out that the error can be reproduced as simply as this:

import type { ForwardedRef, Ref } from "preact/compat";

export const fun = (ref: ForwardedRef<HTMLInputElement>): Ref<HTMLInputElement> => ref;

And here is the patch with a testcase that fixes the error:

diff --git a/compat/src/index.d.ts b/compat/src/index.d.ts
index 810629c1..08f2cbb6 100644
--- a/compat/src/index.d.ts
+++ b/compat/src/index.d.ts
@@ -18,7 +18,7 @@ declare namespace React {
    export import PropRef = _hooks.PropRef;
    export import Reducer = _hooks.Reducer;
    export import Dispatch = _hooks.Dispatch;
-   export import Ref = _hooks.Ref;
+   export import Ref = preact.Ref;
    export import SetStateAction = _hooks.StateUpdater;
    export import useCallback = _hooks.useCallback;
    export import useContext = _hooks.useContext;
diff --git a/compat/test/ts/forward-ref.tsx b/compat/test/ts/forward-ref.tsx
index 9a920cb0..e83ae540 100644
--- a/compat/test/ts/forward-ref.tsx
+++ b/compat/test/ts/forward-ref.tsx
@@ -24,3 +24,7 @@ export const Bar = React.forwardRef<HTMLDivElement, { children: any }>(
        return <div ref={ref}>{props.children}</div>;
    }
 );
+
+export const baz = (
+   ref: React.ForwardedRef<HTMLInputElement>
+): React.Ref<HTMLInputElement> => ref;

If it feels ok, I can submit a pull request.