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

fix(windows): CompositionRootView was renamed to ReactNativeIsland #2067

Merged
merged 1 commit into from
Jun 17, 2024

Conversation

tido64
Copy link
Member

@tido64 tido64 commented Jun 7, 2024

Description

CompositionRootView was renamed to ReactNativeIsland in microsoft/react-native-windows@af10f99

Platforms affected

  • Android
  • iOS
  • macOS
  • visionOS
  • Windows

Test plan

yarn clean
npm run set-react-version canary-windows
yarn
cd example
yarn install-windows-test-app --use-fabric
yarn windows

@tido64 tido64 requested a review from JasonVMo June 7, 2024 06:27
@tido64 tido64 requested review from acoates-ms and kelset as code owners June 7, 2024 06:27
@github-actions github-actions bot added the platform: Windows This affects Windows label Jun 7, 2024
@acoates-ms
Copy link
Contributor

What is your support matrix for the fabric version of this code?

For 0.74 specifically we are back pointing large chunks of our fabric work. The APIs are all marked experimental, and so are not guaranteed to be stable.

In particular microsoft/react-native-windows#13319 will apply this rename to 0.74 too. Do you want to do a patch version check to have the code work on older 0.74 versions, or just update the code for 0.74 too?

The APIs to size the Island also changed a little while back, so I dont think this will compile?

@tido64
Copy link
Member Author

tido64 commented Jun 10, 2024

What is your support matrix for the fabric version of this code?

Since this is experimental, I don't think we want to support more than the very latest. Since you've backported it, I'll just use the new name directly.

The APIs to size the Island also changed a little while back, so I dont think this will compile?

I can't even get latest nightly to build atm so I will have to wait till a new 0.74 is published to verify.

image

@tido64 tido64 marked this pull request as draft June 10, 2024 11:20
@tido64 tido64 force-pushed the tido/windows/rename-CompositionRootView branch 2 times, most recently from 14d4633 to 9f86365 Compare June 13, 2024 13:44
@tido64 tido64 force-pushed the tido/windows/rename-CompositionRootView branch from 9f86365 to dea9285 Compare June 13, 2024 14:05
@tido64
Copy link
Member Author

tido64 commented Jun 13, 2024

Apart from microsoft/react-native-windows#13344 and #2085, I'm hitting these build errors and I have no idea why:

 × Building Solution: Package.appxmanifest(51,27): error APPX0703: Manifest references file 'Images\SplashScreen....
 × Build failed with message 2:6>Package.appxmanifest(47,9): error APPX0703: Manifest references file 'Images\Square150x
150Logo.png' which is not part of the payload. [D:\react-native-test-app\example\node_modules\.generated\windows\Win32\R
eactApp.Package.wapproj]. Check your build configuration.

And if I build from Visual Studio:

3>Package.appxmanifest : error APPX0703: Manifest references file 'ReactApp\Microsoft.ReactNative.dll' which is not part of the payload.
3>Package.appxmanifest : error APPX0703: Manifest references file 'ReactApp\ReactNativeWebStorage.dll' which is not part of the payload.

It builds successfully, I think. It's just the packaging that fails.

@tido64 tido64 marked this pull request as ready for review June 13, 2024 14:22
@tido64 tido64 force-pushed the tido/windows/rename-CompositionRootView branch from dea9285 to 6b9a5fe Compare June 14, 2024 10:10
@tido64
Copy link
Member Author

tido64 commented Jun 17, 2024

Merging this for now since the packaging issue seems unrelated.

@tido64 tido64 merged commit aa185a8 into trunk Jun 17, 2024
30 checks passed
@tido64 tido64 deleted the tido/windows/rename-CompositionRootView branch June 17, 2024 06:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
platform: Windows This affects Windows
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants