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

fix: save methods of children Date instance (#437) #480

Merged
merged 4 commits into from
Aug 24, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions integration-test/fake-clock-integration-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -125,8 +125,8 @@ describe("globally configured browser objects", function () {
now: mockNow,
});

assert.equals(new Date(Date.now()), mockNow);
assert.equals(new Date(), mockNow);
assert.equals(new Date(Date.now()).toString(), mockNow.toString());
assert.equals(new Date().toString(), mockNow.toString());

clock.uninstall();

Expand Down
137 changes: 43 additions & 94 deletions src/fake-timers-src.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
/**
* Queues a function to be called during a browser's idle periods
* @callback RequestIdleCallback
* @param {function(IdleDeadline)} callback

Check warning on line 27 in src/fake-timers-src.js

View workflow job for this annotation

GitHub Actions / lint

Syntax error in type: function(IdleDeadline)
* @param {{timeout: number}} options - an options object
* @returns {number} the id
*/
Expand All @@ -51,13 +51,13 @@

/**
* @typedef RequestAnimationFrame
* @property {function(number):void} requestAnimationFrame

Check warning on line 54 in src/fake-timers-src.js

View workflow job for this annotation

GitHub Actions / lint

Missing JSDoc @Property "requestAnimationFrame" description
* @returns {number} - the id
*/

/**
* @typedef Performance
* @property {function(): number} now

Check warning on line 60 in src/fake-timers-src.js

View workflow job for this annotation

GitHub Actions / lint

Missing JSDoc @Property "now" description
*/

/* eslint-disable jsdoc/require-property-description */
Expand Down Expand Up @@ -329,7 +329,7 @@
return timer && timer.callAt >= from && timer.callAt <= to;
}

/**

Check warning on line 332 in src/fake-timers-src.js

View workflow job for this annotation

GitHub Actions / lint

Missing JSDoc @returns declaration
* @param {Clock} clock
* @param {Timer} job
*/
Expand Down Expand Up @@ -398,109 +398,59 @@
return infiniteLoopError;
}

