Panel onResize not called if there is no onLayout

[rijk]

For the initial render, if the layout is restored from local storage, the Panel's onResize function is only called if the PanelGroup has a onLayout. This is not expected and I assume was not intentional?

In this PR I just removed the if (onLayout) check in favor of a onLayout?.(sizes) later on.

[vercel]

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	Jun 24, 2023 3:46am

[bvaughn]

Thanks!

[bvaughn]

Published as 0.0.52!

[bvaughn]

I think that either this PR or the ordering change of setSizes and callPanelCallbacks in a87cdb6 caused a regression:

react-dom.development.js:26131 Uncaught TypeError: Cannot read properties of undefined (reading 'current')
    at eval (react-resizable-panels.esm.js:270:7)
    at Array.forEach (<anonymous>)
    at callPanelCallbacks (react-resizable-panels.esm.js:264:1)
    at eval (react-resizable-panels.esm.js:897:1)
    at commitHookEffectListMount (react-dom.development.js:26466:1)

I tried updating to this version in Replay (replayio/devtools#9381) and here is a recording of a failed end to end test:
https://app.replay.io/recording/affac99d-d290-4cc6-91db-6c542545ef12

---

Edit 1: Fortunately I'm able to run the test locally and reproduce the failure. Reverting this PR fixes the bug, so it does seem to be related to this change rather than a87cdb6.

Adding some logging to my local build of resizable panels shows that the panelsArray is empty when callPanelCallbacks is called for the first time (after this change).

---

Edit 2: I think the problem is that the panel refs are stored in state:

const [panels, setPanels] = useState<PanelDataMap>(new Map());

Each Panel registers itself after mounting (in an effect) which is also when the PanelGroup now tries to call onResize – but at that point, the panels value in the effect is the one that rendered initially (before registration).

Maybe it's possible to remove registered panels from state and just store them in a mutable ref.

---

Edit 3: This is trickier than I thought it was going to be. Storing panels in state seems to be important since sometimes re-renders are needed to call effects after changes. I also tried mutating the committed values in the registarPanels callback, to handle the case that was erroring, but while it fixed that case it broken another one of this project's e2e tests.

I'm not sure how to fix this.

---

Edit 4: I think I have found a workaround: 3bf394a

[rijk]

Woa, nice work tracking that down. Sorry this PR introduced that issue!

[bvaughn]

No worries ☺️

…

On Sun, Jun 25, 2023 at 2:17 PM Rijk van Wel ***@***.***> wrote: Woa, nice work tracking that down. Sorry this PR introduced that issue! — Reply to this email directly, view it on GitHub <#161 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAHHHIDQX6KIUYHBZLCRYLXNB6E5ANCNFSM6AAAAAAZQIH36Y> . You are receiving this because you modified the open/close state.Message ID: ***@***.***>