Skip to content
This repository has been archived by the owner on Apr 17, 2023. It is now read-only.

Parsing local storage data in React Native #28

Closed
evelant opened this issue Apr 22, 2019 · 7 comments
Closed

Parsing local storage data in React Native #28

evelant opened this issue Apr 22, 2019 · 7 comments

Comments

@evelant
Copy link

evelant commented Apr 22, 2019

As the title says I upgraded to 0.5.0 and it crashes here

[Unhandled promise rejection: SyntaxError: JSON Parse error: Unexpected identifier "object"]
* [native code]:null in parse
- node_modules/offix-client/dist/offline/OfflineStore.js:79:45 in <unknown>
- node_modules/offix-client/dist/offline/OfflineStore.js:32:27 in step
- node_modules/offix-client/dist/offline/OfflineStore.js:7:13 in <unknown>

It looks like the getOfflineData function doesn't properly handle invalid data from the store. I'm using 0.5.0 on react-native (expo) with AsyncStorage.

Edit: It actually crashes using AsyncStorage no matter what now. It isn't just with old data. I cleared the data on the simulator and the result is the same. This worked in the previous version without problems.

@wtrocki
Copy link
Contributor

wtrocki commented Apr 23, 2019

We have integration test that tests default storage, but we do not test async store/react native case. I will try to test that early in the morning and see what is actually saved to store. Please use previos version for now.

@wtrocki
Copy link
Contributor

wtrocki commented Apr 23, 2019

@AndrewMorsillo I will need your help with this.
I haven't changed anything in the codebase apart from package renames so the code will be 100% the same. React Native support is still in progress in #24
I have tested that using window.localStorage as store and it works ok because returned data is a string.

I will need the following details in order to assist here:

  1. Dropping your client configuration into this PR React native package #24
    Configuration was mentioned in React Native support #23 but we will need the package to test it.

  2. If you have some basic app do you mind dropping that into: https://github.com/graphql-heroes/offix-react-native-example

You should have full access rights for both repositories.

@wtrocki wtrocki changed the title 0.5.0 crashes trying to parse local storage data from previous version Parsing local storage data in React Native Apr 23, 2019
@wtrocki
Copy link
Contributor

wtrocki commented Apr 23, 2019

@AndrewMorsillo I think that we need to simply check if object needs to be parsed or it is actual js object. window.localStorage returns string and current implementation support is. Not sure what AsyncStore we have been using for this. Once we get some changes into the React Native we will unify interface.

I think we need to have a wrapper for window.localStorage and our interface will always return Promise<OfflineEntry[]> and we will no longer perform JSON.parse/stringify on the store. We cannot use official AsyncStore for React Native as it is deprecated.

@evelant
Copy link
Author

evelant commented Apr 23, 2019

Thanks for making that repo. I'm working on an example project hopefully to be ready in the next few days. I'll be sure to push it there.

I'll be investigating the AsyncStorage thing today. Got delayed yesterday because I didn't realize that react native bundler (metro) doesn't work with symlinks (npm link) so I had a hard time setting up a local copy of offix-client.

I'm pretty new to graphql, react-native, expo, and developing npm libraries so it may take me a bit. A lot to learn!

@wtrocki
Copy link
Contributor

wtrocki commented Jun 12, 2019

We have done a couple of improvements to abstract storage interface inside offix and hoping to release new version over the comming days. So most of the problems should dissapear and things should work out of the box.

@evelant
Copy link
Author

evelant commented Jun 12, 2019

Unfortunately my team has decided not to use apollo/graphql for our latest project so I won't have time to contribute here for some time. I think this issue can be closed since it sounds like the storage abstraction should render it a non-issue.

@evelant evelant closed this as completed Jun 12, 2019
@wtrocki
Copy link
Contributor

wtrocki commented Jun 12, 2019

@AndrewMorsillo Thank you so much for the update!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants