-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Xaml support prototype #14811
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
base: main
Are you sure you want to change the base?
Xaml support prototype #14811
Conversation
@@ -83,6 +83,17 @@ | |||
"boost": "[1.83.0, )" | |||
} | |||
}, | |||
"microsoft.reactnative.xaml": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure why this entry is here -- "microsoft.reactnative.xaml" isn't a package, is it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lock files include project dependencies
@@ -39,6 +41,9 @@ _Use_decl_annotations_ int CALLBACK WinMain(HINSTANCE instance, HINSTANCE, PSTR | |||
RegisterAutolinkedNativeModulePackages(settings.PackageProviders()); | |||
// Register any native modules defined within this app project | |||
settings.PackageProviders().Append(winrt::make<CompReactPackageProvider>()); | |||
// TODO: Can we make apps register this automatically? (e.g. with autolinking) | |||
// TODO: But ideally we don't load the M.RN.Xaml.dll at all if we're not using Xaml. | |||
settings.PackageProviders().Append(winrt::Microsoft::ReactNative::Xaml::ReactPackageProvider()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TODO: I don't know how to make this automatically happen with autolinking or whatever. Can the xaml-calendar-view package do this somehow? OTOH, if the app's not using Xaml, I don't want it to load Microsoft.ReactNative.Xaml.dll. Not sure the best way to handle this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We would probably want to update autolinking to conditionally add the winrt::Microsoft::ReactNative::Xaml::ReactPackageProvider()
to be loaded in the generated AutoLinkedNativeModules.g.cpp
file.
We can either detect it needs to be there during autolinking (modules might register that they need it) and inject it there, OR we could just always put it in, behind a #ifdef RNW_NEW_ARCH_XAML_SUPPORT
, powered by an aforementioned RnwNewArchXamlSupport
variable.
); | ||
} | ||
|
||
export { CalendarView, RawCalendarView }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The idea here is that the CalendarView object is already wrapped in a XamlHost, so is all ready to be put into a JSX tree. RawCalendarView is not. In the future maybe we make it possible to group multiple "Raw" elements under the same XamlHost.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you had a tree of Xaml elements, would you expect something like this? (maybe not supporting layout panels with multiple children, but other controls that have 1 child)
<XamlHost>
<RawStackPanel>
<RawButton />
</RawStackPanel>
</XamlHost>
}; | ||
|
||
[webhosthidden] | ||
interface IXamlControl |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Implement this if your wrapping a Xaml object and want to be hostable in a XamlHost.
@@ -83,6 +83,17 @@ | |||
"boost": "[1.83.0, )" | |||
} | |||
}, | |||
"microsoft.reactnative.xaml": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lock files include project dependencies
@@ -18,5 +18,13 @@ | |||
<Name>Microsoft.ReactNative</Name> | |||
<Private Condition="'$(ConfigurationType)' != 'Application'">false</Private> | |||
</ProjectReference> | |||
|
|||
<!-- TODO: Should we only include this project ref conditionally? --> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This files is used by any external app/lib that tries to build RNW from source - and for NewArch we're trying to get everyone to build on the NuGets by default. The only apps that are building NewArch from source should be the ones inside this repo.
I would definitely put this in a conditional behind a new variable, maybe RnwNewArchXamlSupport
? We could also use that variable to control a package reference (depending on if we publish this as a separate nuget or just as part of the existing nuget)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
peferably XamlApplication.idl?
@@ -39,6 +41,9 @@ _Use_decl_annotations_ int CALLBACK WinMain(HINSTANCE instance, HINSTANCE, PSTR | |||
RegisterAutolinkedNativeModulePackages(settings.PackageProviders()); | |||
// Register any native modules defined within this app project | |||
settings.PackageProviders().Append(winrt::make<CompReactPackageProvider>()); | |||
// TODO: Can we make apps register this automatically? (e.g. with autolinking) | |||
// TODO: But ideally we don't load the M.RN.Xaml.dll at all if we're not using Xaml. | |||
settings.PackageProviders().Append(winrt::Microsoft::ReactNative::Xaml::ReactPackageProvider()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We would probably want to update autolinking to conditionally add the winrt::Microsoft::ReactNative::Xaml::ReactPackageProvider()
to be loaded in the generated AutoLinkedNativeModules.g.cpp
file.
We can either detect it needs to be there during autolinking (modules might register that they need it) and inject it there, OR we could just always put it in, behind a #ifdef RNW_NEW_ARCH_XAML_SUPPORT
, powered by an aforementioned RnwNewArchXamlSupport
variable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't want another file here, we already have one at the repo root to keep all of the projects aligned.
<ClInclude Include="pch.h" /> | ||
</ItemGroup> | ||
<ItemGroup> | ||
<Midl Include="Class.idl" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Out of date files
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if we need this to be it's own solution - maybe fine for now, will depend on how we want to package the dll
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If XamlCalendarView doesn't have any non-UI functionality, you can probably remove the struct here.
<AppContainerApplication>false</AppContainerApplication> | ||
<ApplicationType>Windows Store</ApplicationType> | ||
<ApplicationTypeRevision>10.0</ApplicationTypeRevision> | ||
<WindowsTargetPlatformVersion Condition=" '$(WindowsTargetPlatformVersion)' == '' ">10.0.22621.0</WindowsTargetPlatformVersion> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't want to hardcode the SDK versions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does having this type here preclude app authors from having their own Xaml app? I.e. A classic WinAppSDK app with it's own App, using a ReactNativeIsland, to put RNW in just part of the UI, then have that RNW also load Xaml?
Here's the idea:
Microsoft Reviewers: Open in CodeFlow