When Using New App Template (with ReactNativeAppBuilder) In-App Defined Native Modules Are Not Automatically Registered #13946
Assignees
Labels
Area: App Template
bug
New Architecture
Broad category for issues that apply to the RN "new" architecture of Turbo Modules + Fabric
Workstream: New Arch Soft Launch
Soft launch the new architecture in 0.76.
Milestone
Comments
microsoft-github-policy-service
bot
added
the
Needs: Triage 🔍
jonthysell
added
Area: App Template
New Architecture
jonthysell
added
the
Workstream: New Arch Soft Launch
jonthysell
removed
the
Needs: Triage 🔍
microsoft-github-policy-service
bot
added
the
Invalid Triage
|
I think it's worse than that, I don't think any turbo modules from any external package providers are getting registered:
|
|
I've figured it out - the new app template registers the package providers, which puts them into the host's instance settings, then, we set the instance settings, which replaces those providers. |
jonthysell
added
bug
and removed
Invalid Triage
jonthysell
added a commit
to jonthysell/react-native-windows
that referenced
this issue
Oct 16, 2024
This PR simplifies and scopes down the API for ReactNativeAppBuilder and ReactNativeWin32App. Closes microsoft#13946
jonthysell
added a commit
that referenced
this issue
Oct 21, 2024
## Description This PR simplifies and scopes down the API for `ReactNativeAppBuilder` and `ReactNativeWin32App`. ### Type of Change - Bug fix (non-breaking change which fixes an issue) ### Why `ReactNativeAppBuilder`'s API surface made it too easy to call incorrectly and not realize it. Resolves #13946 ### What There are a variety of changes to the API surface: * `ReactInstanceSettingsBuilder` deleted: not only are there are simply too many APIs that would need to be exposed to be useful, the very act of creating and replacing the app's `ReactNativeHost`'s `ReactInstanceSettings` with a new one is what caused the bug in #13946 in the first place * `ReactNativeAppBuilder` now only exposes APIs to specify the intial, non-ReactNative, WinAppSDK types, (i.e. `DispatcherQueueController`, `Compositor`, and `AppWindow`), objects the app developer may already have created for their existing app, and otherwise is only responsible for building a `ReactNativeWin32App` with those types properly pre-made * `ReactNativeWin32App::Start()` is now more responsible for the stitching together all of the relevant types to make a working Win32 fabric app * All WinRT APIs without an immediate use-case have been commented out until we are sure they are necessary and that it is safe to expose them * The template has been updated to follow the pattern of: * Use `ReactNativeAppBuilder` to get a `ReactNativeWin32` app with the base WinAppSDK types ready * Get and modify the types as necessary from the created app object (like the `ReactInstanceSettings` and the `AppWindow`) * Call `app.Start()` ## Screenshots N/A ## Testing Verified new apps and example apps in libraryes build and run properly. ## Changelog Should this change be included in the release notes: _yes_ Cleanup ReactNativeAppBuilder and ReactNativeWin32App
jonthysell
added a commit
to jonthysell/react-native-windows
that referenced
this issue
Oct 23, 2024
This PR backports microsoft#13983 to 0.76. ## Description This PR simplifies and scopes down the API for `ReactNativeAppBuilder` and `ReactNativeWin32App`. ### Type of Change - Bug fix (non-breaking change which fixes an issue) ### Why `ReactNativeAppBuilder`'s API surface made it too easy to call incorrectly and not realize it. Resolves microsoft#13946 ### What There are a variety of changes to the API surface: * `ReactInstanceSettingsBuilder` deleted: not only are there are simply too many APIs that would need to be exposed to be useful, the very act of creating and replacing the app's `ReactNativeHost`'s `ReactInstanceSettings` with a new one is what caused the bug in microsoft#13946 in the first place * `ReactNativeAppBuilder` now only exposes APIs to specify the intial, non-ReactNative, WinAppSDK types, (i.e. `DispatcherQueueController`, `Compositor`, and `AppWindow`), objects the app developer may already have created for their existing app, and otherwise is only responsible for building a `ReactNativeWin32App` with those types properly pre-made * `ReactNativeWin32App::Start()` is now more responsible for the stitching together all of the relevant types to make a working Win32 fabric app * All WinRT APIs without an immediate use-case have been commented out until we are sure they are necessary and that it is safe to expose them * The template has been updated to follow the pattern of: * Use `ReactNativeAppBuilder` to get a `ReactNativeWin32` app with the base WinAppSDK types ready * Get and modify the types as necessary from the created app object (like the `ReactInstanceSettings` and the `AppWindow`) * Call `app.Start()` ## Screenshots N/A ## Testing Verified new apps and example apps in libraryes build and run properly. ## Changelog Should this change be included in the release notes: _yes_ Cleanup ReactNativeAppBuilder and ReactNativeWin32App
jonthysell
added a commit
that referenced
this issue
Oct 24, 2024
This PR backports #13983 to 0.76. ## Description This PR simplifies and scopes down the API for `ReactNativeAppBuilder` and `ReactNativeWin32App`. ### Type of Change - Bug fix (non-breaking change which fixes an issue) ### Why `ReactNativeAppBuilder`'s API surface made it too easy to call incorrectly and not realize it. Resolves #13946 ### What There are a variety of changes to the API surface: * `ReactInstanceSettingsBuilder` deleted: not only are there are simply too many APIs that would need to be exposed to be useful, the very act of creating and replacing the app's `ReactNativeHost`'s `ReactInstanceSettings` with a new one is what caused the bug in #13946 in the first place * `ReactNativeAppBuilder` now only exposes APIs to specify the intial, non-ReactNative, WinAppSDK types, (i.e. `DispatcherQueueController`, `Compositor`, and `AppWindow`), objects the app developer may already have created for their existing app, and otherwise is only responsible for building a `ReactNativeWin32App` with those types properly pre-made * `ReactNativeWin32App::Start()` is now more responsible for the stitching together all of the relevant types to make a working Win32 fabric app * All WinRT APIs without an immediate use-case have been commented out until we are sure they are necessary and that it is safe to expose them * The template has been updated to follow the pattern of: * Use `ReactNativeAppBuilder` to get a `ReactNativeWin32` app with the base WinAppSDK types ready * Get and modify the types as necessary from the created app object (like the `ReactInstanceSettings` and the `AppWindow`) * Call `app.Start()` ## Screenshots N/A ## Testing Verified new apps and example apps in libraryes build and run properly. ## Changelog Should this change be included in the release notes: _yes_ Cleanup ReactNativeAppBuilder and ReactNativeWin32App
acoates-ms
pushed a commit
to acoates-ms/react-native-windows
that referenced
this issue
Nov 2, 2024
This PR simplifies and scopes down the API for `ReactNativeAppBuilder` and `ReactNativeWin32App`. - Bug fix (non-breaking change which fixes an issue) `ReactNativeAppBuilder`'s API surface made it too easy to call incorrectly and not realize it. Resolves microsoft#13946 There are a variety of changes to the API surface: * `ReactInstanceSettingsBuilder` deleted: not only are there are simply too many APIs that would need to be exposed to be useful, the very act of creating and replacing the app's `ReactNativeHost`'s `ReactInstanceSettings` with a new one is what caused the bug in microsoft#13946 in the first place * `ReactNativeAppBuilder` now only exposes APIs to specify the intial, non-ReactNative, WinAppSDK types, (i.e. `DispatcherQueueController`, `Compositor`, and `AppWindow`), objects the app developer may already have created for their existing app, and otherwise is only responsible for building a `ReactNativeWin32App` with those types properly pre-made * `ReactNativeWin32App::Start()` is now more responsible for the stitching together all of the relevant types to make a working Win32 fabric app * All WinRT APIs without an immediate use-case have been commented out until we are sure they are necessary and that it is safe to expose them * The template has been updated to follow the pattern of: * Use `ReactNativeAppBuilder` to get a `ReactNativeWin32` app with the base WinAppSDK types ready * Get and modify the types as necessary from the created app object (like the `ReactInstanceSettings` and the `AppWindow`) * Call `app.Start()` N/A Verified new apps and example apps in libraryes build and run properly. Should this change be included in the release notes: _yes_ Cleanup ReactNativeAppBuilder and ReactNativeWin32App
acoates-ms
added a commit
that referenced
this issue
Nov 5, 2024
* [Fabric] Introducing autocapitalize prop in TextInput - Take 2 (#13343) * New implementation of autocapitalize! * Change files * Fixed bug for sentences scenario * Just keep characters mode for now * Revert "Just keep characters mode for now" This reverts commit 60ca1ce. * Re-apply changes minus packages.json.lock * The original js file was deleted, re-applying changes * Fixed snapshot and lint errors * Fix override mismatch, added comments * Remove stale test check * Minor changes * Update obsolete snapshot * Cleanup ReactNativeAppBuilder and ReactNativeWin32App (#13983) This PR simplifies and scopes down the API for `ReactNativeAppBuilder` and `ReactNativeWin32App`. - Bug fix (non-breaking change which fixes an issue) `ReactNativeAppBuilder`'s API surface made it too easy to call incorrectly and not realize it. Resolves #13946 There are a variety of changes to the API surface: * `ReactInstanceSettingsBuilder` deleted: not only are there are simply too many APIs that would need to be exposed to be useful, the very act of creating and replacing the app's `ReactNativeHost`'s `ReactInstanceSettings` with a new one is what caused the bug in #13946 in the first place * `ReactNativeAppBuilder` now only exposes APIs to specify the intial, non-ReactNative, WinAppSDK types, (i.e. `DispatcherQueueController`, `Compositor`, and `AppWindow`), objects the app developer may already have created for their existing app, and otherwise is only responsible for building a `ReactNativeWin32App` with those types properly pre-made * `ReactNativeWin32App::Start()` is now more responsible for the stitching together all of the relevant types to make a working Win32 fabric app * All WinRT APIs without an immediate use-case have been commented out until we are sure they are necessary and that it is safe to expose them * The template has been updated to follow the pattern of: * Use `ReactNativeAppBuilder` to get a `ReactNativeWin32` app with the base WinAppSDK types ready * Get and modify the types as necessary from the created app object (like the `ReactInstanceSettings` and the `AppWindow`) * Call `app.Start()` N/A Verified new apps and example apps in libraryes build and run properly. Should this change be included in the release notes: _yes_ Cleanup ReactNativeAppBuilder and ReactNativeWin32App * Update Guardrails on Provider Instantiation (#13961) * Update Provider Guardrails * Change files * Format * Update Snapshots * Add very basic box-shadow support (#14028) * Add very basic box-shadow support * Change files * Focus should notify island host when tab loop wraps to give host oportunity to take focus (#14026) * Focus should notify island host when tab loop wraps to give host oportunity to take focus * Change files * Default scroll to bring a component into view should have padding around the viewport (#14018) * Default scroll to bring a component into view should have padding around the viewport * Change files * Update focus visuals to use cornerRadius and inner/outer strokes (#14008) * Update focus visuals to use cornerRadius and inner/outer strokes. * Change files * Format * lint fix * Scale focus border for scaleFactor * [Fabric] Get Modal to host RN components in new hwnd (#13500) * save state * add example * build but blank page still :( * clean up comments * visuals show up in new hwnd! * clean up code * better naming and unfork Modal examples * testing save state * Make the RN island a Modal member var * Failed attempt at skipping root view in CEH, leaving it for learning purposes * you can click on UI! * clean up code * Change files * save state * remove hardcoded rootTag * add width/height to example * add test * revert simple.tsx * remove test * update snapshot * feedback part 1: make Modal a RootComponentView * feedback part2: simplify MountChildren * fix deleting modal * feedback round2 * remove comment * remove imports * feedback part 3 * fix overrides * add simple layout - still has issues with padding/flex * feedback part4 * lint * update overrides * Change files * feedback --------- Co-authored-by: Daniel Ayala <[email protected]> * Support accessibilityState 'checked' (#13962) * Implement accessibilityState checked * Change files * Add Testing * Format and Update Snapshots * Adjust Guardrails * Merge * Format * Format * Lint * Change files * Fix Merge Error * Fix issue with prop cloning with custom native props (#14061) * Fix issue with prop cloning with custom native props * format * prettier * Change files --------- Co-authored-by: React-Native-Windows Bot <[email protected]> * change files * Build fixes * fix * fix * snapshot --------- Co-authored-by: Daniel Ayala <[email protected]> Co-authored-by: Jon Thysell <[email protected]> Co-authored-by: Chiara Mooney <[email protected]> Co-authored-by: Tatiana Kapos <[email protected]> Co-authored-by: React-Native-Windows Bot <[email protected]>
acoates-ms
pushed a commit
to acoates-ms/react-native-windows
that referenced
this issue
Nov 12, 2024
This PR simplifies and scopes down the API for `ReactNativeAppBuilder` and `ReactNativeWin32App`. - Bug fix (non-breaking change which fixes an issue) `ReactNativeAppBuilder`'s API surface made it too easy to call incorrectly and not realize it. Resolves microsoft#13946 There are a variety of changes to the API surface: * `ReactInstanceSettingsBuilder` deleted: not only are there are simply too many APIs that would need to be exposed to be useful, the very act of creating and replacing the app's `ReactNativeHost`'s `ReactInstanceSettings` with a new one is what caused the bug in microsoft#13946 in the first place * `ReactNativeAppBuilder` now only exposes APIs to specify the intial, non-ReactNative, WinAppSDK types, (i.e. `DispatcherQueueController`, `Compositor`, and `AppWindow`), objects the app developer may already have created for their existing app, and otherwise is only responsible for building a `ReactNativeWin32App` with those types properly pre-made * `ReactNativeWin32App::Start()` is now more responsible for the stitching together all of the relevant types to make a working Win32 fabric app * All WinRT APIs without an immediate use-case have been commented out until we are sure they are necessary and that it is safe to expose them * The template has been updated to follow the pattern of: * Use `ReactNativeAppBuilder` to get a `ReactNativeWin32` app with the base WinAppSDK types ready * Get and modify the types as necessary from the created app object (like the `ReactInstanceSettings` and the `AppWindow`) * Call `app.Start()` N/A Verified new apps and example apps in libraryes build and run properly. Should this change be included in the release notes: _yes_ Cleanup ReactNativeAppBuilder and ReactNativeWin32App
acoates-ms
added a commit
that referenced
this issue
Nov 23, 2024
* [Fabric] Introducing autocapitalize prop in TextInput - Take 2 (#13343) * New implementation of autocapitalize! * Change files * Fixed bug for sentences scenario * Just keep characters mode for now * Revert "Just keep characters mode for now" This reverts commit 60ca1ce. * Re-apply changes minus packages.json.lock * The original js file was deleted, re-applying changes * Fixed snapshot and lint errors * Fix override mismatch, added comments * Remove stale test check * Minor changes * Update obsolete snapshot * Cleanup ReactNativeAppBuilder and ReactNativeWin32App (#13983) This PR simplifies and scopes down the API for `ReactNativeAppBuilder` and `ReactNativeWin32App`. - Bug fix (non-breaking change which fixes an issue) `ReactNativeAppBuilder`'s API surface made it too easy to call incorrectly and not realize it. Resolves #13946 There are a variety of changes to the API surface: * `ReactInstanceSettingsBuilder` deleted: not only are there are simply too many APIs that would need to be exposed to be useful, the very act of creating and replacing the app's `ReactNativeHost`'s `ReactInstanceSettings` with a new one is what caused the bug in #13946 in the first place * `ReactNativeAppBuilder` now only exposes APIs to specify the intial, non-ReactNative, WinAppSDK types, (i.e. `DispatcherQueueController`, `Compositor`, and `AppWindow`), objects the app developer may already have created for their existing app, and otherwise is only responsible for building a `ReactNativeWin32App` with those types properly pre-made * `ReactNativeWin32App::Start()` is now more responsible for the stitching together all of the relevant types to make a working Win32 fabric app * All WinRT APIs without an immediate use-case have been commented out until we are sure they are necessary and that it is safe to expose them * The template has been updated to follow the pattern of: * Use `ReactNativeAppBuilder` to get a `ReactNativeWin32` app with the base WinAppSDK types ready * Get and modify the types as necessary from the created app object (like the `ReactInstanceSettings` and the `AppWindow`) * Call `app.Start()` N/A Verified new apps and example apps in libraryes build and run properly. Should this change be included in the release notes: _yes_ Cleanup ReactNativeAppBuilder and ReactNativeWin32App * Update Guardrails on Provider Instantiation (#13961) * Update Provider Guardrails * Change files * Format * Update Snapshots * Add very basic box-shadow support (#14028) * Add very basic box-shadow support * Change files * Focus should notify island host when tab loop wraps to give host oportunity to take focus (#14026) * Focus should notify island host when tab loop wraps to give host oportunity to take focus * Change files * Default scroll to bring a component into view should have padding around the viewport (#14018) * Default scroll to bring a component into view should have padding around the viewport * Change files * Update focus visuals to use cornerRadius and inner/outer strokes (#14008) * Update focus visuals to use cornerRadius and inner/outer strokes. * Change files * Format * lint fix * Scale focus border for scaleFactor * [Fabric] Get Modal to host RN components in new hwnd (#13500) * save state * add example * build but blank page still :( * clean up comments * visuals show up in new hwnd! * clean up code * better naming and unfork Modal examples * testing save state * Make the RN island a Modal member var * Failed attempt at skipping root view in CEH, leaving it for learning purposes * you can click on UI! * clean up code * Change files * save state * remove hardcoded rootTag * add width/height to example * add test * revert simple.tsx * remove test * update snapshot * feedback part 1: make Modal a RootComponentView * feedback part2: simplify MountChildren * fix deleting modal * feedback round2 * remove comment * remove imports * feedback part 3 * fix overrides * add simple layout - still has issues with padding/flex * feedback part4 * lint * update overrides * Change files * feedback --------- Co-authored-by: Daniel Ayala <[email protected]> * Support accessibilityState 'checked' (#13962) * Implement accessibilityState checked * Change files * Add Testing * Format and Update Snapshots * Adjust Guardrails * Merge * Format * Format * Lint * Change files * Fix Merge Error * Fix issue with prop cloning with custom native props (#14061) * Fix issue with prop cloning with custom native props * format * prettier * Change files --------- Co-authored-by: React-Native-Windows Bot <[email protected]> * Export MS.RN.Color ctor in Office dll (#14082) * Export MS.RN.Color ctor in Office dll * Change files * Implement TxScreenToClient and TxClientToScreen * format --------- Co-authored-by: React-Native-Windows Bot <[email protected]> * TextInput caret becomes visible on non-focused TextInputs on resize (#14091) * TextInput caret becomes visible on non-focused TextInputs on resize * Change files --------- Co-authored-by: React-Native-Windows Bot <[email protected]> * Fix focus visuals being obscured by adjacent views (#14093) * Fix focus visuals being obscured by adjacent views * Change files * update snapshots * Fix uimplemented view * review feedback --------- Co-authored-by: React-Native-Windows Bot <[email protected]> * change files * Build fixes * fix * Update Test Website to dotnet8 * update snapshots * fix overrides --------- Co-authored-by: Daniel Ayala <[email protected]> Co-authored-by: Jon Thysell <[email protected]> Co-authored-by: Chiara Mooney <[email protected]> Co-authored-by: Tatiana Kapos <[email protected]> Co-authored-by: React-Native-Windows Bot <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
When working on my AI x RN sample app which uses the new app template for Fabric with ReactNativeAppBuilder, the code I used previously to defined a native module within my app's code no longer works. The module returns as null in my JS. When I switch my app back to the old app template the native module is registered and can be accessed from my JS.
Is it still expected behavior for app's using ReactNativeAppBuilder to have in-app defined native module be automatically registered? If so, it looks like there is a bug here. If not, we should document the change in behavior somewhere.
The text was updated successfully, but these errors were encountered: