Description
We've seen several layout issues after switching to New Architecture as documented and reproduced here https://github.com/Jpoliachik/NewArchNavLayoutIssues
I spent some time digging into the Android issues and here is what I found:
(I did a similar dive for iOS here: #2802)
Isolated Issue
I focused on issue 4 in the repo above: When a modal is presented over a stack, and dismissTo is called to return to an earlier screen in the stack, it breaks the view behind it and puts the app in a very weird state. See this happen below when dismissTo is pressed on green/modal, and then we navigate back to a broken view. This is a known issue: #2578

When debugging in Android Studio, we see an error thrown when dismissTo is called. This is thrown by React Native Fabric MountingManager.addViewAt. And then subsequent errors suggesting that the native view is out of sync with the ShadowTree.


View Recycling
Digging further I found that react-native-screens implements a recycle() function on views, intended to detach screens from old parent views.

I added logs inside recycle() and saw that React Native's error always happens first, and recycle() gets called later. So the error makes sense - if Fabric first tries to addViewAt before the view has been prepped for recycle, we would expect it to still have a parent view. And then we'd expect the screen to not render properly if it was never re-added to the Fabric surface. Why is this view not getting prepped for recycle before getting added to ShadowTree? I'm not sure.
I wish I had a workaround or a specific fix to offer, but my Native Android knowledge taps out here. I'm unsure of the view recycle mechanisms and how they pair with Fabric. But the root of this issue seems to point to cases where Fabric's ShadowTree gets out of sync with the native view state. I'm unsure how that might be happening.
Root Issue
It's worth noting: this syncing issue seems very similar to many of the issues found on iOS, which makes me wonder if they share a similar root cause? iOS is also dealing with issues where the Fabric Shadow Tree is out of sync with views: #2802 and there are a lot of Android issues that could potentially be attributed to an out-of-sync Fabric state:
One thought that also relates to the issue iOS sees: do we know if its okay to call updateState directly on a Shadow Node outside of a scheduled render cycle in Fabric at all? Or should layout updates be handled in JS so we can let React diff and apply view updates? I wonder if Fabric has specific assumptions about view updates that are being bypassed with direct updates in updateScreenSizeFabric. I'm out of my depth with Fabric internals to know myself.
Thanks for reading, I apologize for the wall of text but I wanted to outline all the info I collected while debugging and potentially spark a larger discussion on how to best move forward with New Architecture overall.
Steps to reproduce
- Presend a modal over a stack with > 2 views
- dismissTo the earlier view on the stack
See: https://github.com/Jpoliachik/NewArchNavLayoutIssues - issue 4
Snack or a link to a repository
https://github.com/Jpoliachik/NewArchNavLayoutIssues
Screens version
4.9.0
React Native version
0.76.7
Platforms
Android
JavaScript runtime
None
Workflow
None
Architecture
Fabric (New Architecture)
Build type
None
Device
None
Device model
No response
Acknowledgements
Yes
Description
We've seen several layout issues after switching to New Architecture as documented and reproduced here https://github.com/Jpoliachik/NewArchNavLayoutIssues
I spent some time digging into the Android issues and here is what I found:
(I did a similar dive for iOS here: #2802)
Isolated Issue
I focused on issue 4 in the repo above: When a modal is presented over a stack, and
dismissTois called to return to an earlier screen in the stack, it breaks the view behind it and puts the app in a very weird state. See this happen below whendismissTois pressed on green/modal, and then we navigate back to a broken view. This is a known issue: #2578When debugging in Android Studio, we see an error thrown when
dismissTois called. This is thrown by React Native FabricMountingManager.addViewAt. And then subsequent errors suggesting that the native view is out of sync with the ShadowTree.View Recycling
Digging further I found that react-native-screens implements a
recycle()function on views, intended to detach screens from old parent views.I added logs inside
recycle()and saw that React Native's error always happens first, andrecycle()gets called later. So the error makes sense - if Fabric first tries toaddViewAtbefore the view has been prepped for recycle, we would expect it to still have a parent view. And then we'd expect the screen to not render properly if it was never re-added to the Fabric surface. Why is this view not getting prepped for recycle before getting added to ShadowTree? I'm not sure.I wish I had a workaround or a specific fix to offer, but my Native Android knowledge taps out here. I'm unsure of the view recycle mechanisms and how they pair with Fabric. But the root of this issue seems to point to cases where Fabric's ShadowTree gets out of sync with the native view state. I'm unsure how that might be happening.
Root Issue
It's worth noting: this syncing issue seems very similar to many of the issues found on iOS, which makes me wonder if they share a similar root cause? iOS is also dealing with issues where the Fabric Shadow Tree is out of sync with views: #2802 and there are a lot of Android issues that could potentially be attributed to an out-of-sync Fabric state:
One thought that also relates to the issue iOS sees: do we know if its okay to call
updateStatedirectly on a Shadow Node outside of a scheduled render cycle in Fabric at all? Or should layout updates be handled in JS so we can let React diff and apply view updates? I wonder if Fabric has specific assumptions about view updates that are being bypassed with direct updates inupdateScreenSizeFabric. I'm out of my depth with Fabric internals to know myself.Thanks for reading, I apologize for the wall of text but I wanted to outline all the info I collected while debugging and potentially spark a larger discussion on how to best move forward with New Architecture overall.
Steps to reproduce
See: https://github.com/Jpoliachik/NewArchNavLayoutIssues - issue 4
Snack or a link to a repository
https://github.com/Jpoliachik/NewArchNavLayoutIssues
Screens version
4.9.0
React Native version
0.76.7
Platforms
Android
JavaScript runtime
None
Workflow
None
Architecture
Fabric (New Architecture)
Build type
None
Device
None
Device model
No response
Acknowledgements
Yes