/**
* @param {Date} target
* @param {Date} source
* @returns {Date} the target after modifications
*/
function mirrorDateProperties(target, source) {
let prop;
for (prop in source) {
if (source.hasOwnProperty(prop)) {
target[prop] = source[prop];
//eslint-disable-next-line jsdoc/require-jsdoc
function createDate() {
class ClockDate extends NativeDate {
/**
* @param {number} year
* @param {number} month
* @param {number} date
* @param {number} hour
* @param {number} minute
* @param {number} second
* @param {number} ms
* @returns void

Check warning on line 412 in src/fake-timers-src.js

View workflow job for this annotation

GitHub Actions / lint

Missing JSDoc @returns type
*/
// eslint-disable-next-line no-unused-vars
constructor(year, month, date, hour, minute, second, ms) {
// Defensive and verbose to avoid potential harm in passing
// explicit undefined when user does not pass argument
if (arguments.length === 0) {
super(ClockDate.clock.now);
} else {
fatso83 marked this conversation as resolved.
Show resolved Hide resolved
super(...arguments);
}
}
}

// set special now implementation
if (source.now) {
target.now = function now() {
return target.clock.now;
ClockDate.isFake = true;

if (NativeDate.now) {
ClockDate.now = function now() {
return ClockDate.clock.now;
};
} else {
delete target.now;
}

// set special toSource implementation
if (source.toSource) {
target.toSource = function toSource() {
return source.toSource();
if (NativeDate.toSource) {
ClockDate.toSource = function toSource() {
return NativeDate.toSource();

Check warning on line 436 in src/fake-timers-src.js

View check run for this annotation

Codecov / codecov/patch

src/fake-timers-src.js#L435-L436

Added lines #L435 - L436 were not covered by tests
};
} else {
delete target.toSource;
}

// set special toString implementation
target.toString = function toString() {
return source.toString();
};

target.prototype = source.prototype;
target.parse = source.parse;
target.UTC = source.UTC;
target.prototype.toUTCString = source.prototype.toUTCString;
target.isFake = true;

return target;
}

//eslint-disable-next-line jsdoc/require-jsdoc
function createDate() {
/**
* @param {number} year
* @param {number} month
* @param {number} date
* @param {number} hour
* @param {number} minute
* @param {number} second
* @param {number} ms
* @returns {Date}
*/
function ClockDate(year, month, date, hour, minute, second, ms) {
// the Date constructor called as a function, ref Ecma-262 Edition 5.1, section 15.9.2.
// This remains so in the 10th edition of 2019 as well.
if (!(this instanceof ClockDate)) {
return new NativeDate(ClockDate.clock.now).toString();
}
const ClockDateProxy = new Proxy(ClockDate, {
apply(Target, thisArg, argumentsList) {
// the Date constructor called as a function, ref Ecma-262 Edition 5.1, section 15.9.2.
// This remains so in the 10th edition of 2019 as well.
if (!(this instanceof ClockDate)) {
return new NativeDate(ClockDate.clock.now).toString();
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

had to do a little brush up on Proxy objects and its apply to grok the logic, which made me pay so much attention to the details that I failed to see the toString() bit. That was until I actually read section 15.9.2:

When Date is called as a function rather than as a constructor, it returns a String representing the current time (UTC).

Ooh. Did not remember this. Had to check:

❯ node -p 'Date(2023,01,10)'
Sat Feb 10 2024 11:54:24 GMT+0100 (GMT+01:00)

❯ node -p 'typeof Date(2023,01,10)'
string

I'd rather extract that into a silly, but aptly named, little method called currentDateTimeAsString() and stuff all the details there, though! Makes small minds as my own able to grok the bigger picture faster. Makes for quicker 🆗 on PRs 😅 I know the bit of code was already there, though, so no worries!


// if Date is called as a constructor with 'new' keyword
// Defensive and verbose to avoid potential harm in passing
// explicit undefined when user does not pass argument
switch (arguments.length) {
case 0:
return new NativeDate(ClockDate.clock.now);
case 1:
return new NativeDate(year);
case 2:
return new NativeDate(year, month);
case 3:
return new NativeDate(year, month, date);
case 4:
return new NativeDate(year, month, date, hour);
case 5:
return new NativeDate(year, month, date, hour, minute);
case 6:
return new NativeDate(
year,
month,
date,
hour,
minute,
second,
);
default:
return new NativeDate(
year,
month,
date,
hour,
minute,
second,
ms,
);
}
}
// if Date is called as a constructor with 'new' keyword
return new Target(...argumentsList);

Check warning on line 449 in src/fake-timers-src.js

View check run for this annotation

Codecov / codecov/patch

src/fake-timers-src.js#L449

Added line #L449 was not covered by tests
},
});

return mirrorDateProperties(ClockDate, NativeDate);
return ClockDateProxy;
}

/**
Expand Down Expand Up @@ -679,7 +629,7 @@
}

/* eslint consistent-return: "off" */
/**

Check warning on line 632 in src/fake-timers-src.js

View workflow job for this annotation

GitHub Actions / lint

JSDoc @returns declaration present but return expression not available in function
* Timer comparitor
* @param {Timer} a
* @param {Timer} b
Expand Down Expand Up @@ -810,7 +760,7 @@
}
}

/**

Check warning on line 763 in src/fake-timers-src.js

View workflow job for this annotation

GitHub Actions / lint

Missing JSDoc @returns declaration
* Gets clear handler name for a given timer type
* @param {string} ttype
*/
Expand All @@ -821,7 +771,7 @@
return `clear${ttype}`;
}

/**

Check warning on line 774 in src/fake-timers-src.js

View workflow job for this annotation

GitHub Actions / lint

Missing JSDoc @returns declaration
* Gets schedule handler name for a given timer type
* @param {string} ttype
*/
Expand All @@ -832,7 +782,7 @@
return `set${ttype}`;
}

/**

Check warning on line 785 in src/fake-timers-src.js

View workflow job for this annotation

GitHub Actions / lint

Missing JSDoc @returns declaration
* Creates an anonymous function to warn only once
*/
function createWarnOnce() {
Expand All @@ -844,7 +794,7 @@
}
const warnOnce = createWarnOnce();

/**

Check warning on line 797 in src/fake-timers-src.js

View workflow job for this annotation

GitHub Actions / lint

Missing JSDoc @returns declaration
* @param {Clock} clock
* @param {number} timerId
* @param {string} ttype
Expand Down Expand Up @@ -995,8 +945,7 @@
clock[`_${method}`] = target[method];

if (method === "Date") {
const date = mirrorDateProperties(clock[method], target[method]);
target[method] = date;
target[method] = clock[method];
} else if (method === "Intl") {
target[method] = clock[method];
} else if (method === "performance") {
Expand Down
13 changes: 5 additions & 8 deletions test/fake-timers-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -3155,7 +3155,7 @@ describe("FakeTimers", function () {
});
});

describe("date", function () {
describe("Date", function () {
beforeEach(function () {
this.now = new GlobalDate().getTime() - 3000;
this.clock = FakeTimers.createClock(this.now);
Expand Down Expand Up @@ -3189,10 +3189,7 @@ describe("FakeTimers", function () {

const date = new this.clock.Date();

assert.same(
date.constructor.prototype,
realDate.constructor.prototype,
);
assert(date instanceof realDate.constructor);
});

it("creates Date objects representing clock time", function () {
Expand Down Expand Up @@ -3300,8 +3297,8 @@ describe("FakeTimers", function () {
assert.equals(fakeDateStr, new this.clock.Date().toString());
});

it("mirrors native Date.prototype", function () {
assert.same(this.clock.Date.prototype, Date.prototype);
it("creates objects that are instances of Date", function () {
assert(new this.clock.Date() instanceof Date);
});

it("supports now method if present", function () {
Expand Down Expand Up @@ -3362,7 +3359,7 @@ describe("FakeTimers", function () {
});

it("mirrors toString", function () {
assert.same(this.clock.Date.toString(), Date.toString());
fatso83 marked this conversation as resolved.
Show resolved Hide resolved
assert.same(this.clock.Date.toString, Date.toString);
});
});

Expand Down
29 changes: 29 additions & 0 deletions test/issue-437-test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
"use strict";

const { FakeTimers, assert } = require("./helpers/setup-tests");

describe("issue #437", function () {
it("should save methods of subclass instance", function () {
const clock = FakeTimers.install();

class DateTime extends Date {
constructor() {
super();

this.bar = "bar";
}

foo() {
return "Lorem ipsum";
}
}

const dateTime = new DateTime();

// this would throw an error before issue #437 was fixed
assert.equals(dateTime.foo(), "Lorem ipsum");
assert.equals(dateTime.bar, "bar");

clock.uninstall();
});
});
Loading