Change instanceof(Date) to util.types.isDate(Date)

[Srayman]

Fixes #2860. isDate handles additional cases where a Date object may not be an instance of Date. This impacts whether a date is correctly formatted with the correct timezone for the database.

[brianc]

Thanks Srayman - is there a way you could include a unit/integration test for this? I often make tests for particular github issues here: https://github.com/brianc/node-postgres/tree/master/packages/pg/test/integration/gh-issues

[Srayman]

I'll have to think about how to setup the Date object to properly test it. We're using a custom Rust library currently in our project and where this problem originated from. I'm looking to see if there's an easy way to recreate the issue natively in JS/TS. If you have any thoughts I'm open to suggestions on how to create a Date that is not an instance of Date.

[charmander]

vm.runInNewContext('new Date()')

should do it, IIRC

[Srayman]

vm.runInNewContext('new Date()')

should do it, IIRC

Awesome, did not know about vm! That seems to do the trick locally for the 2862 test. Pushed the test and also updated the util function to use both methods to check for a date in case there are any edge cases the other direction. Let me know what you think.

[Srayman]

Any feedback on this @brianc ?

[charmander]

The package change looks good to me!

The test could be improved, but so could a lot of other tests. I think it’s fair to take care of it along with those :)

[charmander]

Oh, I forgot pg still declares support for Node 8 (#2865). I don’t think util.types is compatible, unless it was backported without updating the docs. It’s not in CI…

[Srayman]

@charmander @brianc It's been a while! Just curious what's the latest on Node support and this change? We're running pg@8.13.1 and still running into this bug with dates.