sinonjs / fake-timers

Fake setTimeout and friends (collectively known as "timers"). Useful in your JavaScript tests. Extracted from Sinon.JS
BSD 3-Clause "New" or "Revised" License
794 stars 103 forks source link

TypeError while initializing in Node 19 #438

Closed jonathanschoeller closed 1 year ago

jonathanschoeller commented 1 year ago

What did you expect to happen? The code below to execute without throwing an exception.

What actually happens An exception is thrown:

TypeError: Cannot assign to read only property 'performance' of object '[object global]

at hijackMethod (node_modules/@sinonjs/fake-timers/src/fake-timers-src.js:946:32)
at Object.install (node_modules/@sinonjs/fake-timers/src/fake-timers-src.js:1733:17)

How to reproduce Run the following:

var FakeTimers = require("@sinonjs/fake-timers");

var clock = FakeTimers.install();
jonathanschoeller commented 1 year ago

I appreciate that node 19 is just barely a day old and it's expected that things might not be compatible at first. 😄

benjamingr commented 1 year ago

Hey, this looks like a pretty simple fix - would you be interested in contributing a PR?

SimenB commented 1 year ago

I was unable to reproduce it here in @sinonjs/fake-timers, so I ended up hacking around it in Jest: https://github.com/facebook/jest/pull/13467/files#diff-7e6b39bebf54c704e027085672211430b8241b080e103d997757bcea915d2041R95-R96

Would love for it to be a fix here though, so I can revert that PR and bump the dep 🙂

SimenB commented 1 year ago

@jonathanschoeller I'm unable to reproduce the issue with your snippet, could you verify?

jonathanschoeller commented 1 year ago

@SimenB I can't seem to reproduce the issue now either. I must have had some other environment details I didn't record. Happy to close this off. Thanks for having a look.

akwodkiewicz commented 4 months ago

FYI the issue with performance being not writable is also present in Node 18 since 18.19.0.

The Jest fix for Node 19 (https://github.com/jestjs/jest/pull/13467) works fine there.

fatso83 commented 4 months ago

@akwodkiewicz since we are not able to reproduce this outside of Jest, would you mind telling us exactly what you did?

akwodkiewicz commented 4 months ago

It would be very hard, because what made me look into this issue is one of my projects had Node upgraded from 18.18.2 to 18.19.1 and some of the tests suites (run with Jest) started failing with this issue.

All of those errors looked like this

    TypeError: Cannot assign to read only property 'performance' of object '[object global]'

      462 |
      463 |         beforeEach(() => {
    > 464 |             jest.useFakeTimers();
          |                  ^
      465 |         });
      466 |
      467 |         afterEach(() => {

      at hijackMethod (../../../node_modules/@sinonjs/fake-timers/src/fake-timers-src.js:946:32)
      at Object.install (../../../node_modules/@sinonjs/fake-timers/src/fake-timers-src.js:1733:17)
      at Object.useFakeTimers (redacted.spec.ts:464:18)

Ultimately my issue was related to unexpected package hoisting by Yarn and having outdated jest-config used for tests. [^1] So the only thing I'm doing here is calling jest.useFakeTimers. Since you are not able to reproduce the error, maybe it's somehow Jest's fault that it modifies some invariants that you rely on.

I just posted my previous comment to let people know that it's not only Node 19 upwards that needs to use Jest >=29, but 18.19.x as well.

EDIT: There were also these:

TypeError: Can't install fake timers twice on the same global object.

      32 |     beforeEach(async () => {
      33 |         fixtures = await getFixtures();
    > 34 |         jest.useFakeTimers();
         |              ^
      35 |     });
      36 |
      37 |     afterEach(() => {

      at Object.install (../../../node_modules/@sinonjs/fake-timers/src/fake-timers-src.js:1642:19)
      at Object.useFakeTimers (redacted.spec.ts:34:14)

but I guess it's just a side-effect of having the first error, so I dismissed it.

[^1]: I knew that there's this issue with Node >=19 and Jest 28, and I am explicitly using Jest 29. But I dug deeper and found Jest 28 in my indirect dependencies. Once I removed unnecessary packages that relied on Jest 28, the issue went away.