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

[windows] Add ReactPropertyBag option for DB path initialization #1087

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

rozele
Copy link
Contributor

@rozele rozele commented Apr 23, 2024

Summary

WinAppSDK apps do not support the current methods for setting / retrieving the DB path for the default storage in @react-native-async-storage/async-storage. Specifically, winrt::CoreApplication::Properties() causes a crashes on WinAppSDK.

To work around this, this change adds a specific namespaced key (ReactNativeAsyncStorage.Path) to the ReactPropertyBag for apps to register the overridden DB path for WinAppSDK apps (or apps that otherwise cannot use winrt::CoreApplication::Properties()).

Test Plan

Compiled with WinAppSDK build of react-native-windows, set ReactNativeAsyncStorage.Path on the ReactPropertyBag for InstanceSettings when initializing the react-native-windows ReactHost, and confirmed the sqlite DB is written to the correct location.

WinAppSDK apps do not support the current methods for setting / retrieving the DB path for the default storage in @react-native-async-storage/async-storage. Specifically, winrt::CoreApplication::Properties() causes a crashes on WinAppSDK.

To work around this, this change adds a specific namespaced key (ReactNativeAsyncStorage.Path) to the ReactPropertyBag for apps to register the overridden DB path for WinAppSDK apps (or apps that otherwise cannot use winrt::CoreApplication::Properties()).
@acoates-ms
Copy link

One thing I'd consider is exposing a function to set the Property rather than relying on magic strings.

ReactNativeAsyncStorage::SetDatabasePath(propertyBag, path)

Ideally that would be exposed in the idl file as a public method. -- I dont know how strict the current API is about ensuring that all public APIs are part of the idl interface.

@rozele rozele marked this pull request as draft April 23, 2024 19:47
@rozele
Copy link
Contributor Author

rozele commented Apr 23, 2024

Looks like something happened to the line endings, creating a huge diff. Will need to redo this commit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants