Open dilipk3 opened 1 year ago
Thanks for the great detail on this bug report. Potentially could have been affected by this PR: https://github.com/microsoft/react-native-windows/pull/10712 Certainly though this shouldn't crash. Thanks for the set of suggested fixes.
@chrisglein you are welcome, glad it could help.
Ahh yes, in the PR https://github.com/microsoft/react-native-windows/pull/10712 that you linked, that could potentially fix this if we didn't dispatch the layout event for elements that are in collapsed sub trees.
However, the question I wasn't able to answer, and whether it's expected behavior, is why YGNodeLayoutGetWidth
or YGNodeLayoutGetHeight
return NaN
(as opposed to something like 0) for such elements. Do you know?
However, the question I wasn't able to answer, and whether it's expected behavior, is why
YGNodeLayoutGetWidth
orYGNodeLayoutGetHeight
returnNaN
(as opposed to something like 0) for such elements. Do you know?
That I don't know. @acoates-ms @rozele do you know?
Does this reproduce in older versions of react-native-windows or only v0.71? There were a couple changes that first landed in v0.71 (#10658 and #10422) that may have introduced a regression. I can have a look tomorrow to see what the problem might be.
I know prior to #10422, I saw a similar issue when a node might have been created but not attached to any root view in a single UIManager operation, but this must be something else.
I debugged this a bit - no luck in identifying the root cause, though one interesting thing I noticed is if I switch the setCount
call from the interval timer to instead fire in an onPress callback on the rendered Text, the issue still sometimes reproduces, but not always 🤯.
https://user-images.githubusercontent.com/1106239/218638891-e429fcc0-cc65-49bd-96cb-01d590e2d8e3.mp4
The faster the clicks (re-renders), the more likely the issue will reproduce.
My hunch is that this is a bug in Yoga, but I don't have definitive evidence of that yet.
@rozele This reproduces in older versions as well.
Is there any update on this? I am getting this crash on the latest RNW and navigation.
I am experiencing this as well with a view component that references onLayout
import React, { useState } from 'react';
import { View, Text } from 'react-native';
export const InfoDisplayComponent = ({ containerStyle }) => {
const [frameSize, setFrameSize] = useState(50);
return (
<View
onLayout={e => {
const { height, width } = e?.nativeEvent?.layout ?? {};
setFrameSize(height < width ? height / 2 : width / 2);
}}
style={[{ width: '50%', flex: 1 }, containerStyle]}>
<Text>Hello World</Text>
</View>
);
};
Problem Description
Background
When updating views with a new layout who have hidden parents, the app crashes when you
The crash with the remote debugger on is "Windows APP: folly::toJson: JSON object value was a NaN when serializing value at "arguments"->2->2->"layout"->"height". This is because NaN isn't serializable to JSON.
Without remote debugging on, the onLayout callback is fired with NaN width and height. This also doesn't seem correct so this problem is not just for remote debugging.
I believe this issue is similar: https://github.com/microsoft/react-native-windows/issues/8318 but since I have much more simper reproducible steps, I decided to file a different issue.
Although the example code I have is contrived, this pattern is very common in React navigation where you have a stack of screens. Once you navigate into another screen in the stack, the previous screen is hidden using
display: none
. If the previous screen does some kind of state update and has a layout change, it causes this crash. There seems be a similar issue filed in React Navigation repo here: https://github.com/react-navigation/react-navigation/issues/10719My investigation
I tried debugging the code and the issue seems to arise because of NaN values returned for both width and height in the NativeUIManager here https://github.com/microsoft/react-native-windows/blob/main/vnext/Microsoft.ReactNative/Modules/NativeUIManager.cpp#L942-L943 in this code block:
The NaN values cause the
hasLayoutChanged
check to be true.In release mode, the app doesn't crash however the JS
onLayout
callback receives NaN values. SeePotential fixes
YGNodeLayoutGetWidth
orYGNodeLayoutGetHeight
should not return NaN - I'm not sure how feasible this is? I'm not sure why theYGNodeLayoutGetWidth
orYGNodeLayoutGetHeight
return NaN - maybe because there is no corresponding native element for this Yoga node?onLayout
for NaN values - doing something like this seems to fix the issue for meSteps To Reproduce
Run the following code:
Expected Results
I don't think the onLayout callback should be run in this case. That is at least how upstream RN seems to work for iOS and Android as seen in this Expo snack here: https://snack.expo.dev/ekeguyQ98
CLI version
0.71
Environment