Skip to content

Allow consumption of RCTAsyncStorage as TurboModule #965

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Jun 6, 2023
Merged

Allow consumption of RCTAsyncStorage as TurboModule #965

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
577cd24
Allow consumption of RCTAsyncStorage as TurboModule
rozele May 26, 2023
ef813bc
Remove spurious semi-colon
rozele May 26, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 10 additions & 7 deletions src/RCTAsyncStorage.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,18 @@
import { NativeModules, TurboModuleRegistry } from 'react-native';
import { shouldFallbackToLegacyNativeModule } from './shouldFallbackToLegacyNativeModule';

let RCTAsyncStorage =
NativeModules['PlatformLocalStorage'] || // Support for external modules, like react-native-windows
NativeModules['RNC_AsyncSQLiteDBStorage'] ||
NativeModules['RNCAsyncStorage'];
// TurboModuleRegistry falls back to NativeModules so we don't have to try go
// assign NativeModules' counterparts if TurboModuleRegistry would resolve
// with undefined.
let RCTAsyncStorage = TurboModuleRegistry
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we should just bump the minimum requirement to 0.61 so we don't have to do this check. What do you think @krizzu?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm fine with the change, although that means we'd have to keep a compatibility table in readme (so that, we show which version is supported by which RN version). I guess it's safe to say we support 0.60 and up version

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, let's do this after this PR merges. Can you fix the Release job? It looks like it's unable to fetch user and password for some reason.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tido64 sorry, missed the message
I created PR to separate release from Pull Request checks:
#972

The PRs from external contributor fails, because of nature of "pull_request" trigger preventing access to Secrets and limiting GH token to READ only

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I didn't realize this was an issue. Thanks for looking into it. I guess that means we can merge this now?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, let's get this in 🙏

? TurboModuleRegistry.get('PlatformLocalStorage') || // Support for external modules, like react-native-windows
TurboModuleRegistry.get('RNC_AsyncSQLiteDBStorage') ||
TurboModuleRegistry.get('RNCAsyncStorage')
: NativeModules['PlatformLocalStorage'] || // Support for external modules, like react-native-windows
NativeModules['RNC_AsyncSQLiteDBStorage'] ||
NativeModules['RNCAsyncStorage'];

if (!RCTAsyncStorage && shouldFallbackToLegacyNativeModule()) {
// TurboModuleRegistry falls back to NativeModules so we don't have to try go
// assign NativeModules' counterparts if TurboModuleRegistry would resolve
// with undefined.
if (TurboModuleRegistry) {
RCTAsyncStorage =
TurboModuleRegistry.get('AsyncSQLiteDBStorage') ||
Expand Down