jest

jest copied to clipboard

Version

27/28/29

Steps to reproduce

run the following test case:

it('Object.getOwnPropertySymbols does not work against global instance with jest+node18', () => { const KEY = Symbol.for('key');

expect(global[KEY]).toBe(undefined); expect(Object.getOwnPropertySymbols(global).includes(KEY)).toBe(false);

global[KEY] = 'hello';

expect(global[KEY]).toBe('hello'); expect(Object.getOwnPropertySymbols(global).includes(KEY)).toBe(true); });

Expected behavior

the test should pass

Actual behavior

the test passes with Node JS <16, fails starting with Node JS 18

Additional context

No response

Environment

System:
    OS: macOS 12.5.1
    CPU: (10) arm64 Apple M1 Max
  Binaries:
    Node: 18.10.0 - ~/.nvm/versions/node/v18.10.0/bin/node
    npm: 8.19.2 - ~/.nvm/versions/node/v18.10.0/bin/npm
  npmPackages:
    jest: 28.1.3 => 28.1.3

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 30 days.

hi Jest team, is it possible for you to take a look of this issue? thank you so much!

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 30 days.

hi Jest team, is it possible for you to take a look of this issue? thank you so much!

any chance you could take a look, jest team? Thank you so much!

I just reported the very same issue on #13696. Definitely something strange going on 🤔 I tried to investigate a bit what can go wrong but so far no clue at all

If it can help, it passed on some legacy versions of Node 18 and started to fail at Node 18.2.0 (it passed on 18.1.0). On CodeSpace environments all versions of Jest had the issue on my end (I tested from 24 to 29).

So it seems the bug appeared due to something in this release of Node: https://github.com/nodejs/node/blob/main/doc/changelogs/CHANGELOG_V18.md#18.2.0

Probably related to the fact that this code started to fail at node 18.2.0:

const assert = require('assert');
const vm = require('vm');
const global = vm.runInContext('this', vm.createContext());
const totoSymbol = Symbol.for('toto');
Object.defineProperty(global, totoSymbol, {
  enumerable: true,
  writable: true,
  value: 4,
  configurable: true,
});
assert(Object.getOwnPropertySymbols(global).includes(totoSymbol));

Cc @SimenB: I opened this issue on node side: https://github.com/nodejs/node/issues/45983. From my understanding of the code and my manual tests I suspect that the issue comes from the piece of code in https://github.com/facebook/jest/blob/fb2de8a10f8e808b080af67aa771f67b5ea537ce/packages/jest-environment-node/src/index.ts#L72. But issue seems to be on node side if I get well how vm work 🤔

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 30 days.

Actually while the issue is probably not on Jest's side, the issue is still there as vm based code is somehow not behaving the same way following recent releases of node.

A fix should land soon in node 19. If no issue occurs, it will possibly be backported in node 18. More at https://github.com/nodejs/node/pull/46458

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 30 days.

@SimenB The fix number 1 has been merged on node side. It should be fixed with the next release of node 19. Node 18 still pending at least 2 more weeks after 19 (LTS). I also attempted another fix on node side as there is another pending bug on that part of their code.

Sweet, thanks for tackling this @dubzzz! Do you think we can close the issue here as it's an upstream bug (that's being fixed)?

Let's probably wait Node 19 release 🤔 I think it will come soon. At least I hope it will 😂

I'll check it soon but it might be fixed in node since 19.7.0.

https://github.com/nodejs/node/blob/main/doc/changelogs/CHANGELOG_V19.md#2023-02-21-version-1970-current-mylesborins

Hi @dubzzz, thank you for the effort. After trying out the latest version of node19 I found something interesting.

Basically whatever you fixed probably addressed a particular issue, which is "use symbol as key + set global property using Object.defineProperty() + apply writable=true for the descriptor". But the issue I reported originally is still not resolved. the different between our tests is that mine is using directly assignment ("=") instead of Object.defineProperty() function when setting up the global property.

To better illustrate the problem (and prove that it might have something to do with both jest and nodejs), I setup a new test suite which includes both plain js (not using jest) and regular jest tests.

• The plain js code always works disregarding which version of nodejs is used (just run node run-as-plain-node-js.js );
• The regular jest test cases using the exactly same underling code start failing since nodejs 18.0.0

My guess is that certain behavior changed since node18 and it's exposed in jest runtime for some reason. I don't have deep knowledge of the implementation details but maybe Jest team can share some related funding (e.g. possible wrapping/proxying logic against Global instance in jest, etc.?)

The good thing is that you've provided an alternative for us to make it work using Object.defineProperty() instead of direct assignment. I found that if you don't set writable: true it always works in Jest even with 18.2.0 😄

