Prevent SwipeableDrawer crash when parent update while swiping

[xavier-villelegier]

Hey hey ! 👋

We noticed a lot of sentries in getMaxTranslate because this crashes when the parent updates while the user is swiping the drawer.

This is already handled in handleBodyTouchMove here
https://github.com/mui-org/material-ui/blob/98c9aad08696f44976f55aa1e254d013df6c38e1/packages/mui-material/src/SwipeableDrawer/SwipeableDrawer.js#L296-L300

but not in handleBodyTouchEnd yet

Tested locally in multiple cases and it works fine 👌

• I have followed (at least) the PR section of the contributing guide.

Cheers from Doctolib ✌️

[mui-pr-bot]

Details of bundle changes

Generated by 🚫 dangerJS against fcf2a8a

[oliviertassinari]

@xavier-villelegier Thanks for the PR, however, we miss a reproduction at minimum in order to be able to move forward.

[oliviertassinari]

This type of defensive check usually hides root causes. To always be suspicious.

Here, I suspect that because the cleanup happens in a useEffect, it's async, but it should be a layout effect to be sync. We might have similar cleanup problems in the other components.

https://github.com/mui-org/material-ui/blob/fcf2a8a5c90e6a975b0a8c391aa2f95f6e860b15/packages/mui-material/src/SwipeableDrawer/SwipeableDrawer.js#L510

I hope it illustrates why #30440 (comment) (not the only reason, e.g. so we can add a test case)

[xavier-villelegier]

Hey @oliviertassinari, thanks for you answer, I definitely agree. We've inlined the component in production to check, and it's not the root cause anyway, the sentries are still there, which sounds a bit surprising 🤔 I've also tried to replace this useEffect by a useLayoutEffect, and still the same errors

I've tried various things to trigger the error, but no luck I didn't reproduce it yet. But it's not that edge case as we have 1k/day occurences on iOS and 1k/day on Android. Thus it doesn't look like OS-related, i'd say it's more about the react lifecycle itself

[siriwatknp]

👋 The migration PR has been merged.

Please follow these steps to make sure that the contents are all updated. (Sorry for the inconvenience)

1. pull latest master from upstream to your branch
2. if your PR has changes on the *.md or demo files, you are likely to encounter conflict because all of them have been moved to the new folder.
   2.1 revert the change on those markdown files you did
   2.2 pull latest master from upstream (you should not see conflict)
   2.3 make the changes again at docs/data/material/*
3. run yarn docs:api
   • you might see the changes in docs/pages/material/api/* if your PR touches some of the components
   • it is okay if there is no changes

If you are struggle with the steps above, feel free to tag @siriwatknp

[xavier-villelegier]

Closing this for now. We still have the issue on production though, we'll try to find a way to fix this on our side