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

Simpler and safer handling of non-valid jsons #28

Merged
merged 2 commits into from
Feb 25, 2016

Conversation

stropitek
Copy link
Contributor

No description provided.

@stropitek
Copy link
Contributor Author

See #27

@tsironis
Copy link
Owner

This does not work as expected because the localStorage value can be a serialized JSON. I think we need to encode the values before persisting to localStorage, then decode, then parse as the json string.

@stropitek
Copy link
Contributor Author

I don't really understand. It seems you are implying that JSON.parse could fail on a serialized JSON, since the part I change is only in this case. What do you mean then by serialized JSON?

@tsironis
Copy link
Owner

@stropitek sorry, my bad! Yes, it seems that this will work just fine. Can you add a test case for this please?

@stropitek
Copy link
Contributor Author

Yes, I will do that

@tsironis
Copy link
Owner

Awesome!

@stropitek
Copy link
Contributor Author

Done. For info your test script does not work with node v5. I had to use v4. Probably your dev dependencies are outdated. It seems to be the grunt-contrib-jasmine package that causes the issue.

tsironis added a commit that referenced this pull request Feb 25, 2016
Simpler and safer handling of non-valid jsons
@tsironis tsironis merged commit 791a95b into tsironis:master Feb 25, 2016
@tsironis
Copy link
Owner

Absolutely wonderful. Also, I bumped the devDependencies and now grunt runs as expected (node v5.3.0).

Thank you for your contributions!

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.

2 participants