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

GetItemAsync<string> fails if value doesn't contain quotes #57

Closed
dominicusmento opened this issue Mar 19, 2020 · 3 comments · Fixed by #59
Closed

GetItemAsync<string> fails if value doesn't contain quotes #57

dominicusmento opened this issue Mar 19, 2020 · 3 comments · Fixed by #59
Labels
Bug Something isn't working

Comments

@dominicusmento
Copy link

Describe the bug
localStorage.GetItemAsync<**string**>("test") fails if localStorage record looks like this:
test|demoValue

If it is test|"demoValue" then everything works fine.

To Reproduce
Steps to reproduce the behavior:
Just add a new localStorage value manually in browser and try to read it with library. Try changing the value to be with and without quotes.

Expected behavior
Read the value without quotes as string as it's not expected to read our own localStorage values. Sometimes other library writes the values and we should read them.

Hosting Model (is this issue happening with a certain hosting model?):

  • Blazor WebAssembly

Additional context
Using latest library package, 2.1.1

@dominicusmento dominicusmento added Bug Something isn't working Triage Issue needs to be triaged labels Mar 19, 2020
@chrissainty chrissainty removed the Triage Issue needs to be triaged label Mar 22, 2020
@chrissainty
Copy link
Member

chrissainty commented Mar 22, 2020

Thanks for reporting this @tomidix. I can see what the problem is, all values are serialised to a string before being saved to local storage, even strings 🤦‍♂️.

In your example, when trying to read values out the library tries to deserialise them as well, again even if they're strings 🤦‍♂️, which is why you're seeing the problem, if the string hasn't been serialised on the way in it thrown when trying to deserialise on the way out. Does that make sense?

I'll get a fix coded up for this.

Note to self: Need to be careful as this will break some peoples data. Need to have some kind of migration plan.

@dominicusmento
Copy link
Author

dominicusmento commented Mar 23, 2020

I didn't look at the code of the library but the fix seems to be obvious to me and I think it should be backward compatible.
Before any deserialization library needs to check if the value startsWith && endsWith " or {; if it does then proceed as usual, if it doesn't just take the value as a string.
As for writing the values, possibly you can add a parameter to force the string serialization even if it's useless in my opinion.

@chrissainty
Copy link
Member

I agree, @tomidix and that's pretty much exactly what I've done. I've fixed the set code so that a string will no longer be serialised, it will just be persisted as-is.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants