Skip to content

Commit

Permalink
fix: save methods of children Date instance (#437) (#480) (potentiall…
Browse files Browse the repository at this point in the history
…y BREAKING)

See #437 for background. All tests run fine and I have no reason to expect this to do anything but improve things, but it is changing something that has been working for a long time. 

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

* rename

see pr comment

* fix skipped tests

by ensuring we are checking something meaningful.

* refactor: trim out code that will never be hit in the Proxy

---------

Co-authored-by: Степан Карташев <kartashev@byndyusoft.com>
Co-authored-by: Carl-Erik Kopseng <carlerik@gmail.com>
  • Loading branch information
3 people authored Aug 24, 2024
1 parent c136830 commit 2433719
Show file tree
Hide file tree
Showing 4 changed files with 85 additions and 102 deletions.
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
141 changes: 49 additions & 92 deletions src/fake-timers-src.js
Original file line number Diff line number Diff line change
Expand Up @@ -398,109 +398,67 @@ function withGlobal(_global) {
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 {
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();
};
} 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() {
// noinspection UnnecessaryLocalVariableJS
/**
* @param {number} year
* @param {number} month
* @param {number} date
* @param {number} hour
* @param {number} minute
* @param {number} second
* @param {number} ms
* @returns {Date}
* A normal Class constructor cannot be called without `new`, but Date can, so we need
* to wrap it in a Proxy in order to ensure this functionality of Date is kept intact
* @type {ClockDate}

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

View workflow job for this annotation

GitHub Actions / lint

The type 'ClockDate' is undefined
*/
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();
}

// 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,
const ClockDateProxy = new Proxy(ClockDate, {
// handler for [[Call]] invocations (i.e. not using `new`)
apply() {
// 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) {
throw new TypeError(
"A Proxy should only capture `new` calls with the `construct` handler. This is not supposed to be possible, so check the logic.",
);
}
}
}

return mirrorDateProperties(ClockDate, NativeDate);
return new NativeDate(ClockDate.clock.now).toString();
},
});

return ClockDateProxy;
}

/**
Expand Down Expand Up @@ -995,8 +953,7 @@ function withGlobal(_global) {
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());
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();
});
});

0 comments on commit 2433719

Please sign in to comment.