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

restoreDates implementation is too loose #138

Open
imolorhe opened this issue Dec 28, 2019 · 1 comment
Open

restoreDates implementation is too loose #138

imolorhe opened this issue Dec 28, 2019 · 1 comment

Comments

@imolorhe
Copy link

imolorhe commented Dec 28, 2019

Hey there! I use this tool to manage the state of my application in local storage. Recently I have been receiving issues about weird behaviours in my app, and I couldn't really detect the reason for them, until someone was able to give me a step-by-step guide on how to reproduce it, with a sample data. In my app, the user can provide JSON data in a text editor interface, which I store in local storage as a string. Whenever the app is restarted, the JSON string is serialized to null. After digging through the code to check if I am indeed resetting the value to null, I realized the null value was actually an Invalid Date object.

Here's a sample string:

{
  "BIRTHDAY": "1999-12-01T06:23:22.886Z"
}

It turns out, that the dateReviver simply tests for a match in the provided string with a date regex and if found, it instantiates the entire string as a Date object. As you can see from the example string above, while there is indeed a date sub string matching a date regex, the string itself is not a date and so reviving it as a date is a wrong assumption. At the least, the regex should check that only the date string is found and it isn't part of a larger string. Setting restoreDates to false fixed the issue for me.

Also, I don't think enabling the dateReviver should be the default case for this. It seems to only be useful in specific cases. It's already known that once an object is serialized to JSON (to be stored), it loses all its dynamic properties, and so one would expect the date to be a string, and only if required, should the restoreDates be true.

@fredrikj
Copy link

fredrikj commented Apr 6, 2020

NGRX can not handle Date objects, so I am confused on why this library even does Date parsing. In what scenario do we want ISO date strings to be parsed into Date objects?

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

2 participants