microsoft / react-native-windows

A framework for building native Windows apps with React.
https://microsoft.github.io/react-native-windows/
Other
16.33k stars 1.14k forks source link

Creating a Popup with more than one child causes app to hit an assert in PopupShadowNode::AddView #11173

Open chrisglein opened 1 year ago

chrisglein commented 1 year ago

Problem Description

Point of failure (index is 1)

void PopupShadowNode::AddView(ShadowNode &child, int64_t index) {
  if (index != 0) {
    assert(false);
    return;
  }

Callstack

>   Microsoft.ReactNative.dll!Microsoft::ReactNative::PopupShadowNode::AddView(Microsoft::ReactNative::ShadowNode & child, __int64 index) Line 125  C++
    Microsoft.ReactNative.dll!Microsoft::ReactNative::UIManagerModule::setChildren(__int64 containerTag, winrt::Microsoft::ReactNative::JSValueArray && reactTags) Line 316 C++
    Microsoft.ReactNative.dll!Microsoft::ReactNative::UIManager::setChildren::__l2::<lambda_1>::operator()() Line 790   C++
    Microsoft.ReactNative.dll!Mso::Details::FunctionObjectWrapper<`Microsoft::ReactNative::UIManager::setChildren'::`2'::<lambda_1>,void>::Invoke() Line 165    C++
    Microsoft.ReactNative.dll!Mso::Functor<void __cdecl(void)>::operator()() Line 412   C++
    [External Code] 
    Microsoft.ReactNative.dll!Microsoft::ReactNative::BatchingQueueCallInvoker::PostBatch::__l5::<lambda_1>::operator()() Line 46   C++
    [External Code] 
    Microsoft.ReactNative.dll!Mso::React::MessageDispatchQueue2::tryFunc(const std::function<void __cdecl(void)> & func) Line 120   C++
    Microsoft.ReactNative.dll!Mso::React::MessageDispatchQueue2::runOnQueue::__l2::<lambda_1>::operator()() Line 115    C++
    Microsoft.ReactNative.dll!Mso::Details::FunctionObjectWrapper<`Mso::React::MessageDispatchQueue2::runOnQueue'::`2'::<lambda_1>,void>::Invoke() Line 166 C++
    Microsoft.ReactNative.dll!Mso::QueueService::InvokeTask(Mso::Functor<void __cdecl(void)> && task, std::optional<std::chrono::time_point<std::chrono::steady_clock,std::chrono::duration<__int64,std::ratio<1,1000000000>>>> endTime) Line 208   C++
    Microsoft.ReactNative.dll!Mso::TaskDispatcherHandler::Invoke() Line 173 C++

Steps To Reproduce

Here's the part of the code that produced this error:

function Section({children, title}: SectionProps): JSX.Element {
  const [showPopup, setShowPopup] = React.useState(false);

  return (
    <View style={[styles.sectionContainer, styles.aiSection]}>
      <View style={{flexDirection: 'row'}}>
        <Text style={[styles.sectionTitle, {flexGrow: 1}]}>TITLE</Text>
        <FeedbackButton content="👍" onPress={() => { setShowPopup(true); console.log("like"); }}/>
        <FeedbackButton content="👎" onPress={() => { setShowPopup(true); console.log("dislike"); }}/>
        <Popup isOpen={showPopup}>
          <Text>Provide additional feedback</Text>
          <TextInput></TextInput>
          <Button title="Submit feedback" onPress={() => setShowPopup(false)}/>
        </Popup>
      </View>
      {children}
    </View>
  );
}

There's a problem with this: Popup only expects 1 child, not a list of children. But this manifests as an assert instead of a clear error.

Expected Results

Not an assert (which in debug is just silently taking down the whole app - without attaching a debugger nothing is shown)

CLI version

error: unknown option '--version'

Environment

info Fetching system and libraries information...
System:
    OS: Windows 10 10.0.23145
    CPU: (24) x64 AMD Ryzen Threadripper PRO 3945WX 12-Cores
    Memory: 38.63 GB / 63.86 GB
  Binaries:
    Node: 16.17.0 - C:\Program Files\nodejs\node.EXE
    Yarn: 1.22.19 - C:\Program Files (x86)\Yarn\bin\yarn.CMD
    npm: 9.4.1 - C:\Program Files\nodejs\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: Not Found
    Visual Studio: 17.4.33213.308 (Visual Studio Community 2022), 16.11.33214.272 (Visual Studio Community 2019)
  Languages:
    Java: Not Found
  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

Target Platform Version

None

Target Device(s)

Desktop

Visual Studio Version

Visual Studio 2022

Build Configuration

Debug

Snack, code example, screenshot, or link to a repository

No response

rozele commented 1 year ago

Seems like it would be fairly simply to just embed a ViewPanel as the top-level child of the Popup component. Have you also checked the Flyout component? I believe it may also have a 1 child assumption: https://github.com/microsoft/react-native-windows/blob/main/vnext/Microsoft.ReactNative/Views/FlyoutViewManager.cpp#L159