useResizeObserver ref type aligned with useRef

ZeeCoder / use-resize-observer

A React hook that allows you to use a ResizeObserver to measure an element's size.

MIT License

651 stars 42 forks source link

useResizeObserver ref type aligned with useRef #13

Closed troyspencer closed 5 years ago

troyspencer commented 5 years ago

MutableRefObject is what useRef returns. With this change, the refs are interchangeable. This also allows it to be placed on more elements without TypeScript complaining. Solves #12

ZeeCoder commented 5 years ago

I'm sorry I didn't answer for so long, I'm quite busy with work / personal life stuff at the moment and as I don't know TypeScript I have no way to confirm this change.

Could you recommend articles / docs where I could confirm what it means that you did here?

I need to up my TypeScript game at some point anyway 😅

henrycatalinismith commented 5 years ago

Came here after Googling this error:

Error

Type 'RefObject' is not assignable to type 'string | ((instance: HTMLDivElement | null) => void) | RefObject | null | undefined'.
  Type 'RefObject' is not assignable to type 'RefObject'.
    Property 'align' is missing in type 'HTMLElement' but required in type 'HTMLDivElement'.ts(2322)
index.d.ts(87, 9): The expected type comes from property 'ref' which is declared here on type 'DetailedHTMLProps, HTMLDivElement>'

Screenshot

Then I made the following change to my use-resize-observer call after reading the diff of this PR:

-const [ref, width, height] = useResizeObserver()
+const [ref, width, height] = useResizeObserver() as [MutableRefObject<any>, number, number]

I haven't spent any time understanding this at any deeper level, but it's made my error go away! 😆

ZeeCoder commented 5 years ago

I'm actually thinking of just removing TS support now. 🤷‍♂️ Maybe in the future I might try rewriting the source, but the need of maintaining types without knowing the language is already stopping me from updating this lib. 😅

troyspencer commented 5 years ago

The PR works, it fixes the issue I referenced. It does so in the typing source, rather than being patched upon usage. The original typing was wrong, and too restrictive to the intended elements to place it on.

ZeeCoder commented 5 years ago

I'd rather just not bother with types as I have no way of ensuring they're right. From my point of view, someone else could come along in a week's time, claiming that your changes were not right either. 🤷‍♂️

Not to mention it's blocking changes I would've released ages ago otherwise: https://github.com/ZeeCoder/use-resize-observer/pull/10

troyspencer commented 5 years ago

I've got my fork, have fun over here then

ZeeCoder commented 5 years ago

It's annoying that TS would force you to fork a lib like this. Wonder if Flow does the same or if it's more forgiving.

troyspencer commented 5 years ago

TS isn't what forced the fork, making a PR to fix an issue did that. But the maintainer removing TS because learning is hard, and forcing solutions like hencatsmith did above is a good reason continue using that fork, and not look back on this repo.

ZeeCoder commented 5 years ago

Ugh, sorry that I made you do all that effort, must've been horrible, I can't imagine the hours you must've spent on it. 🙄

"learning is hard" Yeah, sure that was it. 🤦‍♂️

Can't believe even at this level people get annoyed when the maintainer doesn't do exactly what they want with a lib he spent countless hours on already.

Have fun with your fork of free code, I have no time for such toxicity in my life.