feat(ext/node): properly segregate node globals (#19307)

Code run within Deno-mode and Node-mode should have access to a
slightly different set of globals. Previously this was done through a
compile time code-transform for Node-mode, but this is not ideal and has
many edge cases, for example Node's globalThis having a different
identity than Deno's globalThis.

This commit makes the `globalThis` of the entire runtime a semi-proxy.
This proxy returns a different set of globals depending on the caller's
mode. This is not a full proxy, because it is shadowed by "real"
properties on globalThis. This is done to avoid the overhead of a full
proxy for all globalThis operations.

The globals between Deno-mode and Node-mode are now properly segregated.
This means that code running in Deno-mode will not have access to Node's
globals, and vice versa. Deleting a managed global in Deno-mode will
NOT delete the corresponding global in Node-mode, and vice versa.

---------

Co-authored-by: Bartek Iwańczuk <biwanczuk@gmail.com>
Co-authored-by: Aapo Alasuutari <aapo.alasuutari@gmail.com>

13 files changed, 115 insertions, 238 deletions

diff --git a/cli/cache/node.rs b/cli/cache/node.rs
index 825bdfef4..1637cbc78 100644
--- a/cli/cache/node.rs
+++ b/cli/cache/node.rs
@@ -12,26 +12,12 @@ use super::FastInsecureHasher;
 
 pub static NODE_ANALYSIS_CACHE_DB: CacheDBConfiguration =
   CacheDBConfiguration {
-    table_initializer: concat!(
-      "CREATE TABLE IF NOT EXISTS cjsanalysiscache (
+    table_initializer: "CREATE TABLE IF NOT EXISTS cjsanalysiscache (
       specifier TEXT PRIMARY KEY,
       source_hash TEXT NOT NULL,
       data TEXT NOT NULL
     );",
-      "CREATE UNIQUE INDEX IF NOT EXISTS cjsanalysiscacheidx
-      ON cjsanalysiscache(specifier);",
-      "CREATE TABLE IF NOT EXISTS esmglobalscache (
-      specifier TEXT PRIMARY KEY,
-      source_hash TEXT NOT NULL,
-      data TEXT NOT NULL
-    );",
-      "CREATE UNIQUE INDEX IF NOT EXISTS esmglobalscacheidx
-      ON esmglobalscache(specifier);",
-    ),
-    on_version_change: concat!(
-      "DELETE FROM cjsanalysiscache;",
-      "DELETE FROM esmglobalscache;",
-    ),
+    on_version_change: "DELETE FROM cjsanalysiscache;",
     preheat_queries: &[],
     on_failure: CacheFailure::InMemory,
   };
@@ -91,29 +77,6 @@ impl NodeAnalysisCache {
       cjs_analysis,
     ));
   }
-
-  pub fn get_esm_analysis(
-    &self,
-    specifier: &str,
-    expected_source_hash: &str,
-  ) -> Option<Vec<String>> {
-    Self::ensure_ok(
-      self.inner.get_esm_analysis(specifier, expected_source_hash),
-    )
-  }
-
-  pub fn set_esm_analysis(
-    &self,
-    specifier: &str,
-    source_hash: &str,
-    top_level_decls: &Vec<String>,
-  ) {
-    Self::ensure_ok(self.inner.set_esm_analysis(
-      specifier,
-      source_hash,
-      top_level_decls,
-    ))
-  }
 }
 
 #[derive(Clone)]
@@ -172,54 +135,6 @@ impl NodeAnalysisCacheInner {
     )?;
     Ok(())
   }
-
-  pub fn get_esm_analysis(
-    &self,
-    specifier: &str,
-    expected_source_hash: &str,
-  ) -> Result<Option<Vec<String>>, AnyError> {
-    let query = "
-      SELECT
-        data
-      FROM
-        esmglobalscache
-      WHERE
-        specifier=?1
-        AND source_hash=?2
-      LIMIT 1";
-    let res = self.conn.query_row(
-      query,
-      params![specifier, &expected_source_hash],
-      |row| {
-        let top_level_decls: String = row.get(0)?;
-        let decls: Vec<String> = serde_json::from_str(&top_level_decls)?;
-        Ok(decls)
-      },
-    )?;
-    Ok(res)
-  }
-
-  pub fn set_esm_analysis(
-    &self,
-    specifier: &str,
-    source_hash: &str,
-    top_level_decls: &Vec<String>,
-  ) -> Result<(), AnyError> {
-    let sql = "
-      INSERT OR REPLACE INTO
-        esmglobalscache (specifier, source_hash, data)
-      VALUES
-        (?1, ?2, ?3)";
-    self.conn.execute(
-      sql,
-      params![
-        specifier,
-        &source_hash,
-        &serde_json::to_string(top_level_decls)?,
-      ],
-    )?;
-    Ok(())
-  }
 }
 
 #[cfg(test)]
@@ -245,23 +160,10 @@ mod test {
     assert_eq!(actual_cjs_analysis.exports, cjs_analysis.exports);
     assert_eq!(actual_cjs_analysis.reexports, cjs_analysis.reexports);
 
-    assert!(cache.get_esm_analysis("file.js", "2").unwrap().is_none());
-    let esm_analysis = vec!["esm1".to_string()];
-    cache
-      .set_esm_analysis("file.js", "2", &esm_analysis)
-      .unwrap();
-    assert!(cache.get_esm_analysis("file.js", "3").unwrap().is_none()); // different hash
-    let actual_esm_analysis =
-      cache.get_esm_analysis("file.js", "2").unwrap().unwrap();
-    assert_eq!(actual_esm_analysis, esm_analysis);
-
     // adding when already exists should not cause issue
     cache
       .set_cjs_analysis("file.js", "2", &cjs_analysis)
       .unwrap();
-    cache
-      .set_esm_analysis("file.js", "2", &esm_analysis)
-      .unwrap();
 
     // recreating with same cli version should still have it
     let conn = cache.conn.recreate_with_version("1.0.0");
@@ -270,14 +172,10 @@ mod test {
       cache.get_cjs_analysis("file.js", "2").unwrap().unwrap();
     assert_eq!(actual_analysis.exports, cjs_analysis.exports);
     assert_eq!(actual_analysis.reexports, cjs_analysis.reexports);
-    let actual_esm_analysis =
-      cache.get_esm_analysis("file.js", "2").unwrap().unwrap();
-    assert_eq!(actual_esm_analysis, esm_analysis);
 
     // now changing the cli version should clear it
     let conn = cache.conn.recreate_with_version("2.0.0");
     let cache = NodeAnalysisCacheInner::new(conn);
     assert!(cache.get_cjs_analysis("file.js", "2").unwrap().is_none());
-    assert!(cache.get_esm_analysis("file.js", "2").unwrap().is_none());
   }
 }
diff --git a/cli/factory.rs b/cli/factory.rs
index 4e8da49d5..cb835181c 100644
--- a/cli/factory.rs
+++ b/cli/factory.rs
@@ -25,7 +25,7 @@ use crate::module_loader::CjsResolutionStore;
 use crate::module_loader::CliModuleLoaderFactory;
 use crate::module_loader::ModuleLoadPreparer;
 use crate::module_loader::NpmModuleLoader;
-use crate::node::CliCjsEsmCodeAnalyzer;
+use crate::node::CliCjsCodeAnalyzer;
 use crate::node::CliNodeCodeTranslator;
 use crate::npm::create_npm_fs_resolver;
 use crate::npm::CliNpmRegistryApi;
@@ -475,7 +475,7 @@ impl CliFactory {
         let caches = self.caches()?;
         let node_analysis_cache =
           NodeAnalysisCache::new(caches.node_analysis_db());
-        let cjs_esm_analyzer = CliCjsEsmCodeAnalyzer::new(node_analysis_cache);
+        let cjs_esm_analyzer = CliCjsCodeAnalyzer::new(node_analysis_cache);
 
         Ok(Arc::new(NodeCodeTranslator::new(
           cjs_esm_analyzer,
diff --git a/cli/js.rs b/cli/js.rs
index e3a5b94be..6a312a206 100644
--- a/cli/js.rs
+++ b/cli/js.rs
@@ -28,7 +28,6 @@ mod tests {
       if (!(bootstrap.mainRuntime && bootstrap.workerRuntime)) {
         throw Error("bad");
       }
-      console.log("we have console.log!!!");
     "#,
       )
       .unwrap();
diff --git a/cli/module_loader.rs b/cli/module_loader.rs
index 54efcd9dd..8395016b3 100644
--- a/cli/module_loader.rs
+++ b/cli/module_loader.rs
@@ -786,10 +786,8 @@ impl NpmModuleLoader {
         permissions,
       )?
     } else {
-      // only inject node globals for esm
-      self
-        .node_code_translator
-        .esm_code_with_node_globals(specifier, &code)?
+      // esm code is untouched
+      code
     };
     Ok(ModuleCodeSource {
       code: code.into(),
diff --git a/cli/node.rs b/cli/node.rs
index 8b54d0d42..75e0d9ef9 100644
--- a/cli/node.rs
+++ b/cli/node.rs
@@ -1,24 +1,17 @@
 // Copyright 2018-2023 the Deno authors. All rights reserved. MIT license.
 
-use std::collections::HashSet;
-
-use deno_ast::swc::common::SyntaxContext;
-use deno_ast::view::Node;
-use deno_ast::view::NodeTrait;
 use deno_ast::CjsAnalysis;
 use deno_ast::MediaType;
 use deno_ast::ModuleSpecifier;
-use deno_ast::ParsedSource;
-use deno_ast::SourceRanged;
 use deno_core::error::AnyError;
 use deno_runtime::deno_node::analyze::CjsAnalysis as ExtNodeCjsAnalysis;
-use deno_runtime::deno_node::analyze::CjsEsmCodeAnalyzer;
+use deno_runtime::deno_node::analyze::CjsCodeAnalyzer;
 use deno_runtime::deno_node::analyze::NodeCodeTranslator;
 
 use crate::cache::NodeAnalysisCache;
 use crate::util::fs::canonicalize_path_maybe_not_exists;
 
-pub type CliNodeCodeTranslator = NodeCodeTranslator<CliCjsEsmCodeAnalyzer>;
+pub type CliNodeCodeTranslator = NodeCodeTranslator<CliCjsCodeAnalyzer>;
 
 /// Resolves a specifier that is pointing into a node_modules folder.
 ///
@@ -39,11 +32,11 @@ pub fn resolve_specifier_into_node_modules(
     .unwrap_or_else(|| specifier.clone())
 }
 
-pub struct CliCjsEsmCodeAnalyzer {
+pub struct CliCjsCodeAnalyzer {
   cache: NodeAnalysisCache,
 }
 
-impl CliCjsEsmCodeAnalyzer {
+impl CliCjsCodeAnalyzer {
   pub fn new(cache: NodeAnalysisCache) -> Self {
     Self { cache }
   }
@@ -86,7 +79,7 @@ impl CliCjsEsmCodeAnalyzer {
   }
 }
 
-impl CjsEsmCodeAnalyzer for CliCjsEsmCodeAnalyzer {
+impl CjsCodeAnalyzer for CliCjsCodeAnalyzer {
   fn analyze_cjs(
     &self,
     specifier: &ModuleSpecifier,
@@ -98,104 +91,4 @@ impl CjsEsmCodeAnalyzer for CliCjsEsmCodeAnalyzer {
       reexports: analysis.reexports,
     })
   }
-
-  fn analyze_esm_top_level_decls(
-    &self,
-    specifier: &ModuleSpecifier,
-    source: &str,
-  ) -> Result<HashSet<String>, AnyError> {
-    // TODO(dsherret): this code is way more inefficient than it needs to be.
-    //
-    // In the future, we should disable capturing tokens & scope analysis
-    // and instead only use swc's APIs to go through the portions of the tree
-    // that we know will affect the global scope while still ensuring that
-    // `var` decls are taken into consideration.
-    let source_hash = NodeAnalysisCache::compute_source_hash(source);
-    if let Some(decls) = self
-      .cache
-      .get_esm_analysis(specifier.as_str(), &source_hash)
-    {
-      Ok(HashSet::from_iter(decls))
-    } else {
-      let parsed_source = deno_ast::parse_program(deno_ast::ParseParams {
-        specifier: specifier.to_string(),
-        text_info: deno_ast::SourceTextInfo::from_string(source.to_string()),
-        media_type: deno_ast::MediaType::from_specifier(specifier),
-        capture_tokens: true,
-        scope_analysis: true,
-        maybe_syntax: None,
-      })?;
-      let top_level_decls = analyze_top_level_decls(&parsed_source)?;
-      self.cache.set_esm_analysis(
-        specifier.as_str(),
-        &source_hash,
-        &top_level_decls.clone().into_iter().collect::<Vec<_>>(),
-      );
-      Ok(top_level_decls)
-    }
-  }
-}
-
-fn analyze_top_level_decls(
-  parsed_source: &ParsedSource,
-) -> Result<HashSet<String>, AnyError> {
-  fn visit_children(
-    node: Node,
-    top_level_context: SyntaxContext,
-    results: &mut HashSet<String>,
-  ) {
-    if let Node::Ident(ident) = node {
-      if ident.ctxt() == top_level_context && is_local_declaration_ident(node) {
-        results.insert(ident.sym().to_string());
-      }
-    }
-
-    for child in node.children() {
-      visit_children(child, top_level_context, results);
-    }
-  }
-
-  let top_level_context = parsed_source.top_level_context();
-
-  parsed_source.with_view(|program| {
-    let mut results = HashSet::new();
-    visit_children(program.into(), top_level_context, &mut results);
-    Ok(results)
-  })
-}
-
-fn is_local_declaration_ident(node: Node) -> bool {
-  if let Some(parent) = node.parent() {
-    match parent {
-      Node::BindingIdent(decl) => decl.id.range().contains(&node.range()),
-      Node::ClassDecl(decl) => decl.ident.range().contains(&node.range()),
-      Node::ClassExpr(decl) => decl
-        .ident
-        .as_ref()
-        .map(|i| i.range().contains(&node.range()))
-        .unwrap_or(false),
-      Node::TsInterfaceDecl(decl) => decl.id.range().contains(&node.range()),
-      Node::FnDecl(decl) => decl.ident.range().contains(&node.range()),
-      Node::FnExpr(decl) => decl
-        .ident
-        .as_ref()
-        .map(|i| i.range().contains(&node.range()))
-        .unwrap_or(false),
-      Node::TsModuleDecl(decl) => decl.id.range().contains(&node.range()),
-      Node::TsNamespaceDecl(decl) => decl.id.range().contains(&node.range()),
-      Node::VarDeclarator(decl) => decl.name.range().contains(&node.range()),
-      Node::ImportNamedSpecifier(decl) => {
-        decl.local.range().contains(&node.range())
-      }
-      Node::ImportDefaultSpecifier(decl) => {
-        decl.local.range().contains(&node.range())
-      }
-      Node::ImportStarAsSpecifier(decl) => decl.range().contains(&node.range()),
-      Node::KeyValuePatProp(decl) => decl.key.range().contains(&node.range()),
-      Node::AssignPatProp(decl) => decl.key.range().contains(&node.range()),
-      _ => false,
-    }
-  } else {
-    false
-  }
 }
diff --git a/cli/standalone/mod.rs b/cli/standalone/mod.rs
index 15abf77ec..d08df4b12 100644
--- a/cli/standalone/mod.rs
+++ b/cli/standalone/mod.rs
@@ -13,7 +13,7 @@ use crate::file_fetcher::get_source_from_data_url;
 use crate::http_util::HttpClient;
 use crate::module_loader::CjsResolutionStore;
 use crate::module_loader::NpmModuleLoader;
-use crate::node::CliCjsEsmCodeAnalyzer;
+use crate::node::CliCjsCodeAnalyzer;
 use crate::npm::create_npm_fs_resolver;
 use crate::npm::CliNpmRegistryApi;
 use crate::npm::CliNpmResolver;
@@ -366,7 +366,7 @@ pub async fn run(
   let cjs_resolutions = Arc::new(CjsResolutionStore::default());
   let cache_db = Caches::new(deno_dir_provider.clone());
   let node_analysis_cache = NodeAnalysisCache::new(cache_db.node_analysis_db());
-  let cjs_esm_code_analyzer = CliCjsEsmCodeAnalyzer::new(node_analysis_cache);
+  let cjs_esm_code_analyzer = CliCjsCodeAnalyzer::new(node_analysis_cache);
   let node_code_translator = Arc::new(NodeCodeTranslator::new(
     cjs_esm_code_analyzer,
     fs.clone(),
diff --git a/cli/tests/node_compat/polyfill_globals.js b/cli/tests/node_compat/polyfill_globals.js
new file mode 100644
index 000000000..493cec87a
--- /dev/null
+++ b/cli/tests/node_compat/polyfill_globals.js
@@ -0,0 +1,25 @@
+// Copyright 2018-2023 the Deno authors. All rights reserved. MIT license.
+import process from "node:process";
+import { Buffer } from "node:buffer";
+import {
+  clearImmediate,
+  clearInterval,
+  clearTimeout,
+  setImmediate,
+  setInterval,
+  setTimeout,
+} from "node:timers";
+import { performance } from "node:perf_hooks";
+import console from "node:console";
+globalThis.Buffer = Buffer;
+globalThis.clearImmediate = clearImmediate;
+globalThis.clearInterval = clearInterval;
+globalThis.clearTimeout = clearTimeout;
+globalThis.console = console;
+globalThis.global = globalThis;
+globalThis.performance = performance;
+globalThis.process = process;
+globalThis.setImmediate = setImmediate;
+globalThis.setInterval = setInterval;
+globalThis.setTimeout = setTimeout;
+delete globalThis.window;
diff --git a/cli/tests/node_compat/runner.ts b/cli/tests/node_compat/runner.ts
index f12cc69b0..93fca6548 100644
--- a/cli/tests/node_compat/runner.ts
+++ b/cli/tests/node_compat/runner.ts
@@ -1,4 +1,5 @@
 // Copyright 2018-2023 the Deno authors. All rights reserved. MIT license.
+import "./polyfill_globals.js";
 import { createRequire } from "node:module";
 const file = Deno.args[0];
 if (!file) {
diff --git a/cli/tests/testdata/npm/compare_globals/main.out b/cli/tests/testdata/npm/compare_globals/main.out
index 286834168..46900b882 100644
--- a/cli/tests/testdata/npm/compare_globals/main.out
+++ b/cli/tests/testdata/npm/compare_globals/main.out
@@ -4,7 +4,20 @@ Download http://localhost:4545/npm/registry/@denotest/globals/1.0.0.tgz
 Download http://localhost:4545/npm/registry/@types/node/node-18.8.2.tgz
 Check file:///[WILDCARD]/npm/compare_globals/main.ts
 true
+true
 []
-5
-undefined
+false
+function
+function
+function
 undefined
+false
+false
+true
+true
+true
+true
+false
+false
+bar
+bar
diff --git a/cli/tests/testdata/npm/compare_globals/main.ts b/cli/tests/testdata/npm/compare_globals/main.ts
index 0468404a8..8d3ae1ea0 100644
--- a/cli/tests/testdata/npm/compare_globals/main.ts
+++ b/cli/tests/testdata/npm/compare_globals/main.ts
@@ -2,6 +2,8 @@
 
 import * as globals from "npm:@denotest/globals";
 console.log(globals.global === globals.globalThis);
+// @ts-expect-error even though these are the same object, they have different types
+console.log(globals.globalThis === globalThis);
 console.log(globals.process.execArgv);
 
 type AssertTrue<T extends true> = never;
@@ -13,15 +15,36 @@ type _TestHasNodeJsGlobal = NodeJS.Architecture;
 const controller = new AbortController();
 controller.abort("reason"); // in the NodeJS declaration it doesn't have a reason
 
+// Some globals are not the same between Node and Deno.
+// @ts-expect-error incompatible types between Node and Deno
+console.log(globalThis.setTimeout === globals.getSetTimeout());
+
 // Super edge case where some Node code deletes a global where the
 // Node code has its own global and the Deno code has the same global,
 // but it's different. Basically if some Node code deletes
 // one of these globals then we don't want it to suddenly inherit
-// the Deno global.
-globals.withNodeGlobalThis((nodeGlobalThis: any) => {
-  (globalThis as any).setTimeout = 5;
-  console.log(setTimeout);
-  delete nodeGlobalThis["setTimeout"];
-  console.log(nodeGlobalThis["setTimeout"]); // should be undefined
-  console.log(globalThis["setTimeout"]); // should be undefined
-});
+// the Deno global (or touch the Deno global at all).
+console.log(typeof globalThis.setTimeout);
+console.log(typeof globals.getSetTimeout());
+globals.deleteSetTimeout();
+console.log(typeof globalThis.setTimeout);
+console.log(typeof globals.getSetTimeout());
+
+// In Deno, the process global is not defined, but in Node it is.
+console.log("process" in globalThis);
+console.log(
+  Object.getOwnPropertyDescriptor(globalThis, "process") !== undefined,
+);
+globals.checkProcessGlobal();
+
+// In Deno, the window global is defined, but in Node it is not.
+console.log("window" in globalThis);
+console.log(
+  Object.getOwnPropertyDescriptor(globalThis, "window") !== undefined,
+);
+globals.checkWindowGlobal();
+
+// "Non-managed" globals are shared between Node and Deno.
+(globalThis as any).foo = "bar";
+console.log((globalThis as any).foo);
+console.log(globals.getFoo());
diff --git a/cli/tests/testdata/npm/registry/@denotest/globals/1.0.0/index.d.ts b/cli/tests/testdata/npm/registry/@denotest/globals/1.0.0/index.d.ts
index 3f3eeb92a..1bbb82047 100644
--- a/cli/tests/testdata/npm/registry/@denotest/globals/1.0.0/index.d.ts
+++ b/cli/tests/testdata/npm/registry/@denotest/globals/1.0.0/index.d.ts
@@ -12,4 +12,10 @@ type _TestHasProcessGlobal = AssertTrue<
   typeof globalThis extends { process: any } ? true : false
 >;
 
-export function withNodeGlobalThis(action: (global: typeof globalThis) => void): void;
+export function deleteSetTimeout(): void;
+export function getSetTimeout(): typeof setTimeout;
+
+export function checkProcessGlobal(): void;
+export function checkWindowGlobal(): void;
+
+export function getFoo(): string;
\ No newline at end of file
diff --git a/cli/tests/testdata/npm/registry/@denotest/globals/1.0.0/index.js b/cli/tests/testdata/npm/registry/@denotest/globals/1.0.0/index.js
index daac83c66..b946bbd2a 100644
--- a/cli/tests/testdata/npm/registry/@denotest/globals/1.0.0/index.js
+++ b/cli/tests/testdata/npm/registry/@denotest/globals/1.0.0/index.js
@@ -2,6 +2,24 @@ exports.globalThis = globalThis;
 exports.global = global;
 exports.process = process;
 
-exports.withNodeGlobalThis = function (action) {
-  action(globalThis);
+exports.deleteSetTimeout = function () {
+  delete globalThis.setTimeout;
 };
+
+exports.getSetTimeout = function () {
+  return globalThis.setTimeout;
+};
+
+exports.checkProcessGlobal = function () {
+  console.log("process" in globalThis);
+  console.log(Object.getOwnPropertyDescriptor(globalThis, "process") !== undefined);
+};
+
+exports.checkWindowGlobal = function () {
+  console.log("window" in globalThis);
+  console.log(Object.getOwnPropertyDescriptor(globalThis, "window") !== undefined);
+}
+
+exports.getFoo = function () {
+  return globalThis.foo;
+}
\ No newline at end of file
diff --git a/cli/tests/testdata/run/with_package_json/npm_binary/main.out b/cli/tests/testdata/run/with_package_json/npm_binary/main.out
index 56cdae6f9..9e8e87bae 100644
--- a/cli/tests/testdata/run/with_package_json/npm_binary/main.out
+++ b/cli/tests/testdata/run/with_package_json/npm_binary/main.out
@@ -1,6 +1,9 @@
 [WILDCARD]package.json file found at '[WILDCARD]with_package_json[WILDCARD]npm_binary[WILDCARD]package.json'
 [WILDCARD]
 this
+[WILDCARD]
 is
+[WILDCARD]
 a
+[WILDCARD]
 test