From f9007d3386bbe9f709ce413ac0cf099b86d4c4bf Mon Sep 17 00:00:00 2001 From: snek Date: Tue, 10 Sep 2024 13:12:36 -0700 Subject: [PATCH] feat: require(esm) (#25501) implement require(esm) using `op_import_sync` from deno_core. possible future changes: - cts and mts - replace Deno.core.evalContext to optimize esm syntax detection Fixes: https://github.com/denoland/deno/issues/25487 --- ext/node/lib.rs | 1 + ext/node/ops/require.rs | 27 ++++++++ ext/node/polyfills/01_require.js | 65 +++++++------------ tests/integration/npm_tests.rs | 21 +++--- .../cjs-require-esm-error/1.0.0/package.json | 4 -- .../1.0.0/esm/my_es_module.js | 0 .../1.0.0/esm/package.json | 0 .../1.0.0/esm_mjs.mjs | 0 .../1.0.0/index.js | 0 .../cjs-require-esm/1.0.0/package.json | 4 ++ .../1.0.0/require_mjs.js | 0 tests/specs/run/require_esm/__test__.jsonc | 5 ++ tests/specs/run/require_esm/async.js | 2 + tests/specs/run/require_esm/main.cjs | 5 ++ tests/specs/run/require_esm/main.out | 13 ++++ tests/specs/run/require_esm/sync.js | 1 + tests/specs/run/require_esm/sync.mjs | 1 + .../{require_esm_error => require_esm}/esm.js | 0 tests/testdata/node/require_esm/main.out | 1 + .../main.ts | 2 +- .../testdata/node/require_esm_error/main.out | 3 - tests/testdata/npm/cjs_require_esm/main.out | 4 ++ tests/testdata/npm/cjs_require_esm/main.ts | 2 + .../npm/cjs_require_esm_error/main.out | 2 - .../npm/cjs_require_esm_error/main.ts | 1 - .../testdata/npm/cjs_require_esm_mjs/main.out | 4 ++ .../testdata/npm/cjs_require_esm_mjs/main.ts | 2 + .../npm/cjs_require_esm_mjs_error/main.out | 2 - .../npm/cjs_require_esm_mjs_error/main.ts | 1 - 29 files changed, 106 insertions(+), 67 deletions(-) delete mode 100644 tests/registry/npm/@denotest/cjs-require-esm-error/1.0.0/package.json rename tests/registry/npm/@denotest/{cjs-require-esm-error => cjs-require-esm}/1.0.0/esm/my_es_module.js (100%) rename tests/registry/npm/@denotest/{cjs-require-esm-error => cjs-require-esm}/1.0.0/esm/package.json (100%) rename tests/registry/npm/@denotest/{cjs-require-esm-error => cjs-require-esm}/1.0.0/esm_mjs.mjs (100%) rename tests/registry/npm/@denotest/{cjs-require-esm-error => cjs-require-esm}/1.0.0/index.js (100%) create mode 100644 tests/registry/npm/@denotest/cjs-require-esm/1.0.0/package.json rename tests/registry/npm/@denotest/{cjs-require-esm-error => cjs-require-esm}/1.0.0/require_mjs.js (100%) create mode 100644 tests/specs/run/require_esm/__test__.jsonc create mode 100644 tests/specs/run/require_esm/async.js create mode 100644 tests/specs/run/require_esm/main.cjs create mode 100644 tests/specs/run/require_esm/main.out create mode 100644 tests/specs/run/require_esm/sync.js create mode 100644 tests/specs/run/require_esm/sync.mjs rename tests/testdata/node/{require_esm_error => require_esm}/esm.js (100%) create mode 100644 tests/testdata/node/require_esm/main.out rename tests/testdata/node/{require_esm_error => require_esm}/main.ts (73%) delete mode 100644 tests/testdata/node/require_esm_error/main.out create mode 100644 tests/testdata/npm/cjs_require_esm/main.out create mode 100644 tests/testdata/npm/cjs_require_esm/main.ts delete mode 100644 tests/testdata/npm/cjs_require_esm_error/main.out delete mode 100644 tests/testdata/npm/cjs_require_esm_error/main.ts create mode 100644 tests/testdata/npm/cjs_require_esm_mjs/main.out create mode 100644 tests/testdata/npm/cjs_require_esm_mjs/main.ts delete mode 100644 tests/testdata/npm/cjs_require_esm_mjs_error/main.out delete mode 100644 tests/testdata/npm/cjs_require_esm_mjs_error/main.ts diff --git a/ext/node/lib.rs b/ext/node/lib.rs index 39b06e9fdd..f569f5b2a1 100644 --- a/ext/node/lib.rs +++ b/ext/node/lib.rs @@ -339,6 +339,7 @@ deno_core::extension!(deno_node, ops::os::op_homedir

, op_node_build_os, op_npm_process_state, + ops::require::op_require_can_parse_as_esm, ops::require::op_require_init_paths, ops::require::op_require_node_module_paths

, ops::require::op_require_proxy_path, diff --git a/ext/node/ops/require.rs b/ext/node/ops/require.rs index 4f88c1913b..3578719d0c 100644 --- a/ext/node/ops/require.rs +++ b/ext/node/ops/require.rs @@ -6,6 +6,7 @@ use deno_core::error::AnyError; use deno_core::normalize_path; use deno_core::op2; use deno_core::url::Url; +use deno_core::v8; use deno_core::JsRuntimeInspector; use deno_core::ModuleSpecifier; use deno_core::OpState; @@ -614,3 +615,29 @@ fn url_to_file_path(url: &Url) -> Result { } } } + +#[op2(fast)] +pub fn op_require_can_parse_as_esm( + scope: &mut v8::HandleScope, + #[string] source: &str, +) -> bool { + let scope = &mut v8::TryCatch::new(scope); + let Some(source) = v8::String::new(scope, source) else { + return false; + }; + let origin = v8::ScriptOrigin::new( + scope, + source.into(), + 0, + 0, + false, + 0, + None, + true, + false, + true, + None, + ); + let mut source = v8::script_compiler::Source::new(source, Some(&origin)); + v8::script_compiler::compile_module(scope, &mut source).is_some() +} diff --git a/ext/node/polyfills/01_require.js b/ext/node/polyfills/01_require.js index e4a781cc44..b84765d31d 100644 --- a/ext/node/polyfills/01_require.js +++ b/ext/node/polyfills/01_require.js @@ -4,9 +4,11 @@ import { core, internals, primordials } from "ext:core/mod.js"; import { + op_import_sync, op_napi_open, op_require_as_file_path, op_require_break_on_next_statement, + op_require_can_parse_as_esm, op_require_init_paths, op_require_is_deno_dir_package, op_require_is_request_relative, @@ -900,16 +902,6 @@ Module.prototype.load = function (filename) { pathDirname(this.filename), ); const extension = findLongestRegisteredExtension(filename); - // allow .mjs to be overridden - if ( - StringPrototypeEndsWith(filename, ".mjs") && !Module._extensions[".mjs"] - ) { - throw createRequireEsmError( - filename, - moduleParentCache.get(this)?.filename, - ); - } - Module._extensions[extension](this, this.filename); this.loaded = true; @@ -987,27 +979,24 @@ function wrapSafe( if (process.mainModule === cjsModuleInstance) { enrichCJSError(err.thrown); } - if (isEsmSyntaxError(err.thrown)) { - throw createRequireEsmError( - filename, - moduleParentCache.get(cjsModuleInstance)?.filename, - ); - } else { - throw err.thrown; - } + throw err.thrown; } return f; } Module.prototype._compile = function (content, filename, format) { - const compiledWrapper = wrapSafe(filename, content, this, format); - if (format === "module") { - // TODO(https://github.com/denoland/deno/issues/24822): implement require esm - throw createRequireEsmError( - filename, - moduleParentCache.get(module)?.filename, - ); + return loadESMFromCJS(this, filename, content); + } + + let compiledWrapper; + try { + compiledWrapper = wrapSafe(filename, content, this, format); + } catch (err) { + if (err instanceof SyntaxError && op_require_can_parse_as_esm(content)) { + return loadESMFromCJS(this, filename, content); + } + throw err; } const dirname = pathDirname(filename); @@ -1065,12 +1054,7 @@ Module._extensions[".js"] = function (module, filename) { if (StringPrototypeEndsWith(filename, ".js")) { const pkg = op_require_read_closest_package_json(filename); if (pkg?.typ === "module") { - // TODO(https://github.com/denoland/deno/issues/24822): implement require esm format = "module"; - throw createRequireEsmError( - filename, - moduleParentCache.get(module)?.filename, - ); } else if (pkg?.type === "commonjs") { format = "commonjs"; } @@ -1081,20 +1065,19 @@ Module._extensions[".js"] = function (module, filename) { module._compile(content, filename, format); }; -function createRequireEsmError(filename, parent) { - let message = `require() of ES Module ${filename}`; +function loadESMFromCJS(module, filename, code) { + const namespace = op_import_sync( + url.pathToFileURL(filename).toString(), + code, + ); - if (parent) { - message += ` from ${parent}`; - } - - message += - ` not supported. Instead change the require to a dynamic import() which is available in all CommonJS modules.`; - const err = new Error(message); - err.code = "ERR_REQUIRE_ESM"; - return err; + module.exports = namespace; } +Module._extensions[".mjs"] = function (module, filename) { + loadESMFromCJS(module, filename); +}; + function stripBOM(content) { if (StringPrototypeCharCodeAt(content, 0) === 0xfeff) { content = StringPrototypeSlice(content, 1); diff --git a/tests/integration/npm_tests.rs b/tests/integration/npm_tests.rs index 61ef0b22d3..aaea65d14c 100644 --- a/tests/integration/npm_tests.rs +++ b/tests/integration/npm_tests.rs @@ -58,26 +58,23 @@ itest!(cjs_invalid_name_exports { http_server: true, }); -itest!(cjs_require_esm_error { - args: "run --allow-read --quiet npm/cjs_require_esm_error/main.ts", - output: "npm/cjs_require_esm_error/main.out", +itest!(cjs_require_esm { + args: "run --allow-read --quiet npm/cjs_require_esm/main.ts", + output: "npm/cjs_require_esm/main.out", envs: env_vars_for_npm_tests(), http_server: true, - exit_code: 1, }); -itest!(cjs_require_esm_mjs_error { - args: "run --allow-read --quiet npm/cjs_require_esm_mjs_error/main.ts", - output: "npm/cjs_require_esm_mjs_error/main.out", +itest!(cjs_require_esm_mjs { + args: "run --allow-read --quiet npm/cjs_require_esm_mjs/main.ts", + output: "npm/cjs_require_esm_mjs/main.out", envs: env_vars_for_npm_tests(), http_server: true, - exit_code: 1, }); -itest!(require_esm_error { - args: "run --allow-read --quiet node/require_esm_error/main.ts", - output: "node/require_esm_error/main.out", - exit_code: 1, +itest!(require_esm { + args: "run --allow-read --quiet node/require_esm/main.ts", + output: "node/require_esm/main.out", }); itest!(dynamic_import_deno_ts_from_npm { diff --git a/tests/registry/npm/@denotest/cjs-require-esm-error/1.0.0/package.json b/tests/registry/npm/@denotest/cjs-require-esm-error/1.0.0/package.json deleted file mode 100644 index 08cd025f1a..0000000000 --- a/tests/registry/npm/@denotest/cjs-require-esm-error/1.0.0/package.json +++ /dev/null @@ -1,4 +0,0 @@ -{ - "name": "@denotest/cjs-require-esm-error", - "version": "1.0.0" -} diff --git a/tests/registry/npm/@denotest/cjs-require-esm-error/1.0.0/esm/my_es_module.js b/tests/registry/npm/@denotest/cjs-require-esm/1.0.0/esm/my_es_module.js similarity index 100% rename from tests/registry/npm/@denotest/cjs-require-esm-error/1.0.0/esm/my_es_module.js rename to tests/registry/npm/@denotest/cjs-require-esm/1.0.0/esm/my_es_module.js diff --git a/tests/registry/npm/@denotest/cjs-require-esm-error/1.0.0/esm/package.json b/tests/registry/npm/@denotest/cjs-require-esm/1.0.0/esm/package.json similarity index 100% rename from tests/registry/npm/@denotest/cjs-require-esm-error/1.0.0/esm/package.json rename to tests/registry/npm/@denotest/cjs-require-esm/1.0.0/esm/package.json diff --git a/tests/registry/npm/@denotest/cjs-require-esm-error/1.0.0/esm_mjs.mjs b/tests/registry/npm/@denotest/cjs-require-esm/1.0.0/esm_mjs.mjs similarity index 100% rename from tests/registry/npm/@denotest/cjs-require-esm-error/1.0.0/esm_mjs.mjs rename to tests/registry/npm/@denotest/cjs-require-esm/1.0.0/esm_mjs.mjs diff --git a/tests/registry/npm/@denotest/cjs-require-esm-error/1.0.0/index.js b/tests/registry/npm/@denotest/cjs-require-esm/1.0.0/index.js similarity index 100% rename from tests/registry/npm/@denotest/cjs-require-esm-error/1.0.0/index.js rename to tests/registry/npm/@denotest/cjs-require-esm/1.0.0/index.js diff --git a/tests/registry/npm/@denotest/cjs-require-esm/1.0.0/package.json b/tests/registry/npm/@denotest/cjs-require-esm/1.0.0/package.json new file mode 100644 index 0000000000..4ab7d0c4b5 --- /dev/null +++ b/tests/registry/npm/@denotest/cjs-require-esm/1.0.0/package.json @@ -0,0 +1,4 @@ +{ + "name": "@denotest/cjs-require-esm", + "version": "1.0.0" +} diff --git a/tests/registry/npm/@denotest/cjs-require-esm-error/1.0.0/require_mjs.js b/tests/registry/npm/@denotest/cjs-require-esm/1.0.0/require_mjs.js similarity index 100% rename from tests/registry/npm/@denotest/cjs-require-esm-error/1.0.0/require_mjs.js rename to tests/registry/npm/@denotest/cjs-require-esm/1.0.0/require_mjs.js diff --git a/tests/specs/run/require_esm/__test__.jsonc b/tests/specs/run/require_esm/__test__.jsonc new file mode 100644 index 0000000000..ee7d1eee96 --- /dev/null +++ b/tests/specs/run/require_esm/__test__.jsonc @@ -0,0 +1,5 @@ +{ + "args": "run -A main.cjs", + "output": "main.out", + "exitCode": 1 +} diff --git a/tests/specs/run/require_esm/async.js b/tests/specs/run/require_esm/async.js new file mode 100644 index 0000000000..b116c66acf --- /dev/null +++ b/tests/specs/run/require_esm/async.js @@ -0,0 +1,2 @@ +export const async_js = 1; +await {}; diff --git a/tests/specs/run/require_esm/main.cjs b/tests/specs/run/require_esm/main.cjs new file mode 100644 index 0000000000..b3418abe1a --- /dev/null +++ b/tests/specs/run/require_esm/main.cjs @@ -0,0 +1,5 @@ +"use strict"; + +console.log(require("./sync.js")); +console.log(require("./sync.mjs")); +require("./async.js"); diff --git a/tests/specs/run/require_esm/main.out b/tests/specs/run/require_esm/main.out new file mode 100644 index 0000000000..57b842b345 --- /dev/null +++ b/tests/specs/run/require_esm/main.out @@ -0,0 +1,13 @@ +[Module: null prototype] { sync_js: 1 } +[Module: null prototype] { sync_mjs: 1 } +error: Uncaught (in promise) Error: Top-level await is not allowed in synchronous evaluation + at loadESMFromCJS (node:module:[WILDCARD]) + at Module._compile (node:module:[WILDCARD]) + at Object.Module._extensions..js (node:module:[WILDCARD]) + at Module.load (node:module:[WILDCARD]) + at Function.Module._load (node:module:[WILDCARD]) + at Module.require (node:module:[WILDCARD]) + at require (node:module:[WILDCARD]) + at Object. (file:[WILDCARD]/tests/specs/run/require_esm/main.cjs:[WILDCARD]) + at Object. (file:[WILDCARD]/tests/specs/run/require_esm/main.cjs:[WILDCARD]) + at Module._compile (node:module:[WILDCARD]) diff --git a/tests/specs/run/require_esm/sync.js b/tests/specs/run/require_esm/sync.js new file mode 100644 index 0000000000..ca40d4eb78 --- /dev/null +++ b/tests/specs/run/require_esm/sync.js @@ -0,0 +1 @@ +export const sync_js = 1; diff --git a/tests/specs/run/require_esm/sync.mjs b/tests/specs/run/require_esm/sync.mjs new file mode 100644 index 0000000000..7a3cc3e276 --- /dev/null +++ b/tests/specs/run/require_esm/sync.mjs @@ -0,0 +1 @@ +export const sync_mjs = 1; diff --git a/tests/testdata/node/require_esm_error/esm.js b/tests/testdata/node/require_esm/esm.js similarity index 100% rename from tests/testdata/node/require_esm_error/esm.js rename to tests/testdata/node/require_esm/esm.js diff --git a/tests/testdata/node/require_esm/main.out b/tests/testdata/node/require_esm/main.out new file mode 100644 index 0000000000..aab0d5c285 --- /dev/null +++ b/tests/testdata/node/require_esm/main.out @@ -0,0 +1 @@ +[Module: null prototype] { Test: [class Test] } diff --git a/tests/testdata/node/require_esm_error/main.ts b/tests/testdata/node/require_esm/main.ts similarity index 73% rename from tests/testdata/node/require_esm_error/main.ts rename to tests/testdata/node/require_esm/main.ts index 612e917144..67ac808f06 100644 --- a/tests/testdata/node/require_esm_error/main.ts +++ b/tests/testdata/node/require_esm/main.ts @@ -2,4 +2,4 @@ import { createRequire } from "node:module"; const require = createRequire(import.meta.url); -require("./esm.js"); +console.log(require("./esm.js")); diff --git a/tests/testdata/node/require_esm_error/main.out b/tests/testdata/node/require_esm_error/main.out deleted file mode 100644 index 3db23ff244..0000000000 --- a/tests/testdata/node/require_esm_error/main.out +++ /dev/null @@ -1,3 +0,0 @@ -error: Uncaught (in promise) Error: require() of ES Module [WILDCARD]esm.js from [WILDCARD]main.ts not supported. Instead change the require to a dynamic import() which is available in all CommonJS modules. - at [WILDCARD] - at file:///[WILDCARD]/require_esm_error/main.ts:5:1 diff --git a/tests/testdata/npm/cjs_require_esm/main.out b/tests/testdata/npm/cjs_require_esm/main.out new file mode 100644 index 0000000000..4afceccc95 --- /dev/null +++ b/tests/testdata/npm/cjs_require_esm/main.out @@ -0,0 +1,4 @@ +[Module: null prototype] { + Test: [Module: null prototype] { Test: [class Test] }, + default: { Test: [Module: null prototype] { Test: [class Test] } } +} diff --git a/tests/testdata/npm/cjs_require_esm/main.ts b/tests/testdata/npm/cjs_require_esm/main.ts new file mode 100644 index 0000000000..069d4b60eb --- /dev/null +++ b/tests/testdata/npm/cjs_require_esm/main.ts @@ -0,0 +1,2 @@ +import * as ns from "npm:@denotest/cjs-require-esm"; +console.log(ns); diff --git a/tests/testdata/npm/cjs_require_esm_error/main.out b/tests/testdata/npm/cjs_require_esm_error/main.out deleted file mode 100644 index b6ade69042..0000000000 --- a/tests/testdata/npm/cjs_require_esm_error/main.out +++ /dev/null @@ -1,2 +0,0 @@ -error: Uncaught (in promise) Error: require() of ES Module [WILDCARD]my_es_module.js from [WILDCARD]index.js not supported. Instead change the require to a dynamic import() which is available in all CommonJS modules. - [WILDCARD] diff --git a/tests/testdata/npm/cjs_require_esm_error/main.ts b/tests/testdata/npm/cjs_require_esm_error/main.ts deleted file mode 100644 index 3fbb1215ad..0000000000 --- a/tests/testdata/npm/cjs_require_esm_error/main.ts +++ /dev/null @@ -1 +0,0 @@ -import "npm:@denotest/cjs-require-esm-error"; diff --git a/tests/testdata/npm/cjs_require_esm_mjs/main.out b/tests/testdata/npm/cjs_require_esm_mjs/main.out new file mode 100644 index 0000000000..4afceccc95 --- /dev/null +++ b/tests/testdata/npm/cjs_require_esm_mjs/main.out @@ -0,0 +1,4 @@ +[Module: null prototype] { + Test: [Module: null prototype] { Test: [class Test] }, + default: { Test: [Module: null prototype] { Test: [class Test] } } +} diff --git a/tests/testdata/npm/cjs_require_esm_mjs/main.ts b/tests/testdata/npm/cjs_require_esm_mjs/main.ts new file mode 100644 index 0000000000..d753c2a831 --- /dev/null +++ b/tests/testdata/npm/cjs_require_esm_mjs/main.ts @@ -0,0 +1,2 @@ +import * as ns from "npm:@denotest/cjs-require-esm/require_mjs.js"; +console.log(ns); diff --git a/tests/testdata/npm/cjs_require_esm_mjs_error/main.out b/tests/testdata/npm/cjs_require_esm_mjs_error/main.out deleted file mode 100644 index e779cfaf84..0000000000 --- a/tests/testdata/npm/cjs_require_esm_mjs_error/main.out +++ /dev/null @@ -1,2 +0,0 @@ -error: Uncaught (in promise) Error: require() of ES Module [WILDCARD]esm_mjs.mjs from [WILDCARD]require_mjs.js not supported. Instead change the require to a dynamic import() which is available in all CommonJS modules. - [WILDCARD] diff --git a/tests/testdata/npm/cjs_require_esm_mjs_error/main.ts b/tests/testdata/npm/cjs_require_esm_mjs_error/main.ts deleted file mode 100644 index 2121f1dbb4..0000000000 --- a/tests/testdata/npm/cjs_require_esm_mjs_error/main.ts +++ /dev/null @@ -1 +0,0 @@ -import "npm:@denotest/cjs-require-esm-error/require_mjs.js";