Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sample for Multiple Windows #757

Open
chiaramooney opened this issue Aug 29, 2022 · 7 comments
Open

Sample for Multiple Windows #757

chiaramooney opened this issue Aug 29, 2022 · 7 comments
Labels
enhancement New feature or request samples
Milestone

Comments

@chiaramooney
Copy link
Collaborator

Summary

Filing issue to raise discussion on whether we officially want to support the multiple windows scenario for RNW. If so, let's create a sample and make sure this configuration is tested in our release validation.

Motivation

Basic Example

No response

Open Questions

No response

@chiaramooney chiaramooney added the enhancement New feature or request label Aug 29, 2022
@ghost ghost added the Needs: Triage 🔍 label Aug 29, 2022
@chrisglein
Copy link
Member

Two questions here:

  • What is promoted as a "sample" (as in code to copy and a well supported scenario?
  • What is covered by a test app (so we don't break apps that are following that pattern)

Our primary goal is the latter, to test scenarios that are important to popular apps. This scenario isn't enough of a golden path for us to actively promote it.

As for coverage, Messenger would need Win32 XAML island multi-window apps, which PlaygroundWin32 does. So it exists, but has no automated test coverage. Today there's no Win32 version of the E2E test app.

@chiaramooney Can you identify what the essential cases to capture here are if there are other apps that are effected? We need to evaluate how much to cover a narrow app scenario vs. something more broad.

@rozele
Copy link

rozele commented Sep 3, 2022

@chrisglein - Playground-win32 doesn't exactly have Messenger's scenario. Messenger uses a single UI thread for all windows (which as I understand is a common pattern for win32 apps, I think based on a comment from @acoates-ms). The Playground-win32 app spins up a new thread (and separate React instance) for each window.

Also, creating multiple windows in Playground-win32 has crashed the last few times I tried to use it ;).

@rozele
Copy link

rozele commented Sep 3, 2022

Here are some variables to consider for multi-window applications and we'll have to decide what's in scope for react-native-windows:

  1. Does each window share a React instance / JavaScript runtime, or are they separate instances per window?
  2. Do all windows run on the same UI thread, or is there a per window UI thread?
  3. Is the multi-window scenario for XAML Islands, WinAppSdk, and/or UWP?

The first variable is pretty trivial. We can probably test issues with apps using multiple React instances without multiple windows. The only kinds of issues you would encounter here are issues with static / otherwise global variables. The more interesting case, and the case that I suspect most applications would want to use, is a shared React instance, especially for common patterns in React apps, like using centralized state management with tools like Redux or MobX.

The second variable is interesting for both UWP and win32 applications. As of 18362, AppWindow instances allow running multiple windows on the same UI thread, so technically speaking you can have react-native-windows UWP apps on multiple windows. If we wanted to support multi-window apps before 18362, we'd need to use ApplicationView (with it's own CoreDispatcher) and overhaul the UIManager and a few native modules to eliminate the single UI thread assumption built into react-native-windows.

The third variable is just what type of samples / examples / tests should be provided. Can we cover the whole surface with just AppWindow tests, or should there also be XAML Islands tests? There are various callsites for CoreWindow::GetForCurrentThread alongside XamlRoot callsites, so we'd probably want to exercise these separately.

@rozele
Copy link

rozele commented Sep 5, 2022

So, what I would recommend is that we officially support multiple windows in both UWP and XAML Islands, with the strict requirement that they’re in on a single UI thread.

A good way to exercise this would be to add a separate Sample to Playground and a custom native module in the Playground projects. We could also put it in RNTester, but I think it’s better to keep RNTester limited to core native modules and views (though we already have an exception for picker).

So, we’d need:

  1. A new project for Playground React package provider.
  2. A custom native module for creating new windows. We could either inject the behavior to create a new window so we can create identical chrome for Playground and Playground-win32 as the initial window, or just create a simple window in the module. Either the default or injected behavior should create an AppWindow for UWP or a new window on the same thread where XAML Islands has already been initialized for XAML Islands.
  3. A sample that invokes the native module and renders a sample into the new window.

@ghost
Copy link

ghost commented Sep 12, 2022

This issue has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 7 days. It will be closed if no further activity occurs within 7 days of this comment.

@ghost ghost closed this as completed Sep 19, 2022
@rozele rozele reopened this Sep 26, 2022
@ghost
Copy link

ghost commented Oct 3, 2022

This issue has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 7 days. It will be closed if no further activity occurs within 7 days of this comment.

@ghost ghost closed this as completed Oct 10, 2022
@rozele rozele reopened this Nov 17, 2022
@ghost
Copy link

ghost commented Nov 24, 2022

This issue has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 7 days. It will be closed if no further activity occurs within 7 days of this comment.

@ghost ghost closed this as completed Dec 2, 2022
@chrisglein chrisglein reopened this Dec 12, 2022
@chrisglein chrisglein transferred this issue from microsoft/react-native-windows Dec 12, 2022
@ghost ghost added the Needs: Triage 🔍 label Dec 12, 2022
@chrisglein chrisglein added this to the Backlog milestone Dec 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request samples
Projects
None yet
Development

No branches or pull requests

3 participants