Skip to content

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

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

Closed
wants to merge 1 commit into 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()).
@rozele rozele requested review from krizzu and tido64 as code owners April 23, 2024 15:38
@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.

Copy link

This PR has been marked as stale due to inactivity. Please address any comments within 7 days or it will be closed.

@github-actions github-actions bot added the Stale label Jun 23, 2024
@github-actions github-actions bot closed this Jun 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants