◐ Shell
reader mode source ↗
Skip to content

Commit b07ad39

Browse files
joyeecheungmarco-ippolito
joyeecheung
authored and
marco-ippolito
committed
module: detect ESM syntax by trying to recompile as SourceTextModule
Instead of using an async function wrapper, just try compiling code with unknown module format as SourceTextModule when it cannot be compiled as CJS and the error message indicates that it's worth a retry. If it can be parsed as SourceTextModule then it's considered ESM. Also, move shouldRetryAsESM() to C++ completely so that we can reuse it in the CJS module loader for require(esm). Drive-by: move methods that don't belong to ContextifyContext out as static methods and move GetHostDefinedOptions to ModuleWrap. PR-URL: #52413 Backport-PR-URL: #56927 Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com> Reviewed-By: Jacob Smith <jacob@frende.me> Refs: #52697
1 parent 132a5c1 commit b07ad39

7 files changed

Lines changed: 303 additions & 339 deletions

File tree

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,7 @@ function getFileProtocolModuleFormat(url, context = { __proto__: null }, ignoreE
114114
// but this gets called again from `defaultLoad`/`defaultLoadSync`.
115115
if (getOptionValue('--experimental-detect-module')) {
116116
const format = source ?
117-
(containsModuleSyntax(`${source}`, fileURLToPath(url)) ? 'module' : 'commonjs') :
117+
(containsModuleSyntax(`${source}`, fileURLToPath(url), url) ? 'module' : 'commonjs') :
118118
null;
119119
if (format === 'module') {
120120
// This module has a .js extension, a package.json with no `type` field, and ESM syntax.
@@ -158,7 +158,7 @@ function getFileProtocolModuleFormat(url, context = { __proto__: null }, ignoreE
158158
if (!source) { return null; }
159159
const format = getFormatOfExtensionlessFile(url);
160160
if (format === 'module') {
161-
return containsModuleSyntax(`${source}`, fileURLToPath(url)) ? 'module' : 'commonjs';
161+
return containsModuleSyntax(`${source}`, fileURLToPath(url), url) ? 'module' : 'commonjs';
162162
}
163163
return format;
164164
}
Lines changed: 0 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -19,15 +19,6 @@ const {
1919
} = require('internal/errors').codes;
2020
const { BuiltinModule } = require('internal/bootstrap/realm');
2121

22-
const {
23-
shouldRetryAsESM: contextifyShouldRetryAsESM,
24-
constants: {
25-
syntaxDetectionErrors: {
26-
esmSyntaxErrorMessages,
27-
throwsOnlyInCommonJSErrorMessages,
28-
},
29-
},
30-
} = internalBinding('contextify');
3122
const { validateString } = require('internal/validators');
3223
const fs = require('fs'); // Import all of `fs` so that it can be monkey-patched.
3324
const internalFS = require('internal/fs/utils');
@@ -338,30 +329,6 @@ function urlToFilename(url) {
338329
return url;
339330
}
340331

341-
let esmSyntaxErrorMessagesSet; // Declared lazily in shouldRetryAsESM
342-
let throwsOnlyInCommonJSErrorMessagesSet; // Declared lazily in shouldRetryAsESM
343-
/**
344-
* After an attempt to parse a module as CommonJS throws an error, should we try again as ESM?
345-
* We only want to try again as ESM if the error is due to syntax that is only valid in ESM; and if the CommonJS parse
346-
* throws on an error that would not have been a syntax error in ESM (like via top-level `await` or a lexical
347-
* redeclaration of one of the CommonJS variables) then we need to parse again to see if it would have thrown in ESM.
348-
* @param {string} errorMessage The string message thrown by V8 when attempting to parse as CommonJS
349-
* @param {string} source Module contents
350-
*/
351-
function shouldRetryAsESM(errorMessage, source) {
352-
esmSyntaxErrorMessagesSet ??= new SafeSet(esmSyntaxErrorMessages);
353-
if (esmSyntaxErrorMessagesSet.has(errorMessage)) {
354-
return true;
355-
}
356-
357-
throwsOnlyInCommonJSErrorMessagesSet ??= new SafeSet(throwsOnlyInCommonJSErrorMessages);
358-
if (throwsOnlyInCommonJSErrorMessagesSet.has(errorMessage)) {
359-
return /** @type {boolean} */(contextifyShouldRetryAsESM(source));
360-
}
361-
362-
return false;
363-
}
364-
365332

366333
// Whether we have started executing any user-provided CJS code.
367334
// This is set right before we call the wrapped CJS code (not after,
@@ -396,7 +363,6 @@ module.exports = {
396363
loadBuiltinModule,
397364
makeRequireFunction,
398365
normalizeReferrerURL,
399-
shouldRetryAsESM,
400366
stripBOM,
401367
toRealPath,
402368
hasStartedUserCJSExecution() {
Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
'use strict';
22

33
const {
4+
ObjectGetPrototypeOf,
45
StringPrototypeEndsWith,
6+
SyntaxErrorPrototype,
57
} = primordials;
68

79
const { getOptionValue } = require('internal/options');
@@ -155,27 +157,29 @@ function runEntryPointWithESMLoader(callback) {
155157
function executeUserEntryPoint(main = process.argv[1]) {
156158
const resolvedMain = resolveMainPath(main);
157159
const useESMLoader = shouldUseESMLoader(resolvedMain);
158-
160+
let mainURL;
159161
// Unless we know we should use the ESM loader to handle the entry point per the checks in `shouldUseESMLoader`, first
160162
// try to run the entry point via the CommonJS loader; and if that fails under certain conditions, retry as ESM.
161163
let retryAsESM = false;
162164
if (!useESMLoader) {
163165
const cjsLoader = require('internal/modules/cjs/loader');
164166
const { Module } = cjsLoader;
165167
if (getOptionValue('--experimental-detect-module')) {
168+
// TODO(joyeecheung): handle this in the CJS loader. Don't try-catch here.
166169
try {
167170
// Module._load is the monkey-patchable CJS module loader.
168171
Module._load(main, null, true);
169172
} catch (error) {
170-
const source = cjsLoader.entryPointSource;
171-
const { shouldRetryAsESM } = require('internal/modules/helpers');
172-
retryAsESM = shouldRetryAsESM(error.message, source);
173-
// In case the entry point is a large file, such as a bundle,
174-
// ensure no further references can prevent it being garbage-collected.
175-
cjsLoader.entryPointSource = undefined;
173+
if (error != null && ObjectGetPrototypeOf(error) === SyntaxErrorPrototype) {
174+
const { shouldRetryAsESM } = internalBinding('contextify');
175+
const mainPath = resolvedMain || main;
176+
mainURL = pathToFileURL(mainPath).href;
177+
retryAsESM = shouldRetryAsESM(error.message, cjsLoader.entryPointSource, mainURL);
178+
// In case the entry point is a large file, such as a bundle,
179+
// ensure no further references can prevent it being garbage-collected.
180+
cjsLoader.entryPointSource = undefined;
181+
}
176182
if (!retryAsESM) {
177-
const { enrichCJSError } = require('internal/modules/esm/translators');
178-
enrichCJSError(error, source, resolvedMain);
179183
throw error;
180184
}
181185
}
@@ -186,7 +190,9 @@ function executeUserEntryPoint(main = process.argv[1]) {
186190

187191
if (useESMLoader || retryAsESM) {
188192
const mainPath = resolvedMain || main;
189-
const mainURL = pathToFileURL(mainPath).href;
193+
if (mainURL === undefined) {
194+
mainURL = pathToFileURL(mainPath).href;
195+
}
190196

191197
runEntryPointWithESMLoader((cascadedLoader) => {
192198
// Note that if the graph contains unfinished TLA, this may never resolve
Lines changed: 92 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -102,9 +102,17 @@ ModuleWrap* ModuleWrap::GetFromModule(Environment* env,
102102
return nullptr;
103103
}
104104

105-
// new ModuleWrap(url, context, source, lineOffset, columnOffset, cachedData)
105+
Local<PrimitiveArray> ModuleWrap::GetHostDefinedOptions(
106+
Isolate* isolate, Local<Symbol> id_symbol) {
107+
Local<PrimitiveArray> host_defined_options =
108+
PrimitiveArray::New(isolate, HostDefinedOptions::kLength);
109+
host_defined_options->Set(isolate, HostDefinedOptions::kID, id_symbol);
110+
return host_defined_options;
111+
}
112+
113+
// new ModuleWrap(url, context, source, lineOffset, columnOffset[, cachedData]);
106114
// new ModuleWrap(url, context, source, lineOffset, columOffset,
107-
// hostDefinedOption)
115+
// idSymbol);
108116
// new ModuleWrap(url, context, exportNames, evaluationCallback[, cjsModule])
109117
void ModuleWrap::New(const FunctionCallbackInfo<Value>& args) {
110118
CHECK(args.IsConstructCall());
@@ -134,7 +142,7 @@ void ModuleWrap::New(const FunctionCallbackInfo<Value>& args) {
134142
int column_offset = 0;
135143

136144
bool synthetic = args[2]->IsArray();
137-
145+
bool can_use_builtin_cache = false;
138146
Local<PrimitiveArray> host_defined_options =
139147
PrimitiveArray::New(isolate, HostDefinedOptions::kLength);
140148
Local<Symbol> id_symbol;
@@ -143,20 +151,24 @@ void ModuleWrap::New(const FunctionCallbackInfo<Value>& args) {
143151
// cjsModule])
144152
CHECK(args[3]->IsFunction());
145153
} else {
146-
// new ModuleWrap(url, context, source, lineOffset, columOffset, cachedData)
154+
// new ModuleWrap(url, context, source, lineOffset, columOffset[,
155+
// cachedData]);
147156
// new ModuleWrap(url, context, source, lineOffset, columOffset,
148-
// hostDefinedOption)
157+
// idSymbol);
149158
CHECK(args[2]->IsString());
150159
CHECK(args[3]->IsNumber());
151160
line_offset = args[3].As<Int32>()->Value();
152161
CHECK(args[4]->IsNumber());
153162
column_offset = args[4].As<Int32>()->Value();
154163
if (args[5]->IsSymbol()) {
155164
id_symbol = args[5].As<Symbol>();
165+
can_use_builtin_cache =
166+
(id_symbol ==
167+
realm->isolate_data()->source_text_module_default_hdo());
156168
} else {
157169
id_symbol = Symbol::New(isolate, url);
158170
}
159-
host_defined_options->Set(isolate, HostDefinedOptions::kID, id_symbol);
171+
host_defined_options = GetHostDefinedOptions(isolate, id_symbol);
160172

161173
if (that->SetPrivate(context,
162174
realm->isolate_data()->host_defined_option_symbol(),
@@ -189,36 +201,34 @@ void ModuleWrap::New(const FunctionCallbackInfo<Value>& args) {
189201
module = Module::CreateSyntheticModule(isolate, url, export_names,
190202
SyntheticModuleEvaluationStepsCallback);
191203
} else {
192-
ScriptCompiler::CachedData* cached_data = nullptr;
204+
// When we are compiling for the default loader, this will be
205+
// std::nullopt, and CompileSourceTextModule() should use
206+
// on-disk cache (not present on v20.x).
207+
std::optional<v8::ScriptCompiler::CachedData*> user_cached_data;
208+
if (id_symbol !=
209+
realm->isolate_data()->source_text_module_default_hdo()) {
210+
user_cached_data = nullptr;
211+
}
193212
if (args[5]->IsArrayBufferView()) {
213+
CHECK(!can_use_builtin_cache); // We don't use this option internally.
194214
Local<ArrayBufferView> cached_data_buf = args[5].As<ArrayBufferView>();
195215
uint8_t* data =
196216
static_cast<uint8_t*>(cached_data_buf->Buffer()->Data());
197-
cached_data =
217+
user_cached_data =
198218
new ScriptCompiler::CachedData(data + cached_data_buf->ByteOffset(),
199219
cached_data_buf->ByteLength());
200220
}
201-
202221
Local<String> source_text = args[2].As<String>();
203-
ScriptOrigin origin(isolate,
204-
url,
205-
line_offset,
206-
column_offset,
207-
true, // is cross origin
208-
-1, // script id
209-
Local<Value>(), // source map URL
210-
false, // is opaque (?)
211-
false, // is WASM
212-
true, // is ES Module
213-
host_defined_options);
214-
ScriptCompiler::Source source(source_text, origin, cached_data);
215-
ScriptCompiler::CompileOptions options;
216-
if (source.GetCachedData() == nullptr) {
217-
options = ScriptCompiler::kNoCompileOptions;
218-
} else {
219-
options = ScriptCompiler::kConsumeCodeCache;
220-
}
221-
if (!ScriptCompiler::CompileModule(isolate, &source, options)
222+
223+
bool cache_rejected = false;
224+
if (!CompileSourceTextModule(realm,
225+
source_text,
226+
url,
227+
line_offset,
228+
column_offset,
229+
host_defined_options,
230+
user_cached_data,
231+
&cache_rejected)
222232
.ToLocal(&module)) {
223233
if (try_catch.HasCaught() && !try_catch.HasTerminated()) {
224234
CHECK(!try_catch.Message().IsEmpty());
@@ -231,8 +241,9 @@ void ModuleWrap::New(const FunctionCallbackInfo<Value>& args) {
231241
}
232242
return;
233243
}
234-
if (options == ScriptCompiler::kConsumeCodeCache &&
235-
source.GetCachedData()->rejected) {
244+
245+
if (user_cached_data.has_value() && user_cached_data.value() != nullptr &&
246+
cache_rejected) {
236247
THROW_ERR_VM_MODULE_CACHED_DATA_REJECTED(
237248
realm, "cachedData buffer was rejected");
238249
try_catch.ReThrow();
@@ -275,6 +286,57 @@ void ModuleWrap::New(const FunctionCallbackInfo<Value>& args) {
275286
args.GetReturnValue().Set(that);
276287
}
277288

289+
MaybeLocal<Module> ModuleWrap::CompileSourceTextModule(
290+
Realm* realm,
291+
Local<String> source_text,
292+
Local<String> url,
293+
int line_offset,
294+
int column_offset,
295+
Local<PrimitiveArray> host_defined_options,
296+
std::optional<ScriptCompiler::CachedData*> user_cached_data,
297+
bool* cache_rejected) {
298+
Isolate* isolate = realm->isolate();
299+
EscapableHandleScope scope(isolate);
300+
ScriptOrigin origin(isolate,
301+
url,
302+
line_offset,
303+
column_offset,
304+
true, // is cross origin
305+
-1, // script id
306+
Local<Value>(), // source map URL
307+
false, // is opaque (?)
308+
false, // is WASM
309+
true, // is ES Module
310+
host_defined_options);
311+
ScriptCompiler::CachedData* cached_data = nullptr;
312+
// When compiling for the default loader, user_cached_data is std::nullptr.
313+
// When compiling for vm.Module, it's either nullptr or a pointer to the
314+
// cached data.
315+
if (user_cached_data.has_value()) {
316+
cached_data = user_cached_data.value();
317+
}
318+
319+
ScriptCompiler::Source source(source_text, origin, cached_data);
320+
ScriptCompiler::CompileOptions options;
321+
if (cached_data == nullptr) {
322+
options = ScriptCompiler::kNoCompileOptions;
323+
} else {
324+
options = ScriptCompiler::kConsumeCodeCache;
325+
}
326+
327+
Local<Module> module;
328+
if (!ScriptCompiler::CompileModule(isolate, &source, options)
329+
.ToLocal(&module)) {
330+
return scope.EscapeMaybe(MaybeLocal<Module>());
331+
}
332+
333+
if (options == ScriptCompiler::kConsumeCodeCache) {
334+
*cache_rejected = source.GetCachedData()->rejected;
335+
}
336+
337+
return scope.Escape(module);
338+
}
339+
278340
static Local<Object> createImportAttributesContainer(
279341
Realm* realm,
280342
Isolate* isolate,
Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,12 @@
33

44
#if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS
55

6-
#include <unordered_map>
6+
#include <optional>
77
#include <string>
8+
#include <unordered_map>
89
#include <vector>
910
#include "base_object.h"
11+
#include "v8-script.h"
1012

1113
namespace node {
1214

@@ -68,6 +70,23 @@ class ModuleWrap : public BaseObject {
6870
return true;
6971
}
7072

73+
static v8::Local<v8::PrimitiveArray> GetHostDefinedOptions(
74+
v8::Isolate* isolate, v8::Local<v8::Symbol> symbol);
75+
76+
// When user_cached_data is not std::nullopt, use the code cache if it's not
77+
// nullptr, otherwise don't use code cache.
78+
// TODO(joyeecheung): when it is std::nullopt, use on-disk cache
79+
// See: https://github.com/nodejs/node/issues/47472
80+
static v8::MaybeLocal<v8::Module> CompileSourceTextModule(
81+
Realm* realm,
82+
v8::Local<v8::String> source_text,
83+
v8::Local<v8::String> url,
84+
int line_offset,
85+
int column_offset,
86+
v8::Local<v8::PrimitiveArray> host_defined_options,
87+
std::optional<v8::ScriptCompiler::CachedData*> user_cached_data,
88+
bool* cache_rejected);
89+
7190
private:
7291
ModuleWrap(Realm* realm,
7392
v8::Local<v8::Object> object,

0 commit comments

Comments
 (0)