JohnWeisz / TypedJSON

Typed JSON parsing and serializing for TypeScript that preserves type information.
MIT License
603 stars 64 forks source link

Fixed error when converting Date object (instead of datestring) to Date #128

Closed sumbricht closed 4 years ago

sumbricht commented 4 years ago

Hi John

I noticed an error when calling TypedJSON.parse with an object instead of JSON. In my use case, I already have a JS object and use TypedJSON to convert it to an instance of the right class, in a nested fashion. As my JS object contains Date properties, TypedJSON failed when trying to convert the corresponding properties to Date objects. It expected ISO-8601 strings and not Date objects. I'm therefore proposing an easy fix for this, by just returning the Date object as is (no conversion needed).

Regards, Simon

Neos3452 commented 4 years ago

Hi Simon, thanks a lot for this improvement, it helps a lot.

Would you mind making your PR a little bit better? I have an automated release script, which does everything in a repeatable fashion. Could you remove the package.json changes, and autogenerated js files? I also noticed that you used tabs while the project currently is using spaces for formatting. The last thing that would make it even great is if you could add a test case with a simplified class that you are serialising in your app (just-json.spec.ts seems like a fit for that). This would guarantee that your case is working in the future versions of TypedJSON.

sumbricht commented 4 years ago

Hi Michał, thanks a lot for your inputs. I have reverted the package.json / js changes and changed the formatting to spaces. Regarding testing, I have amended a Date property to the existing test case for class SomeThing; this amended test fails without the fix I implemented and now succeeds. Please let me know if you see any further need for adaption :-), otherwise thanks a lot for integrating into your extremely helpful library

Neos3452 commented 4 years ago

Hi, sorry for such a long delay, the version 1.5.2 has been release which includes your changes

sumbricht commented 4 years ago

Thanks a lot, @Neos3452 :-). I was very happy to contribute