App crash when registering onLayout on Views with hidden parents

Problem Description

Background

When updating views with a new layout who have hidden parents, the app crashes when you

1. register the onLayout callback
2. have remote debugger on.

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/10719

My 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:

 float width = YGNodeLayoutGetWidth(yogaNode);
 float height = YGNodeLayoutGetHeight(yogaNode);

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. See

Potential fixes

1. YGNodeLayoutGetWidth or YGNodeLayoutGetHeight should not return NaN - I'm not sure how feasible this is? I'm not sure why the YGNodeLayoutGetWidth or YGNodeLayoutGetHeight return NaN - maybe because there is no corresponding native element for this Yoga node?
2. Don't run onLayout for NaN values - doing something like this seems to fix the issue for me

const auto isValidLayout = !isnan(width) && !isnan(height);
 if (isValidLayout && hasLayoutChanged) {
        React::JSValueObject layout{{"x", left}, {"y", top}, {"height", height}, {"width", width}};
        React::JSValueObject eventData{{"target", tag}, {"layout", std::move(layout)}};
        pViewManager->DispatchCoalescingEvent(tag, L"topLayout", MakeJSValueWriter(std::move(eventData)));
  }

3. Change NaN values to 0 before calling topLayout

Steps To Reproduce

Run the following code:

import React, {useState, useEffect} from 'react';

import {Text, View} from 'react-native';

const CrashTest = () => {
  const [count, setCount] = useState(0);

  useEffect(() => {
    const id = setInterval(() => {
      setCount(c => {
        return c + 1;
      });
    }, 1000);

    return () => {
      clearInterval(id);
    };
  }, []);

  console.log(`LayoutTestPage rendering for count: ${count}`);

  return (
    <>
      <Text>
        Layout Crash Test. Check the console log - the app with crash for count
        5
      </Text>
      <View style={{display: 'none', flex: 1}}>
        {/* Removing this view causes the error to disappear*/}
        <View>
          {count % 5 === 0 ? (
            <View
              onLayout={event =>
                console.log('Layout', event.nativeEvent.layout)
              }>
              <Text>This View's on Layout causes crash</Text>
            </View>
          ) : null}
        </View>
      </View>
    </>
  );
}
export default CrashTest;

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

System:
    OS: Windows 10 10.0.19044
    CPU: (20) x64 12th Gen Intel(R) Core(TM) i7-12800H
    Memory: 15.53 GB / 31.68 GB
  Binaries:
    Node: 16.16.0 - ~\AppData\Local\Volta\tools\image\node\16.16.0\node.EXE
    Yarn: 1.22.19 - ~\AppData\Local\Volta\tools\image\yarn\1.22.19\bin\yarn.CMD
    npm: 8.11.0 - ~\AppData\Local\Volta\tools\image\node\16.16.0\npm.CMD
    Watchman: Not Found
  SDKs:
    Android SDK: Not Found
    Windows SDK:
      AllowDevelopmentWithoutDevLicense: Enabled
      AllowAllTrustedApps: Enabled
      Versions: 10.0.18362.0, 10.0.19041.0, 10.0.22000.0, 10.0.22621.0
  IDEs:
    Android Studio: AI-221.6008.13.2211.9514443
    Visual Studio: 16.11.33214.272 (Visual Studio Professional 2019), 17.4.33213.308 (Visual Studio Community 2022)
  Languages:
    Java: 11.0.18 - C:\Program Files\Microsoft\jdk-11.0.18.10-hotspot\bin\javac.EXE
  npmPackages:
    @react-native-community/cli: Not Found
    react: 18.2.0 => 18.2.0
    react-native: 0.71.0 => 0.71.0
    react-native-windows: 0.71.0 => 0.71.0
  npmGlobalPackages:
    *react-native*: Not Found

Start with NativeUIManager.cpp around lines 942-943 and reproduce the issue using the provided hidden-parent and onLayout example with remote debugging enabled and disabled. Trace how NaN width and height reach the onLayout event; done means hidden views no longer produce NaN layout values or crash during serialization.