Repository: sinonjs/fake-timers
Pull request #480 (2023-09-05 10:53:40)

fix: save methods of children Date instance (#437)
Fix issue #437 by creating ClockDate as JS class and extending it from NativeDate

Why I did this way: 
* In initinial solution (creating `ClockDate` by JS function) `ClockDate` can't be correct instance to extend from it. That's why I replaced function on class. And it resolved my problem.
* Mechanics of call `Date()` without `new` keyword I solved by using `Proxy` instance.
* Also in my solution removed an `mirrorDateProperties` function. It's not needed any more.

But I had some troubles with tests:
* I skiped [this test](https://github.com/sinonjs/fake-timers/blob/main/test/fake-timers-test.js#L3106-3117), because I didn't understand what it checks and how it should be now
* I skiped [an other test](https://github.com/sinonjs/fake-timers/blob/main/test/fake-timers-test.js#L3224-3226), because I think it's not corresponding for now. `ClockDate.prototype` never equals to `Date.prototype`, because `ClockDate` is extended from `Date`
* Please, check the other tests, that I changed. Hope, I changed them correct


fatso83: Oooh, I somehow missed that this was added. Sorry about the late look, but while this is interesting, my head is a bit fried 🤯 This is quite a big change, so I (or someone else with a bigger head) needs to take a new look with a fresh head!

Thanks for providing a fix to something that bugged you a year ago. That's persistence we can appreciate 😄 
benjamingr: > It's a bit scary breaking change, so I think I would like some more 👀 on this before going ahead.

I started reviewing it and came to the "bit scary" conclusion as well.
stale[bot]: This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

fatso83: @Oustinger This is getting a bit old, but from the comments I see that I basically OK'd it. Any chance you can get it over the finish line? Was fine work.
codecov[bot]: ## [Codecov](https://app.codecov.io/gh/sinonjs/fake-timers/pull/480?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sinonjs) Report
Attention: Patch coverage is `91.17647%` with `3 lines` in your changes missing coverage. Please review.
> Project coverage is 97.53%. Comparing base [(`6f90a44`)](https://app.codecov.io/gh/sinonjs/fake-timers/commit/6f90a44716583fddefcba0455d3c8f4bd30eca7a?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sinonjs) to head [(`35363ee`)](https://app.codecov.io/gh/sinonjs/fake-timers/commit/35363eeddfed69e196594334f4e60556d710b18c?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sinonjs).
> Report is 6 commits behind head on main.

| [Files](https://app.codecov.io/gh/sinonjs/fake-timers/pull/480?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sinonjs) | Patch % | Lines |
|---|---|---|
| [src/fake-timers-src.js](https://app.codecov.io/gh/sinonjs/fake-timers/pull/480?src=pr&el=tree&filepath=src%2Ffake-timers-src.js&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sinonjs#diff-c3JjL2Zha2UtdGltZXJzLXNyYy5qcw==) | 81.25% | [3 Missing :warning: ](https://app.codecov.io/gh/sinonjs/fake-timers/pull/480?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sinonjs) |

<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##             main     #480      +/-   ##
==========================================
- Coverage   97.56%   97.53%   -0.03%     
==========================================
  Files          16       17       +1     
  Lines        4430     4424       -6     
==========================================
- Hits         4322     4315       -7     
- Misses        108      109       +1     
```

| [Flag](https://app.codecov.io/gh/sinonjs/fake-timers/pull/480/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sinonjs) | Coverage Δ | |
|---|---|---|
| [unit](https://app.codecov.io/gh/sinonjs/fake-timers/pull/480/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sinonjs) | `97.53% <91.17%> (-0.03%)` | :arrow_down: |

Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sinonjs#carryforward-flags-in-the-pull-request-comment) to find out more.

</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/sinonjs/fake-timers/pull/480?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sinonjs).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sinonjs).


Patch:
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") {


Test patch:
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/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();
+    });
+});