-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Do not merge: Xaml in RNW prototyping #14649
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?
Conversation
@@ -39,6 +41,10 @@ _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. |
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.
Hmm, that would actually be an interesting thing to try to get autolinking to do.
Right now auto linking would end up loading the dll. -- Having said that, I wouldn't expect M.RN.Xaml.dll to be particularly big. We will want to make sure that MUX doesnt' get loaded until you use a control though.
@@ -39,6 +41,10 @@ _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) |
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.
To get autolinking to do its thing, we'd need to move the rn-xaml JS code into a new npm package, and then autolinking would bring in this dependency if the app adds that new package to their dependencies.
@@ -124,6 +124,12 @@ | |||
<Import Project="$(ReactNativeWindowsDir)\PropertySheets\External\Microsoft.ReactNative.Composition.CppApp.targets" Condition="Exists('$(ReactNativeWindowsDir)\PropertySheets\External\Microsoft.ReactNative.Composition.CppApp.targets')" /> | |||
</ImportGroup> | |||
<ItemGroup> | |||
<!-- TODO: Can we make this happen automatically, like with autolinking? --> |
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.
Yup, autolinking can do this. But again needs a separate npm package.
@@ -0,0 +1,259 @@ | |||
#pragma once | |||
|
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 can probably get most of this file codegen'd from a spec file.
Can we create a FabricXamlNativeComponent.js?
Or do we want a separate spec for each Xaml Component.... - We probably do since they will all have different properties.
then the M.RN.Xaml dll would provide the helpers to make it easy to implement those components.
auto xamlControl = args.Child().UserData().as<XamlControlComponentView>(); | ||
if (xamlControl) { | ||
auto xamlElement = xamlControl->GetXamlElement(); | ||
m_xamlIsland.Content(xamlElement); |
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 guess we should verify that there is only ever one child attempting to be added to the host.
@@ -178,14 +178,17 @@ ContentIslandComponentView::~ContentIslandComponentView() noexcept { | |||
void ContentIslandComponentView::MountChildComponentView( | |||
const winrt::Microsoft::ReactNative::ComponentView &childComponentView, | |||
uint32_t index) noexcept { | |||
assert(false); | |||
// I assume this is here b/c it doesn't make sense for a Island CV to have children. | |||
// XamlHost does have children, but the children will represent Xaml elements and |
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.
Yeah, we can probably remove the asserts here.
|
||
const nativeComponentRegistry = require('react-native/Libraries/NativeComponent/NativeComponentRegistry'); | ||
|
||
const StackPanel = nativeComponentRegistry.get('StackPanel', () => ({ |
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 we had spec files for these, then we'd be able to export them, and we'd get better type information for the users of these components.
Microsoft Reviewers: Open in CodeFlow