Commit 17b3ee3
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: #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: #45983
PR-URL: #46458
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>1 parent 731a7ae commit 17b3ee3
File tree
2 files changed
+18
-1
lines changed- src
- test/parallel
2 files changed
+18
-1
lines changed| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
528 | 528 | | |
529 | 529 | | |
530 | 530 | | |
531 | | - | |
| 531 | + | |
| 532 | + | |
| 533 | + | |
532 | 534 | | |
533 | 535 | | |
534 | 536 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
0 commit comments