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

ClockTime constructor name is different, breaking mongoose Date checks #510

Closed
alexanderweiss opened this issue Oct 17, 2024 · 4 comments · Fixed by #511
Closed

ClockTime constructor name is different, breaking mongoose Date checks #510

alexanderweiss opened this issue Oct 17, 2024 · 4 comments · Fixed by #511

Comments

@alexanderweiss
Copy link
Contributor

We understand you have a problem and are in a hurry, but please provide us with some info to make it much more likely for your issue to be understood, worked on and resolved quickly.

  • FakeTimers version : 13.0.2
  • Environment : node 20.18 (macOS)
  • Other libraries you are using: sinon 19.0.2, mongoose 8.7.1

What did you expect to happen?
We're using Sinon together with mongoose. When using mongoose's toJSON and toObject to turn mongoose documents into plain objects while faking timers I'd expect fields with Date objects to remain Date objects (or at least look like them). This is the case in fake-timers 11.2.2.

What actually happens
While faking timers, when calling toJSON or toObject on a mongoose document, it turns any Date fields into a number (unix timestamp). This seems to happen because mongoose determines whether an object is a Date or not using the constructor name.

How to reproduce

import mongoose from 'mongoose';
import sinon from 'sinon';

const DocModel = mongoose.model('doc',  new mongoose.Schema({ someDate: { type: Date } }));

const doc = new DocModel({ someDate: new Date() })

const plainDocBefore = doc.toObject()
console.log(plainDocBefore.someDate) // plainDocBefore.someDate is a Date object

sinon.useFakeTimers({ now: new Date(), toFake: ['Date', 'hrtime'] });

const plainDocAfter = doc.toObject()
console.log(plainDocAfter.someDate) // plainDocAfter.someDate is a unix timestamp (number)

Hope this is the right place to report this.

@fatso83
Copy link
Contributor

fatso83 commented Oct 17, 2024

Hi, great report. We are trying to do semantic releases, which means that when going from version 11 to 12, you will know there are breaking changes. So there are at least two breaking changes between version 11 and 13, of which this is the most important change in version 12.

So currently it works as intended, but that does not mean we cannot do stuff to make life easier for people, if it is not a big deal. We did this in #505, for instance, to make instanceof Date checks pass.

I do not immediately see how we can do anything about Mongoose, if it essentially checks function references are the same. I do not think they are doing the right thing here anyway: checking if the instance is a Date has language support. Checking constructor references is really saying you only trust this specific Date constructor, which normally should not be any of your concern.

After checking out your link to the Mongoose code, I see that they are not comparing function references, but the names of those functions (constructors):

> class NewDate extends Date {}
undefined
> d1 = new NewDate
NewDate 2024-10-17T16:51:28.011Z
> d.constructor
[Function: Date]
> d.constructor.name
'Date'
> d1.constructor.name
'NewDate'
> d1.constructor.name = 'Date'
'Date'

This is something we can do something about. Do you want to supply a fix? Usually the quickest way to get it done, if you need it.

@fatso83
Copy link
Contributor

fatso83 commented Oct 17, 2024

Fix supplied. Should fix your case.

@fatso83
Copy link
Contributor

fatso83 commented Oct 17, 2024

npm notice 📦  @sinonjs/[email protected]
npm notice Tarball Contents
npm notice 1.5kB LICENSE
npm notice 18.8kB README.md
npm notice 2.5kB package.json
npm notice 74.3kB src/fake-timers-src.js
npm notice Tarball Details
npm notice name: @sinonjs/fake-timers
npm notice version: 13.0.3
npm notice filename: sinonjs-fake-timers-13.0.3.tgz
npm notice package size: 21.1 kB
npm notice unpacked size: 97.2 kB
npm notice shasum: 3338263cbdfdb5cd40be61c3c315f360bf4f3a3b
npm notice integrity: sha512-golm/Sc4CqLV/[...]Kdu8OE05Ztiiw==
npm notice total files: 4
npm notice
npm notice Publishing to https://registry.npmjs.com/ with tag latest and default access
+ @sinonjs/[email protected]
v13.0.3

@alexanderweiss
Copy link
Contributor Author

Thanks so much! That was super quick. It does indeed fix the issue partially, but in some situations it's sadly still not working. I'll dig deeper and see about creating a PR if it's fixable.

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

Successfully merging a pull request may close this issue.

2 participants