From 24337196dcf34c5f531834d9c843d1c717f30603 Mon Sep 17 00:00:00 2001 From: Stepan Kartashev Date: Sat, 24 Aug 2024 21:59:48 +0500 Subject: [PATCH] fix: save methods of children Date instance (#437) (#480) (potentially BREAKING) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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: Степан Карташев Co-authored-by: Carl-Erik Kopseng --- .../fake-clock-integration-test.js | 4 +- src/fake-timers-src.js | 141 ++++++------------ test/fake-timers-test.js | 13 +- test/issue-437-test.js | 29 ++++ 4 files changed, 85 insertions(+), 102 deletions(-) create mode 100644 test/issue-437-test.js diff --git a/integration-test/fake-clock-integration-test.js b/integration-test/fake-clock-integration-test.js index f198da3..cebfeea 100644 --- a/integration-test/fake-clock-integration-test.js +++ b/integration-test/fake-clock-integration-test.js @@ -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(); diff --git a/src/fake-timers-src.js b/src/fake-timers-src.js index 825454d..3119507 100644 --- a/src/fake-timers-src.js +++ b/src/fake-timers-src.js @@ -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 + */ + // 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} */ - 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; } /** @@ -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") { diff --git a/test/fake-timers-test.js b/test/fake-timers-test.js index be9e9b9..42ea8fb 100644 --- a/test/fake-timers-test.js +++ b/test/fake-timers-test.js @@ -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); @@ -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 () { @@ -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 () { @@ -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); }); }); diff --git a/test/issue-437-test.js b/test/issue-437-test.js new file mode 100644 index 0000000..43d9d4c --- /dev/null +++ b/test/issue-437-test.js @@ -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(); + }); +});