search

scotttrinh / angular-localForage

Angular service & directive for https://github.com/mozilla/localForage (Offline storage, improved.)
MIT License
651 stars 86 forks source link

fix: added support for getting/setting a Date object #139

Closed Sadeka closed 7 years ago

Sadeka commented 7 years ago

Hello,

I have updated the code to add support for getting/setting a Date object (works for IndexedDB). I have also added a unit test for for it. Could you please review my changes?

Thanks & Regards, Sadeka

scotttrinh commented 7 years ago

Actually, @Sadeka , I think there is a better way to accomplish this. Instead of special-casing all non Primitive Objects, we should just change the check to see if the .constructor is Object. Keep all of the tests you wrote, but would you remove the special case for Date and change L#150 to:

} else if (angular.isObject(value) && value.constructor === Object) {

That will fail for Blobs and Dates and moments and anything else that uses a special constructor.

Let me know if that makes sense or if you see a logical flaw in that!

Sadeka commented 7 years ago

Thanks @scotttrinh for your valuable feedback. I have updated the code accordingly. Submitting a new pull request with a fresh single commit.