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

Date detection doesn't work for Dates from other realms #441

Open
michaelwallabi opened this issue Nov 2, 2024 · 5 comments
Open

Date detection doesn't work for Dates from other realms #441

michaelwallabi opened this issue Nov 2, 2024 · 5 comments

Comments

@michaelwallabi
Copy link

Describe the bug

  1. Run a query that returns a date using duckdb in a node environment, say something like await db.all("SELECT DATE '2022-01-01' as dt")
  2. stringify the result

Expected:

1640995200000

Actual:

"""2022-01-01T00:00:00.000Z"""

To Reproduce

   import { Database } from 'duckdb-async';
 
    Database.create(":memory:")
    const t = await db.all("SELECT DATE '2022-01-01' as dt");

    // expect: 1640995200000
    console.warn(stringify(t));
    // actual: """2022-01-01T00:00:00.000Z"""

    // expect: 1640995200000
    console.warn(stringify(t, { cast: { date: (x) => x.getTime().toString() } }));
    // actual: """2022-01-01T00:00:00.000Z"""

    // expect: 1640995200000
    console.warn(stringify(t, { cast: { object: (x) => x.getTime().toString() } }));
    // actual: 1640995200000

Additional context

Some discussion in this stack overflow answer: https://stackoverflow.com/questions/643782/how-to-check-whether-an-object-is-a-date

Looks like the issue is that instanceof Date only works within the same frame/realm.

date-fns handles it this way.

@wdavidw
Copy link
Member

wdavidw commented Nov 7, 2024

I reproduce your setup in demo/issues-esm/lib/441.js. The result corresponds to the expectations unless there is something that I did not understand from your description.

@michaelwallabi
Copy link
Author

I reproduce your setup in demo/issues-esm/lib/441.js. The result corresponds to the expectations unless there is something that I did not understand from your description.

Interesting. I wonder if this is specific to running inside of Jest. That's where I found this issue in testing. Let me see if I can repro stand alone.

@michaelwallabi
Copy link
Author

Ok, confirmed. Looks to be an issue with running in jest. Below is what I tried and the associated output.

Test

/* eslint-disable mocha/no-mocha-arrows */
const { stringify } = require("csv-stringify/sync");
const { Database } = require("duckdb-async");
const assert = require("node:assert");

test("441", async () => {
  const db = await Database.create(":memory:");
  const t = await db.all("SELECT DATE '2022-01-01' as dt");

  // expect: 1640995200000
  console.warn(stringify(t));
  // actual: """2022-01-01T00:00:00.000Z"""

  // expect: 1640995200000
  console.warn(stringify(t, { cast: { date: (x) => x.getTime().toString() } }));
  // actual: """2022-01-01T00:00:00.000Z"""

  // expect: 1640995200000
  console.warn(stringify(t, { cast: { object: (x) => x.getTime().toString() } }));

  // Validate that duckdb returns as an instance of JS date
  assert(t[0].dt instanceof Date);
  // Validate that date objects are handled by `cast.date`
  assert.equal(stringify(t, { cast: { date: () => "ok" } }).trim(), "ok");

  // First assertion raised in the issue
  assert.equal(stringify(t), 1640995200000);
  // Second assertion raised in the issue
  assert.equal(
    stringify(t, { cast: { date: (x) => x.getTime().toString() } }).trim(),
    1640995200000,
  );
});

Output

% npx jest
  console.warn
    """2022-01-01T00:00:00.000Z"""
    

       9 |
      10 |   // expect: 1640995200000
    > 11 |   console.warn(stringify(t));
         |           ^
      12 |   // actual: """2022-01-01T00:00:00.000Z"""
      13 |
      14 |   // expect: 1640995200000

      at Object.warn (lib/441.test.js:11:11)

  console.warn
    """2022-01-01T00:00:00.000Z"""
    

      13 |
      14 |   // expect: 1640995200000
    > 15 |   console.warn(stringify(t, { cast: { date: (x) => x.getTime().toString() } }));
         |           ^
      16 |   // actual: """2022-01-01T00:00:00.000Z"""
      17 |
      18 |   // expect: 1640995200000

      at Object.warn (lib/441.test.js:15:11)

  console.warn
    1640995200000
    

      17 |
      18 |   // expect: 1640995200000
    > 19 |   console.warn(stringify(t, { cast: { object: (x) => x.getTime().toString() } }));
         |           ^
      20 |
      21 |   // Validate that duckdb returns as an instance of JS date
      22 |   assert(t[0].dt instanceof Date);

      at Object.warn (lib/441.test.js:19:11)

 FAIL  lib/441.test.js
  ✕ 441 (78 ms)

  ● 441

    assert(received)

    Expected value to be equal to:
      true
    Received:
      false

      20 |
      21 |   // Validate that duckdb returns as an instance of JS date
    > 22 |   assert(t[0].dt instanceof Date);
         |   ^
      23 |   // Validate that date objects are handled by `cast.date`
      24 |   assert.equal(stringify(t, { cast: { date: () => "ok" } }).trim(), "ok");
      25 |

      at Object.assert (lib/441.test.js:22:3)

Test Suites: 1 failed, 1 total
Tests:       1 failed, 1 total
Snapshots:   0 total
Time:        0.318 s, estimated 1 s

@wdavidw
Copy link
Member

wdavidw commented Nov 12, 2024

You could also remove duckdb from your test to simplify the code and make sure does not influence the behavior.

@michaelwallabi
Copy link
Author

Duckdb is required for the repro. If I update to the following everything passes:

  const t = [{ dt: new Date("2022-01-01") }];
  // expect: 1640995200000
  console.warn(stringify(t));
  // actual: """2022-01-01T00:00:00.000Z"""

  // expect: 1640995200000
  console.warn(stringify(t, { cast: { date: (x) => x.getTime().toString() } }));
...

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