Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

@mantine/hooks useClickOutside ref typescript type is immutable #7259

Open
2 tasks done
scamden opened this issue Dec 13, 2024 · 4 comments
Open
2 tasks done

@mantine/hooks useClickOutside ref typescript type is immutable #7259

scamden opened this issue Dec 13, 2024 · 4 comments
Labels
Needs reproduction Issues without reproduction, closed within a week if the reproduction is not provided

Comments

@scamden
Copy link

scamden commented Dec 13, 2024

Dependencies check up

  • I have verified that I use latest version of all @mantine/* packages

What version of @mantine/* packages do you have in package.json?

7.15.1

What package has an issue?

@mantine/hooks

What framework do you use?

Vite

In which browsers you can reproduce the issue?

None

Describe the bug

useClickOutside returns

import("react").RefObject<T | null>;

Which is readonly. React's types don't allow you to pass this to <div ref= which is the canonical example given in the docs.

If possible, include a link to a codesandbox with a minimal reproduction

No response

Possible fix

I believe this should be MutableRefObject instead

Self-service

  • I would be willing to implement a fix for this issue
@noahw3
Copy link

noahw3 commented Dec 14, 2024

I just came across this with useElementSize as well. It looks like it was introduced in 7.15.0. It switched from ref: import("react").RefObject<T>; to ref: import("react").RefObject<T | null>;.

@noahw3
Copy link

noahw3 commented Dec 14, 2024

Also, looks like this is a dupe of #7252

@rtivital
Copy link
Member

I cannot reproduce the issue on codesandbox https://codesandbox.io/p/sandbox/mantine-react-template-forked-nmnl8y
Please provide a sandbox with a minimal reproduction.

@rtivital rtivital added the Needs reproduction Issues without reproduction, closed within a week if the reproduction is not provided label Dec 15, 2024
@scamden
Copy link
Author

scamden commented Dec 16, 2024

I cannot reproduce the issue on codesandbox https://codesandbox.io/p/sandbox/mantine-react-template-forked-nmnl8y Please provide a sandbox with a minimal reproduction.

https://codesandbox.io/p/sandbox/mantine-react-template-forked-pzk2y5

Needs to specify the element type to see the bug.

Actual issue looks like: useClickOutside adds null to the ref type but LegacyRef on divs don't want the null.

Screenshot 2024-12-16 at 11 14 51 AM

The reason MutableRefObject happens to fix this, for the curious, is because it's type directly accepts the generic without adding null

 interface MutableRefObject<T> {
        current: T;
    }

whereas RefObject adds the null therefore inferring a non-null T for the generic

    interface RefObject<T> {
      /**
       * The current value of the ref.
       */
      readonly current: T | null;
  }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs reproduction Issues without reproduction, closed within a week if the reproduction is not provided
Projects
None yet
Development

No branches or pull requests

3 participants