Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

vm: properly support symbols on globals #46458

Merged
merged 3 commits into from
Feb 4, 2023
Merged

vm: properly support symbols on globals #46458

merged 3 commits into from
Feb 4, 2023

Commits on Feb 1, 2023

  1. vm: properly support symbols on globals 

    A regression has been introduced in node 18.2.0,
it makes the following snippet fails while it used to work in the past:

```
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));
```

Regression introduced by: nodejs#42963.
So I basically attempted to start understanding what it changed to make
it fix the initial issue while not breaking the symbol related one.

Fixes: nodejs#45983
    @dubzzz
    dubzzz committed Feb 1, 2023
    Configuration menu
    Copy the full SHA
    0599236 View commit details
    Browse the repository at this point in the history

  2. Adding missing is_contextual_store||check

    @dubzzz
    dubzzz committed Feb 1, 2023
    Configuration menu
    Copy the full SHA
    020bc92 View commit details
    Browse the repository at this point in the history

  3. Update test/parallel/test-vm-global-symbol.js 

    Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com>
    @dubzzz @aduh95
    dubzzz and aduh95 committed Feb 1, 2023
    Configuration menu
    Copy the full SHA
    b24cd68 View commit details
    Browse the repository at this point in the history