Add option for different types of dimensions

TyMick commented 4 years ago

Hi, @ZeeCoder! Came across your project looking for a hook that would keep track of changes to element offsetWidth/offsetHeight. I love your implementation of the ResizeObserver API, and I ended up finding the API can track offset, client, and scroll dimensions as well via ResizeObserverEntry.target.

What would you think of adding one more option called something like type, leaving "content" as default but adding the option for "offset", "client", and "scroll" as well? This edit's been tracking offsetWidth and offsetHeight perfectly for me so far.

mattlucock commented 4 years ago

Your "offset" option is already in the spec; it's just badly supported at the moment.

My feeling is that this hook should just be a thin abstraction over the ResizeObserver API, and shouldn't return any data not returned by the observer itself. If you want to track clientWidth or scrollWidth (or offsetWidth, for the time being) etc., you could do so very easily by passing in your own ref and using a onResize callback.

mattlucock commented 4 years ago

Perhaps the hook could have an option to track borderBoxSize now and it actually just tracks offsetWidth and offsetHeight until there's better browser support. That would actually be quite useful.

ZeeCoder commented 4 years ago

Sorry for not reacting to this yet, but as there's a lot going on in my life in light of recent events I can't dedicate the time to this it would deserve. Thank you both for your efforts, I'll get back to you as soon as I'm able.

ZeeCoder commented 4 years ago

So I can imagine exposing the entry in one way or another, but I'd like to keep the current interface as simple as possible.

So for example I wouldn't return the entry in the hook response, because I would need to store that in a hook state, which means the hook would trigger a re-render every time there's a new entry, regardless of whether you were interested in the entry or not.

Instead, I would add an observation option forwarded to RO.observe, and then maybe a handleEntry callback, so something like this:

// Notice I omit width/height here, as the hook would omit them,
// similar to when onResize is provided
const { ref } =  useResizeObserver<HTMLDivElement>({
  observeOptions: { box : 'border-box' },
  handleEntry: (entry: ResizeObserverEntry) => { ... },
});

I'd also need to make sure onResize is still usable alongside it though, which also means more tests for such edge cases, and it'll require more TS function overloads too.

The more options you have the more complicated the internals become, as you need to support all combinations. 😅

The good thing about all of this is that it makes the hook more future proof, as now there'd be a way to receive the ResizeObserver entry as-is.

ZeeCoder commented 4 years ago

Or maybe I could simply augment the onResize callback, where it could receive the entry itself as well actually.

ZeeCoder commented 4 years ago

What you say @matthewlucock is exactly right, I'd like to keep the hook as lean as possible. That's the reason I implemented onResize instead of explicit debounce / throttle support within the options, as it just leads down a rabbit hole of options to support.

It's a lot easier to just compose new hooks out of this one as a base, especially if the entry is exposed, then it becomes very easy to implement what @tywmick would like.

I might consider shipping such higher-level hooks alongside the main one though in the future, if there's demand.

mattlucock commented 4 years ago

I see how providing the actual entry itself serves to future-proof the hook in principle, but I'm not sure if that would actually be useful. Tracking the content box and the border box are the only two things that ResizeObservers can do, and, y'know, I can't really think of anything else they could possibly do in the future—the content box and the border box covers it. Tracking the border box this way definitely wouldn't provide the elegance of the existing API of this hook. As for composing this hook to track scrollWidth/scrollHeight or clientWidth/clientHeight or whatever else, I don't see that being necessary to support. I think that doing what I said—passing in a ref and using the onResize callback—is a much more elegant solution for that use case.

I no longer actually have a use case for any of this because I'm no longer in the job where I was using this hook, but I definitely did have a use case for tracking the border box. I just did it using this hook by wrapping the component I was trying to track in a simple <div>, because in that case, the content box of the parent is the border box of the child.

ZeeCoder commented 4 years ago

Fair points

ZeeCoder commented 4 years ago

For reference, these are the entry properties: https://developer.mozilla.org/en-US/docs/Web/API/ResizeObserverEntry

Currently the hook uses contentRect.width and -height, and ignores the rest.