Replace old helper with new

Author: GeoffreyBooth

lib/internal/modules/cjs/loader.js

@@ -90,6 +90,7 @@ const {
  makeContextifyScript,
  runScriptInThisContext,
 } = require('internal/vm');
+const { containsModuleSyntax } = internalBinding('contextify');
 
 const assert = require('internal/assert');
 const fs = require('fs');
@@ -104,7 +105,6 @@ const {
 const {
  getCjsConditions,
  initializeCjsConditions,
- hasEsmSyntax,
  loadBuiltinModule,
  makeRequireFunction,
  normalizeReferrerURL,
@@ -1315,7 +1315,7 @@ function wrapSafe(filename, content, cjsModuleInstance, codeCache) {
  } catch (err) {
  if (process.mainModule === cjsModuleInstance) {
  const { enrichCJSError } = require('internal/modules/esm/translators');
- enrichCJSError(err, content);
+ enrichCJSError(err, content, filename);
  }
  throw err;
  }
@@ -1400,10 +1400,11 @@ Module._extensions['.js'] = function(module, filename) {
  const pkg = packageJsonReader.readPackageScope(filename) || { __proto__: null };
  // Function require shouldn't be used in ES modules.
  if (pkg.data?.type === 'module') {
+ // This is an error path, because `require` of a `.js` file in a `"type": "module"` scope is not allowed.
  const parent = moduleParentCache.get(module);
  const parentPath = parent?.filename;
  const packageJsonPath = path.resolve(pkg.path, 'package.json');
- const usesEsm = hasEsmSyntax(content);
+ const usesEsm = containsModuleSyntax(content, filename);
  const err = new ERR_REQUIRE_ESM(filename, usesEsm, parentPath,
  packageJsonPath);
  // Attempt to reconstruct the parent require frame.

lib/internal/modules/esm/translators.js

@@ -30,11 +30,11 @@ function lazyTypes() {
  return _TYPES = require('internal/util/types');
 }
 
+const { containsModuleSyntax } = internalBinding('contextify');
 const assert = require('internal/assert');
 const { readFileSync } = require('fs');
 const { dirname, extname, isAbsolute } = require('path');
 const {
- hasEsmSyntax,
  loadBuiltinModule,
  stripBOM,
 } = require('internal/modules/helpers');
@@ -166,11 +166,11 @@ translators.set('module', async function moduleStrategy(url, source, isMain) {
  * Provide a more informative error for CommonJS imports.
  * @param {Error | any} err
  * @param {string} [content] Content of the file, if known.
- * @param {string} [filename] Useful only if `content` is unknown.
+ * @param {string} [filename] The filename of the erroring module.
  */
 function enrichCJSError(err, content, filename) {
  if (err != null && ObjectGetPrototypeOf(err) === SyntaxErrorPrototype &&
- hasEsmSyntax(content || readFileSync(filename, 'utf-8'))) {
+ containsModuleSyntax(content, filename ?? '')) {
  // Emit the warning synchronously because we are in the middle of handling
  // a SyntaxError that will throw and likely terminate the process before an
  // asynchronous warning would be emitted.
@@ -217,7 +217,7 @@ function loadCJSModule(module, source, url, filename) {
  importModuleDynamically, // importModuleDynamically
  ).function;
  } catch (err) {
- enrichCJSError(err, source, url);
+ enrichCJSError(err, source, filename);
  throw err;
  }
 
@@ -344,7 +344,7 @@ translators.set('commonjs', async function commonjsStrategy(url, source,
  assert(module === CJSModule._cache[filename]);
  CJSModule._load(filename);
  } catch (err) {
- enrichCJSError(err, source, url);
+ enrichCJSError(err, source, filename);
  throw err;
  }
  } : loadCJSModule;

lib/internal/modules/helpers.js

@@ -3,7 +3,6 @@
 const {
  ArrayPrototypeForEach,
  ArrayPrototypeJoin,
- ArrayPrototypeSome,
  ObjectDefineProperty,
  ObjectPrototypeHasOwnProperty,
  SafeMap,
@@ -299,32 +298,10 @@ function normalizeReferrerURL(referrer) {
  return new URL(referrer).href;
 }
 
-/**
- * For error messages only, check if ESM syntax is in use.
- * @param {string} code
- */
-function hasEsmSyntax(code) {
- debug('Checking for ESM syntax');
- const parser = require('internal/deps/acorn/acorn/dist/acorn').Parser;
- let root;
- try {
- root = parser.parse(code, { sourceType: 'module', ecmaVersion: 'latest' });
- } catch {
- return false;
- }
-
- return ArrayPrototypeSome(root.body, (stmt) =>
- stmt.type === 'ExportDefaultDeclaration' ||
- stmt.type === 'ExportNamedDeclaration' ||
- stmt.type === 'ImportDeclaration' ||
- stmt.type === 'ExportAllDeclaration');
-}
-
 module.exports = {
  addBuiltinLibsToObject,
  getCjsConditions,
  initializeCjsConditions,
- hasEsmSyntax,
  loadBuiltinModule,
  makeRequireFunction,
  normalizeReferrerURL,

src/node_contextify.cc

@@ -1206,32 +1206,17 @@ void ContextifyContext::CompileFunction(
  data + cached_data_buf->ByteOffset(), cached_data_buf->ByteLength());
  }
 
- // Set host_defined_options
  Local<PrimitiveArray> host_defined_options =
- PrimitiveArray::New(isolate, loader::HostDefinedOptions::kLength);
- host_defined_options->Set(
- isolate, loader::HostDefinedOptions::kID, id_symbol);
-
- ScriptOrigin origin(isolate,
- filename,
- line_offset, // line offset
- column_offset, // column offset
- true, // is cross origin
- -1, // script id
- Local<Value>(), // source map URL
- false, // is opaque (?)
- false, // is WASM
- false, // is ES Module
- host_defined_options);
-
+ GetHostDefinedOptions(isolate, id_symbol);
  ScriptCompiler::Source source =
- ScriptCompiler::Source(code, origin, cached_data);
- ScriptCompiler::CompileOptions options;
- if (source.GetCachedData() == nullptr) {
- options = ScriptCompiler::kNoCompileOptions;
- } else {
- options = ScriptCompiler::kConsumeCodeCache;
- }
+ GetCommonJSSourceInstance(isolate,
+ code,
+ filename,
+ line_offset,
+ column_offset,
+ host_defined_options,
+ cached_data);
+ ScriptCompiler::CompileOptions options = GetCompileOptions(source);
 
  Context::Scope scope(parsing_context);
 
@@ -1279,6 +1264,48 @@ void ContextifyContext::CompileFunction(
  args.GetReturnValue().Set(result);
 }
 
+Local<PrimitiveArray> ContextifyContext::GetHostDefinedOptions(
+ Isolate* isolate, Local<Symbol> id_symbol) {
+ Local<PrimitiveArray> host_defined_options =
+ PrimitiveArray::New(isolate, loader::HostDefinedOptions::kLength);
+ host_defined_options->Set(
+ isolate, loader::HostDefinedOptions::kID, id_symbol);
+ return host_defined_options;
+}
+
+ScriptCompiler::Source ContextifyContext::GetCommonJSSourceInstance(
+ Isolate* isolate,
+ Local<String> code,
+ Local<String> filename,
+ int line_offset,
+ int column_offset,
+ Local<PrimitiveArray> host_defined_options,
+ ScriptCompiler::CachedData* cached_data) {
+ ScriptOrigin origin(isolate,
+ filename,
+ line_offset, // line offset
+ column_offset, // column offset
+ true, // is cross origin
+ -1, // script id
+ Local<Value>(), // source map URL
+ false, // is opaque (?)
+ false, // is WASM
+ false, // is ES Module
+ host_defined_options);
+ return ScriptCompiler::Source(code, origin, cached_data);
+}
+
+ScriptCompiler::CompileOptions ContextifyContext::GetCompileOptions(
+ const ScriptCompiler::Source& source) {
+ ScriptCompiler::CompileOptions options;
+ if (source.GetCachedData() != nullptr) {
+ options = ScriptCompiler::kConsumeCodeCache;
+ } else {
+ options = ScriptCompiler::kNoCompileOptions;
+ }
+ return options;
+}
+
 Local<Object> ContextifyContext::CompileFunctionAndCacheResult(
  Environment* env,
  Local<Context> parsing_context,
@@ -1375,35 +1402,11 @@ void ContextifyContext::ContainsModuleSyntax(
  filename))
  .As<Symbol>();
 
- // TODO: Abstract this into a separate function
- // Set host_defined_options
  Local<PrimitiveArray> host_defined_options =
- PrimitiveArray::New(isolate, loader::HostDefinedOptions::kLength);
- host_defined_options->Set(
- isolate, loader::HostDefinedOptions::kID, id_symbol);
-
- ScriptOrigin origin(isolate,
- filename,
- 0, // line offset
- 0, // column offset
- true, // is cross origin
- -1, // script id
- Local<Value>(), // source map URL
- false, // is opaque (?)
- false, // is WASM
- false, // is ES Module
- host_defined_options);
-
- ScriptCompiler::CachedData* cached_data = nullptr;
- ScriptCompiler::Source source =
- ScriptCompiler::Source(code, origin, cached_data);
- ScriptCompiler::CompileOptions options;
- if (source.GetCachedData() == nullptr) {
- options = ScriptCompiler::kNoCompileOptions;
- } else {
- options = ScriptCompiler::kConsumeCodeCache;
- }
- // End TODO
+ GetHostDefinedOptions(isolate, id_symbol);
+ ScriptCompiler::Source source = GetCommonJSSourceInstance(
+ isolate, code, filename, 0, 0, host_defined_options, nullptr);
+ ScriptCompiler::CompileOptions options = GetCompileOptions(source);
 
  std::vector<Local<String>> params = {
  String::NewFromUtf8(isolate, "exports").ToLocalChecked(),

src/node_contextify.h

@@ -93,6 +93,18 @@ class ContextifyContext : public BaseObject {
  bool produce_cached_data,
  v8::Local<v8::Symbol> id_symbol,
  const errors::TryCatchScope& try_catch);
+ static v8::Local<v8::PrimitiveArray> GetHostDefinedOptions(
+ v8::Isolate* isolate, v8::Local<v8::Symbol> id_symbol);
+ static v8::ScriptCompiler::Source GetCommonJSSourceInstance(
+ v8::Isolate* isolate,
+ v8::Local<v8::String> code,
+ v8::Local<v8::String> filename,
+ int line_offset,
+ int column_offset,
+ v8::Local<v8::PrimitiveArray> host_defined_options,
+ v8::ScriptCompiler::CachedData* cached_data);
+ static v8::ScriptCompiler::CompileOptions GetCompileOptions(
+ const v8::ScriptCompiler::Source& source);
  static void ContainsModuleSyntax(
  const v8::FunctionCallbackInfo<v8::Value>& args);
  static void WeakCallback(