`Object.getOwnPropertyDescriptor(global, 'performance').writable` is no longer true in v18.19.0

[merceyz]

Version

v18.19.0

Platform

Linux unknown 5.15.0-92-generic #102-Ubuntu SMP Wed Jan 10 09:33:48 UTC 2024 x86_64 x86_64 x86_64 GNU/Linux

Subsystem

bootstrap

What steps will reproduce the bug?

console.assert(
	Object.getOwnPropertyDescriptor(global, 'performance').writable === true,
	'Expected global.performance to be writable',
);

How often does it reproduce? Is there a required condition?

Always

What is the expected behavior? Why is that the expected behavior?

global.performance should remain writable between minor versions.

What do you see instead?

Assertion failed: Expected global.performance to be writable

Additional information

Ref https://github.com/nodejs/node/issues/51048. Ref https://github.com/jestjs/jest/issues/14741.

git bisect points to 6466acbc899af8d8704bd18f19c9e9d66464c572.

Tests started failing in https://github.com/yarnpkg/berry/tree/a8857df67ba769e265b49542af9c31d9f05ee681 which uses jest@28, minimal reproduction for that:

cd $(mktemp -d)
npm init -y
npm i jest@28
echo "jest.useFakeTimers(); it('foo', () => {})" > jest.test.js
npx jest jest.test.js

which produces this error:

 FAIL  ./jest.test.js
  ● Test suite failed to run

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

    > 1 | jest.useFakeTimers(); it('foo', () => {})
        |                     ^
      2 |

      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 FakeTimers.useFakeTimers (node_modules/@jest/fake-timers/build/modernFakeTimers.js:110:36)
      at Object.<anonymous> (jest.test.js:1:21)

Test Suites: 1 failed, 1 total
Tests:       0 total
Snapshots:   0 total
Time:        0.237 s, estimated 1 s
Ran all test suites matching /jest.test.js/i.

[aduh95]

@nodejs/lts

[targos]

Is it supposed to be true ?

[kjleitz]

From a skim, yes, it looks like it used to be true, and the identified commit (https://github.com/nodejs/node/commit/6466acbc899af8d8704bd18f19c9e9d66464c572) omitted the writable property from the options passed to ObjectDefineProperty, which disabled it. I'm not super familiar with Node development, but if the behavior of ObjectDefineProperty() mirrors Object.defineProperty(), then writable defaults to false; an omission of that key will disable writability.

Was:

defineReplacableAttribute(globalThis, 'performance',
                          perf_hooks.performance);

// ...

function defineReplacableAttribute(target, name, value) {
  ObjectDefineProperty(target, name, {
    __proto__: null,
    writable: true,
    enumerable: true,
    configurable: true,
    value,
  });
}

...is now:

defineReplaceableLazyAttribute(globalThis, 'perf_hooks', ['performance']);

// ...

function defineReplaceableLazyAttribute(target, id, keys, writable = true) {
  let mod;
  for (let i = 0; i < keys.length; i++) {

    // ...

    ObjectDefineProperty(target, key, {
      __proto__: null,
      enumerable: true,
      configurable: true,
      get,
      set: writable ? set : undefined,
    });
  }
}

Notice that writable is now missing from the given options.