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

Add support for enabled TurboModules in Expo #418

Merged
merged 1 commit into from
Aug 16, 2020

Conversation

sjchmiela
Copy link
Contributor

Summary:

At Expo we're on the verge of enabling TurboModules in SDK 39 (expo/expo#9687). While we haven't added the native code for this module to the SDK yet, we want to maintain the .expo.js fallback, so it resolves with the React Native's AsyncStorage.

Test Plan:

I have verified manually that copying and pasting this code to an appropriate file in node_modules for a running Expo project fixes the [@RNC/AsyncStorage]: NativeModule: AsyncStorage is null. error.

The if (TurboModuleRegistry) helps to not break this code on older versions of RN which do not have TMR (I hope).

@krizzu krizzu merged commit ae2c675 into react-native-async-storage:master Aug 16, 2020
@sjchmiela sjchmiela deleted the patch-1 branch August 17, 2020 08:21
@sjchmiela
Copy link
Contributor Author

Thank you for merging it and verifying that the fix is correct!

I'm sorry to ask you more of your time, but would you care to publish a new version of async-storage with this patch included? At Expo we're preparing for release process and we want to both enable TurboModules and include async-storage as a supported library and for that we need to have, well, a release to users point to. 🙂 🙏

@krizzu
Copy link
Member

krizzu commented Aug 18, 2020

v1.12.0 is out :)

sjchmiela added a commit to expo/expo that referenced this pull request Aug 18, 2020
# Why

We need to enable TurboModules to be able to include Reanimated v2.

> A known issue with this PR is that the community AsyncStorage seizes to work — it expects only native module, isn't compatible with TurboModule AsyncStorage and we do not import the AsyncStorage community module properly.
>
> **Actually**, react-native-async-storage/async-storage#418 should be enough.

# How

Apart from the more or less usual TurboModules setup (imports, headers, flags, C++ dialect) I had to move native modules that we override in Expo from `extraModulesForBridge:` to `getModuleInstanceFromClass:`.

I don't like that we have to use `RCTLogFunction` in `EXVersionManager` now (otherwise the code wouldn't compile), but I guess that if the type ever changes somebody will notice and backport the change to versioned SDKs.

# Test Plan

Expo Client compiled, home ran.
@jfrolich
Copy link

would it make sense to not put this under an expo switch and enable this for non-expo (or ejected) expo apps as well?

djhr pushed a commit to smartHelios/async-storage that referenced this pull request Oct 5, 2020
prakashbask pushed a commit to prakashbask/expo that referenced this pull request Mar 16, 2022
# Why

We need to enable TurboModules to be able to include Reanimated v2.

> A known issue with this PR is that the community AsyncStorage seizes to work — it expects only native module, isn't compatible with TurboModule AsyncStorage and we do not import the AsyncStorage community module properly.
>
> **Actually**, react-native-async-storage/async-storage#418 should be enough.

# How

Apart from the more or less usual TurboModules setup (imports, headers, flags, C++ dialect) I had to move native modules that we override in Expo from `extraModulesForBridge:` to `getModuleInstanceFromClass:`.

I don't like that we have to use `RCTLogFunction` in `EXVersionManager` now (otherwise the code wouldn't compile), but I guess that if the type ever changes somebody will notice and backport the change to versioned SDKs.

# Test Plan

Expo Client compiled, home ran.
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.

4 participants