-
-
Notifications
You must be signed in to change notification settings - Fork 813
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
Date problems after upgrading to node-sqlite3 5.0.0 #1355
Comments
Thank you for your comprehensive analysis. This is an unusual case, but one that should probably be supported. I cannot see a better alternative to the one you're proposing. The tool that builds There's more detailed information here: https://github.com/mapbox/node-pre-gyp#n-api-considerations The In the meantime, I can bring this question up for review by the N-API team at its meeting next week to see if there's a solution that does not require using the N-API |
This issue has been under discussion by the N-API team. The underlying cause is apparently not related to N-API specifically, but Node native addons in general so requires more in-depth analysis. In order to address this issue, I would be happy to review a PR that supports both N-API v3 and v5. It would then be up to the |
Happy to accept a PR for this 🙂 |
I know this is an old issue but we are now running into it. Is the only real workaround downgrading to v4.2.0? |
After upgrading from node-sqlite3 4.2.0 to 5.0.0, my Jest test suite started failing because JavaScript
Date
objects were inserted into the database as strings (e.g., "Sat Jul 11 2020 17:06:27 GMT-0400 (Eastern Daylight Time)") instead of (milli)seconds-since-the-epoch.From digging into it, I believe that the problem is caused by Jest's use of Node VM contexts; anything that runs in a separate context has its own view of the Date constructor, causing statement.cc's "instanceof Date" logic to fail.
The simplest fix that I could find is for node-sqlite3 to use the N-API IsDate method; that puts its logic closer to 4.2's approach. The "is date" method requires N-API version 5; I'm not sure if that's a problem. (node-sqlite3 says it works in Node 11.x and 12.x; the compatibility matrix says that version 5 requires Node 12.11.x or newer.)
I assume that regexes are affected by the same problem but did not verify; I couldn't find an equivalent fix for them.
If you'd like for me to investigate further or open a PR, I'd be happy to do so.
Here's some sample code that demonstrates the problem; the two lines of output should both show numbers, but with node-sqlite3 5.0.0, the second line shows date text.
The text was updated successfully, but these errors were encountered: