Leave global scope untouched

[bojavou]

• FakeTimers version : latest, 11.0.0
• Environment : Linux
• Example URL : https://github.com/bojavou/fake-timers-crash
• Other libraries you are using: None

What did you expect to happen?

The library loads without mutating the global scope.

What actually happens

The library attempts to mutate the global scope on load.

How to reproduce

This can be seen with the demo repo.

$ git clone https://github.com/bojavou/fake-timers-crash.git
$ cd fake-timers-crash
$ npm install
$ node load.js

/home/user/demo/fake-timers-crash/node_modules/@sinonjs/fake-timers/src/fake-timers-src.js:204
        _global.setImmediate = _global.setImmediate;
                             ^

TypeError: Cannot assign to read only property 'setImmediate' of object '#<Object>'
    at withGlobal (/home/user/demo/fake-timers-crash/node_modules/@sinonjs/fake-timers/src/fake-timers-src.js:204:30)
    at Object.<anonymous> (/home/user/demo/fake-timers-crash/node_modules/@sinonjs/fake-timers/src/fake-timers-src.js:1811:31)
    at Module._compile (node:internal/modules/cjs/loader:1275:14)
    at Module._extensions..js (node:internal/modules/cjs/loader:1329:10)
    at Module.load (node:internal/modules/cjs/loader:1133:32)
    at Module._load (node:internal/modules/cjs/loader:972:12)
    at Module.require (node:internal/modules/cjs/loader:1157:19)
    at require (node:internal/modules/helpers:119:18)
    at Object.<anonymous> (/home/user/demo/fake-timers-crash/load.js:5:1)
    at Module._compile (node:internal/modules/cjs/loader:1275:14)

Discussion

I'm running in a locked down environment with global properties frozen. This causes the library to crash. I see it through loading sinon in tests. I'd prefer if the library didn't touch the global scope unless I requested it.

On load there's a defaultImplementation constructed around the global scope.

const defaultImplementation = withGlobal(globalObject);

The withGlobal() function writes to setImmediate and clearImmediate.

if (setImmediatePresent) {
    _global.setImmediate = _global.setImmediate;
    _global.clearImmediate = _global.clearImmediate;
}

I'm not sure what the intention here is. The value is read and assigned to the same place. These look like they should be noops to me, but maybe there's some setter magic. Could these be safely removed? I'm willing to submit a patch if someone knows the right solution.

[fatso83]

You can have a look at the git log for those lines (originally from Sinon, so might be in Sinon 1.5 or something like that), but AFAIK this is a workaround for an old peculiarity with Internet Explorer. You needed to do this seemingly pointless song and dance to be able to mutate it later (or something of that sorts).

As we no longer support IE, only evergreen browsers, this and other hacks like it in Sinon or associated libraries, can be removed by anyone willing to provide the PR. So feel free ✌️

[benjamingr]

I think this was fixed in https://github.com/sinonjs/fake-timers/pull/477 ?

[fatso83]

Still need to release a new version, though :)

[fatso83]

Verified to have been fixed:

$ bash testscript.sh
Cloning into 'fake-timers-crash'...
remote: Enumerating objects: 4, done.
remote: Counting objects: 100% (4/4), done.
remote: Compressing objects: 100% (4/4), done.
remote: Total 4 (delta 0), reused 4 (delta 0), pack-reused 0
Receiving objects: 100% (4/4), done.

added 3 packages, and audited 4 packages in 2s

found 0 vulnerabilities
/Users/carlerik/dev/fake-timers-crash/node_modules/@sinonjs/fake-timers/src/fake-timers-src.js:204
        _global.setImmediate = _global.setImmediate;
                             ^

TypeError: Cannot assign to read only property 'setImmediate' of object '#<Object>'
    at withGlobal (/Users/carlerik/dev/fake-timers-crash/node_modules/@sinonjs/fake-timers/src/fake-timers-src.js:204:30)
    at Object.<anonymous> (/Users/carlerik/dev/fake-timers-crash/node_modules/@sinonjs/fake-timers/src/fake-timers-src.js:1811:31)
    at Module._compile (node:internal/modules/cjs/loader:1233:14)
    at Module._extensions..js (node:internal/modules/cjs/loader:1287:10)
    at Module.load (node:internal/modules/cjs/loader:1091:32)
    at Module._load (node:internal/modules/cjs/loader:938:12)
    at Module.require (node:internal/modules/cjs/loader:1115:19)
    at require (node:internal/modules/helpers:119:18)
    at Object.<anonymous> (/Users/carlerik/dev/fake-timers-crash/load.js:5:1)
    at Module._compile (node:internal/modules/cjs/loader:1233:14)

Node.js v20.5.0

carlerik at MacBook-Pro in ~/dev
$ z fake

carlerik at MacBook-Pro in ~/dev/fake-timers (main)
$ npm link

added 1 package, and audited 3 packages in 1s

found 0 vulnerabilities
Reshimming asdf nodejs...

carlerik at MacBook-Pro in ~/dev/fake-timers (main)
$ cd ../fake-timers-crash/

carlerik at MacBook-Pro in ~/dev/fake-timers-crash (main)
$ cat package.json
{
  "dependencies": {
    "@sinonjs/fake-timers": "^11.0.0"
  }
}

carlerik at MacBook-Pro in ~/dev/fake-timers-crash (main)
$ npm link @sinonjs/fake-timers

removed 2 packages, changed 1 package, and audited 3 packages in 763ms

found 0 vulnerabilities

carlerik at MacBook-Pro in ~/dev/fake-timers-crash (main)
$ cat ../testscript.sh
git clone https://github.com/bojavou/fake-timers-crash.git
cd fake-timers-crash
npm install
node load.js

carlerik at MacBook-Pro in ~/dev/fake-timers-crash (main)
$ node load.js

[fatso83]

Released as 11.1.0