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

Bug: Storage API always returns string on web #4

Open
ricoberger opened this issue Jan 6, 2020 · 4 comments
Open

Bug: Storage API always returns string on web #4

ricoberger opened this issue Jan 6, 2020 · 4 comments

Comments

@ricoberger
Copy link

ricoberger commented Jan 6, 2020

Hi, the following code always returns a string instead of an object after a page reload:

const [ myObj , setMyObj ] = useStorageItem<object>('myobj');

const set = () => {
  setMyObj({
    'key1': 'value1',
    'key2': 'value2',
  });
};

// After the set function is calledmyObj is of type object, but after a page reload myObj is of type string
console.log(myObj);

I think it's caused by the following line https://github.com/ionic-team/ionic-react-hooks/blob/76745d1b3a0b4c62949dd6095ede3cbb4843c9ab/src/storage/useStorage.ts#L87 because on the web the type of result.value is always string. Which is because of the implementation of the storage API for the web, where localStorage.getItem() always returns a string or null for the value key: https://github.com/ionic-team/capacitor/blob/f08e4a4f3cff1eedca3ca7292da7892ab2de5806/core/src/web/storage.ts#L21

I fixed it for me the following way:

-          setStoredValue(typeof result.value === 'string' ? result.value : JSON.parse(result.value!));
          
+          try {
+            const parsedValue = JSON.parse(result.value!);
+            setStoredValue(parsedValue);
+          } catch (err) {
+            setStoredValue(result.value);
+          }

If you want I can submit a PR with the change, but I think there should be a better solution than using try and catch.

@slevy85
Copy link

slevy85 commented May 18, 2020

Hi,

I made a fix for this in the the pull request #8

@jakobhellermann
Copy link

hey, I stumbled upon this bug today. Is there a path forward for merging the PR? Thanks for your time on this project.

@leandrogehlen
Copy link
Contributor

@elylucas Is there any estimate to approve the PR #8 ?

@ricoberger ricoberger mentioned this issue Sep 23, 2020
33 tasks
@gugahoi
Copy link

gugahoi commented Jun 1, 2021

It's been over a year since a PR was proposed to address this but not a peep from maintainers. Also close to a year since any material changes were made to this repo, can we assume this is unmaintained?

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

No branches or pull requests

6 participants