[Bug]: Object.getOwnPropertySymbols() does not work against global instance with jest+node18

[le-cong]

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

[github-actions]

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.

[le-cong]

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

[github-actions]

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.

[le-cong]

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

[le-cong]

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

[dubzzz]

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

[dubzzz]

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

[dubzzz]

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: nodejs/node#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 🤔

[github-actions]

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.

[dubzzz]

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.

[dubzzz]

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

[github-actions]

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.

[dubzzz]

@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.

[SimenB]

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)?

[le-cong]

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.

[dubzzz]

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.

[mrazauskas]

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

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

Reference: #2549 (comment)

[dubzzz]

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).

[github-actions]

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.

[le-cong]

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.

[github-actions]

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.

[le-cong]

not sure the issue is from jest or node

[dubzzz]

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.

[dubzzz]

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

Just landed: nodejs/node#46615 (comment)

[github-actions]

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.

[mrazauskas]

Fails on Node v18.16, but works as expected on Node v18.17.

@dubzzz, you fixed it! Thanks.

[github-actions]

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.