you can find all the code in this attachment x.zip

There is a follow up fix not yet merged released on node side. Let's hope it will fix this issue

The good thing is that you've provided an alternative for us to make it work using Object.defineProperty() instead of direct assignment. I found that if you don't set writable: true it always works in Jest even with 18.2.0 😄

Cc @SimenB, maybe something to do on Jest side, so that Jest don't even suffer from the issue. In the meantime I really hope my last node fix will fix 19 and then be backported to 18.

Node 19.9.0 just got released. I retried to run your test with it, but it still does not work 😭

it("Object.getOwnPropertySymbols does not work against global instance with jest+node18", () => {
  const KEY = Symbol.for("key");

  expect(globalThis[KEY]).toBe(undefined);
  expect(Object.getOwnPropertySymbols(globalThis).includes(KEY)).toBe(false);

  globalThis[KEY] = "hello";

  expect(globalThis[KEY]).toBe("hello");
  expect(Object.getOwnPropertySymbols(globalThis).includes(KEY)).toBe(true);
});

🔴 In Node 19.9.0 - includes https://github.com/nodejs/node/pull/46458 and https://github.com/nodejs/node/pull/46615 🔴 In Node 19.7.0 - includes https://github.com/nodejs/node/pull/46458 🔴 In Node 19.0.0 🔴 In Node 18.15.0 🔴 In Node 18.2.0 🔴 In Node 18.0.0 🟢 In Node 16.20.0

---

In the meantime, it seems that my attempt fixed #13696:

test('has toto', () => {
  const totoSymbol = Symbol.for('toto');
  Object.defineProperty(globalThis, totoSymbol, {
    enumerable: true,
    writable: true,
    value: 4,
    configurable: true,
  });
  expect(Object.getOwnPropertySymbols(globalThis).includes(totoSymbol)).toBe(true); // works with node 14, 16 and fails with node 18
});

🟢 In Node 19.9.0 - includes https://github.com/nodejs/node/pull/46458 and https://github.com/nodejs/node/pull/46615 🟢 In Node 19.7.0 - includes https://github.com/nodejs/node/pull/46458 🔴 In Node 19.0.0 🔴 In Node 18.15.0 🔴 In Node 18.2.0 🟢 In Node 18.0.0 🟢 In Node 16.20.0

---

I'll probably try to spend a little bit more time on Node vm code, to check how we can make this last case work fine.

it's great that you've fixed the issue with Object.defineProperty when writable: true flag is used. your effort will be greatly appreciated!

still the fact that the same code works in node.js only but fails in Jest+node suggests that it's not likely an issue caused by node.js only.

still the fact that the same code works in node.js only but fails in Jest+node suggests that it's not likely an issue caused by node.js only.

It works but not when executed within vm part of node. Jest uses vm internally: something coming from node and allowing Jest to properly make runs independent from each others.

It works but not when executed within vm part of node.

So using jest-light-runner could be a solution, right?

Reference: https://github.com/facebook/jest/issues/2549#issuecomment-1098071474

As jest-light-runner does not rely on vm it might work 🤔

I ran this code snippet (jest-free) against node versions 18.x and 19.x and confirmed the issue was linked to vm:

// Arrange
const assert = require("assert");
const vm = require("vm");
const global = vm.runInContext("this", vm.createContext());
const totoSymbol = Symbol.for("toto");
const titiSymbol = Symbol.for("titi");

// Act
Object.defineProperty(global, totoSymbol, {
  enumerable: true,
  writable: true,
  value: 4,
  configurable: true,
});
global[titiSymbol] = 4;

// Assert
assert(Object.getOwnPropertySymbols(global).includes(totoSymbol)); // fails with: >=18.2.0 <18.16.0, >=19.0.0 <19.7.0
assert(Object.getOwnPropertySymbols(global).includes(titiSymbol)); // fails with: 18.x, 19.x

Both assertions pass well in node 17.x! Version 18.x of node corresponds to a bump of v8. It seems that the bumped caused some issues within vm (attempts to patch them have been done on 18.2.0, then 18.16.0, then not yet released 18.17.0 but there are still others).

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 30 days.

I've been using Object.defineProperty as a workaround so far, it works but hopefully we can have a better solution to solve it perfectly.

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 30 days.

not sure the issue is from jest or node

The issue is definitely from node. But so far I'm waiting for the fix to be back-ported in node 18 and maybe 19 too.

Hope it will be totally fixed in node 18, now that the last fix landed.

Just landed: https://github.com/nodejs/node/pull/46615#issuecomment-1604484636