Do not call onResize on mount if the defaultSize is provided

[myklt]

Actual behavior: onResize should be called only, when the actual change happens Expected behavior: onResize should not be called on mount, when the passed size matches the provided default size Workaround: check on user's side, if the default size is provided and prevSize is undefined. Proposed solution: do the check on the library side

[vercel[bot]]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
react-resizable-panels	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Jul 2, 2024 10:22am

[bvaughn]

This change seems reasonable and I understand why you would propose it. I'm concerned that it makes the library's behavior less predictable though. In my experience, you sometimes need to write code that syncs a value to external state (e.g. onResize(value => setState(value)) and it's nicer in that case when the callback fires for the initial value also. With your proposal, you could just initialize state to the same value as defaultSize but that can be something you forget to do (especially when the code might be in two different locations, like a custom hook). The current behavior protects against that, at the cost of an extra state update– but that update should be low cost, since React would bail out of any potential render work if the same value was passed as was already in the state. Does that make sense?

[myklt]

Thanks for such a quick response, @bvaughn .

I partially agree with your concern. As usual, it is a trade-off:

1. Without a fix: in case the user has side effect (e.g. calling api to store the selected size) in case of resize, then redundant actions (e.g. network calls) are being made while user expects them to be triggered only on actual change.
2. With a fix: the user needs to be aware, that the first onResize can be triggered also without any actual change and avoid doing any side effect action.

So I'm fine to solve it on my side, but I thought it would be useful for other people as well. In case of setState I don't see any problem, but I had side effects in mind.

[bvaughn]

That makes sense!

In the case of external side effects like you describe, it seems to me like this change could complicate the implementation too, but that would depend on whether the side effect cared about the initial/default size. In my experience with other similar component libraries, they often do, which makes the syncing logic a bit more complicated. (You need to write a mount effect to send the initial value, and then the rest can be sent in by the change handler.)

To me there doesn't seem to be a clear "right way" to do this. Given that, I'll pass on this suggested change. (It would be a breaking change and require a major release, and I'm not convinced it's better just equally valid.)

Thanks for the discussion!