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

Invalid Date object will throw a TypeError in Vitest #2216

Closed
6 tasks done
dotnetCarpenter opened this issue Oct 27, 2022 · 5 comments
Closed
6 tasks done

Invalid Date object will throw a TypeError in Vitest #2216

dotnetCarpenter opened this issue Oct 27, 2022 · 5 comments
Labels

Comments

@dotnetCarpenter
Copy link
Contributor

Describe the bug

The following two assertions should both succeed. But due to Vitest usage of .toJSON() on Date objects, the second line will fail with a TypeError.

expect(NaN).toBe(NaN);
expect(new Date('not a date')).toBe(NaN);

The reason is that .toJSON() returns undefined on invalid Date objects.

new Date('1990-01-01').toJSON();	// <- "1990-01-01T00:00:00.000Z"
new Date('foo bar').toJSON();		// <- null

On node 18.12 I get the following error:

TypeError: Cannot read properties of null (reading 'split')

On StackBlitz, I get:

TypeError: can't access property "split", dateObject.toJSON() is null

Reproduction

const { it, expect } = import.meta.vitest

it('Invalid Date is NaN', () => {
	expect (new Date ('not a date')).toBe (NaN)
})

System Info

System:
    OS: Linux 5.10 Ubuntu 22.04.1 LTS 22.04.1 LTS (Jammy Jellyfish)
    CPU: (4) x64 Intel(R) Core(TM) i7-4510U CPU @ 2.00GHz
    Memory: 2.22 GB / 3.83 GB
    Container: Yes
    Shell: 5.1.16 - /bin/bash
  Binaries:
    Node: 18.12.0 - /usr/local/bin/node
    Yarn: 1.22.18 - ~/.yarn/bin/yarn
    npm: 8.19.2 - /usr/local/bin/npm
  npmPackages:
    vite: ^3.2.0 => 3.2.0 
    vitest: ^0.24.3 => 0.24.3

Used Package Manager

npm

Validations

@sheremet-va
Copy link
Member

I think this is chai problem. The error comes from loupe package, according to stack trace. And is used by chai.

@dotnetCarpenter
Copy link
Contributor Author

@sheremet-va I'm looking at package.json but I can not see Chai as a dependency. I do not know pnpm but I do see Chai in pnpm-lock.yaml. Sorry, this might not be important. I am only trying to understand when/why Chai is injected into Vitest, in order to better understand where a fix for invalid dates should exist. I do not think it is obvious that it should be fixed in Loupe.

Could you check out chaijs/loupe#58 and let us know what you think?

@sheremet-va
Copy link
Member

sheremet-va commented Oct 28, 2022

I'm looking at package.json but I can not see Chai as a dependency

Not sure how you missed it: https://github.com/vitest-dev/vitest/blob/main/packages/vitest/package.json#L111

Vitest uses chai's expect with jest-compatible API.

Could you check out chaijs/loupe#58 and let us know what you think?

Not sure what I can say 😄 Your issue is correct, I would expect it to be fixed on their side, and then we just upgrade.

@dotnetCarpenter
Copy link
Contributor Author

@sheremet-va This has been fixed in loupe 2.3.5 (npm).

After some lengthy discussions (chaijs/loupe#58 (comment), chaijs/loupe#59 (comment)), we settled on loupe returning "Invalid Date" for an invalid Date object. So the test code has changed slightly to:

const { it, expect } = import.meta.vitest

it('Invalid Date is "Invalid Date"', () => {
	expect (new Date ('not a date')).toBe ('Invalid Date')
})

@sheremet-va
Copy link
Member

sheremet-va commented Nov 8, 2022

So the test code has changed slightly to

loupe only affects error messages.

You should not get the internal error, when you call expect(new Date('nod')).toBe(NaN), anymore. But you will still get an error, because toBe is using Object.is for equality check.

Correct check would be:

expect (new Date ('not a date').valueOf()).toBe(NaN)

Anyway, this issue can be closed, since Vitest 0.25.0 updated chai version to the one that has the fix.

@github-actions github-actions bot locked and limited conversation to collaborators Jun 11, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

2 participants