Skip to content
This repository was archived by the owner on Jan 31, 2024. It is now read-only.

Store properties injected by the native app #31

Merged
merged 9 commits into from
Jun 20, 2023
Merged

Conversation

Ecarrion
Copy link
Contributor

closes #28

Why

This PR locally stores the BlogIf and token properties that are injected by the native app. This in order to avoid having to pass these properties as parameters through all the react app functions.

How

  • Adds AsyncStorage react dependency.

  • Adds storeDependency() readDependency() functions to store the properties injected from the native app.

  • Update index.tsx to store the injected properties in the local storage.

  • Update the jetpackFetch() function to read the blogId and token properties from the local storage.

Testing steps

  • Run the app and make sure the shipping list loads correctly (like before).

@Ecarrion Ecarrion requested a review from wzieba June 19, 2023 04:16
@Ecarrion Ecarrion changed the title Issue/28 store tokens Store properties injected by the native app Jun 19, 2023
yarn.lock Outdated
@@ -4,27 +4,27 @@

"@ampproject/remapping@^2.2.0":
version "2.2.1"
resolved "https://registry.yarnpkg.com/@ampproject/remapping/-/remapping-2.2.1.tgz#99e8e11851128b8702cd57c33684f1d0f260b630"
resolved "https://registry.npmjs.org/@ampproject/remapping/-/remapping-2.2.1.tgz"
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm uncertain if it was an intended change - I think we should decide on using npm or yarn. I think we should keep with yarn to keep the current setup and consistency.

So, what I've noticed, is that npm would change yarn.lock to its coordinates if used. I'd suggest reverting changes from yarn.lock and use yarn add @react-native-async-storage/async-storage --save

Copy link
Contributor

Choose a reason for hiding this comment

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

I allowed myself to address this comment here: 99e0359

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for this @wzieba. My bad, I did npm install ... and didn't think much about it. I'll use yarn from now on.

@wzieba
Copy link
Contributor

wzieba commented Jun 19, 2023

@Ecarrion which app have you been using to test this change? iOS or Android? I have some difficulties with the Android build and I don't know how to link React Native to the WooCommerce-iOS app.

@wzieba
Copy link
Contributor

wzieba commented Jun 19, 2023

I think that @react-native-async-storage/async-storage dependency suffers from the same issues as @react-navigation/native does in Android.

#27 tries to address it, but for now, the Android app won't build.

Copy link
Contributor

@wzieba wzieba left a comment

Choose a reason for hiding this comment

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

The code looks good! 👍

After merging #27 and adding changes in woocommerce/woocommerce-android#9225 , I was able to successfully run the app and storing credentials works well.

Under the hood, when it comes to Android, values are stored in SQLite database:

image

I wonder why the authors of async-storage decide to do this, instead of SharedPreferences.

@wzieba wzieba enabled auto-merge June 19, 2023 14:07
@wzieba wzieba merged commit 85b8c8d into trunk Jun 20, 2023
@wzieba wzieba deleted the issue/28-store-tokens branch June 20, 2023 16:24
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Store Tokens and Keys
2 participants