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

Swap the native module used when in Expo managed workflow #368

Conversation

brentvatne
Copy link
Contributor

@brentvatne brentvatne commented May 22, 2020

Summary:

Currently tools like AWS Amplify and redux-persist need separate instructions for using AsyncStorage whether you are in an Expo managed project or a bare React Native project, because Expo manage projects do not yet include @react-native-community/async-storage. We would like to include it, but it's a bit trickier than most packages due to complexity around scoping storage access when switching between different projects within the Expo client app.

In order to let library authors just tell people to use @react-native-community/async-storage, I think the easiest solution for now is to swap out the backing native module when in the Expo managed workflow to the solution provided by React Native, which is still included in core as recently as 0.62.2. I did this by moving out the logic for picking which native module to use into RCTAsyncStorage.js and RCTAsyncStorage.expo.js. The .expo.js extension is only used in the Expo managed workflow (more info if you're curious).

As far as I can tell, the API interface is the same and this should not cause any issues. When we update to 0.63 we will likely investigate what it will take to fully integrate this library.

The changes here have no impact on any environment other than Expo managed projects.

Test Plan:

To make this very easy to test I published an alpha version of the package: @react-native-community/async-storage@1.11.0-alpha.0.

You can test that this works in Expo managed apps by running the following:

  • npx crna --template blank
  • Install @react-native-community/async-storage@1.11.0-alpha.0
  • Copy in the "useAsyncStorage Specific Example" into App.js
  • yarn ios and/or yarn android
  • Import it in the app and use it

And in a bare React Native app:

  • npx react-native init
  • Install @react-native-community/async-storage@1.11.0-alpha.0
  • npx pod-install
  • Copy in the "useAsyncStorage Specific Example" into App.js
  • yarn ios and/or yarn android
  • Import it in the app and use it

@brentvatne brentvatne requested a review from krizzu May 22, 2020 22:58
@Ashish-Nanda
Copy link

Ashish-Nanda commented May 23, 2020

Merging this PR would be of great benefit to @aws-amplify and our customers, as well as other React Native libraries that support Expo managed apps

Details:
This is a major issue for libraries like @aws-amplify as @brentvatne stated above, where we try to support Expo managed apps as well as bare apps created by the RNC CLI. Currently we have held off on migrating to the community package for this reason, but it is one of the most frequent requests from customers on our repo. You can see a recent comment where I've addresed it on a customer PR here: aws-amplify/amplify-js#4027 (comment)

It is also a major pain point when an app relies on libraries where some use AsyncStorage from Core and others rely on @react-native-community/async-storage because it leads to a race condition #118 (comment). This issue was opened, incidentally by an Amplify customer and responded to by @krizzu who root caused it.

Recommendation:
I think this PR is probably the fastest path forward in enabling Expo managed apps to support @react-native-community/async-storage and this would be a great way to unblock customers and library developers while not impacting the existing functionality in any other kind of React Native apps.

Copy link
Member

@krizzu krizzu left a comment

Choose a reason for hiding this comment

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

Looks like a way to go here. I'm glad we'll get this sorted. Just reverse version change and we'll be good to go

package.json Outdated Show resolved Hide resolved
This reverts commit d343f79.
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

3 participants