Bug: Node `vm` doesn't have Node's `global` global

[marvinhagemeister]

Node's global variable isn't defined when running code inside Node's vm module.

Steps to reproduce

1. Create a new project with deno init
2. Run deno add npm:@marvinh/node-vm-test
3. Run this script:

import "@marvinh/node-vm-test";

Contents of the npm package:

import vm from "node:vm";
const result = vm.runInThisContext(`global.foo = 1`);
console.log(result);

Error:

error: Uncaught (in promise) TypeError: Cannot set properties of undefined (setting 'foo')
    at <anonymous>:1:12
    at Script.runInThisContext (node:vm:9:12)
    at Object.runInThisContext (node:vm:42:38)
    at file:///Users/marvinh/dev/test/deno-vm-global/foo.mjs:3:15

Running the same script in Node:

$ node foo.mjs
1

Version: Deno 1.43.3

[marvinhagemeister]

The process global isn't available either. We're likely not setting up the node context.

[bartlomieju]

I don't think that's the problem. Most likely it's related to https://github.com/denoland/deno/blob/main/ext/node/global.rs not recognizing that we're running inside node:vm.

[littledivy]

runInThisContext runs the code in the current context i.e Deno. By running it with Node globals we will change that definition.

This code should work when in a Node context i.e node_modules

[marvinhagemeister]

@littledivy Fair point: I've published the snippet as an npm package and updated the reproduction steps accordingly.

[littledivy]

Also blocking the Docusaurus upgrade in denoland/docs#412

[marvinhagemeister]

Took a look at this, and I can get it to work with this patch, but I'm not sure if this is the right fix.

diff --git a/ext/node/global.rs b/ext/node/global.rs
index 0aaa6ed8b..a3722641d 100644
--- a/ext/node/global.rs
+++ b/ext/node/global.rs
@@ -267,7 +267,9 @@ fn current_mode(scope: &mut v8::HandleScope) -> Mode {
   let Some(v8_string) =
     v8::StackTrace::current_script_name_or_source_url(scope)
   else {
-    return Mode::Deno;
+    // TODO: Add a flag to scope for inline scripts created
+    // via node:vm
+    return Mode::Node;
   };
   let op_state = deno_core::JsRuntime::op_state_from(scope);
   let op_state = op_state.borrow();
@@ -276,7 +278,7 @@ fn current_mode(scope: &mut v8::HandleScope) -> Mode {
   };
   let mut buffer = [MaybeUninit::uninit(); 2048];
   let str = v8_string.to_rust_cow_lossy(scope, &mut buffer);
-  if node_resolver.in_npm_package_with_cache(str) {
+  if str.starts_with("node:") || node_resolver.in_npm_package_with_cache(str) {
     Mode::Node
   } else {
     Mode::Deno

The first change isn't necessary for this particular reproduction case, but it is necessary to get vitest to work. Will try next week to find a proper test case that covers that.