Skip to content

Commit 2a3fe14

Browse files
edusperoniedusperoni
committed
fix(modules): throw when a module cannot be resolved
An unresolvable bare specifier resolved to a placeholder module whose exports was a Proxy that threw on property access, so requiring a missing module without dereferencing it succeeded silently. The ESM resolver had the same construct, plus debug-mode branches that returned an empty MaybeLocal without scheduling an exception, which is not a valid resolve callback result. The placeholder never worked in the first place: the module name was interpolated unquoted into a single-quoted JS string literal, so the generated proxy script was always a SyntaxError. On V8 10.3 the pending exception left by that failed compile escaped the require callback, which is the only reason "should throw error if cant find node module" passed. Both paths now throw "Cannot find module '<specifier>'".
1 parent aa2e26d commit 2a3fe14

3 files changed

Lines changed: 14 additions & 196 deletions

File tree

‎NativeScript/runtime/ModuleInternal.h‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -39,9 +39,6 @@ class ModuleInternal {
3939
const std::string& moduleName);
4040
std::string ResolvePathFromPackageJson(const std::string& packageJson,
4141
bool& error);
42-
v8::Local<v8::Object> CreatePlaceholderModule(v8::Isolate* isolate,
43-
const std::string& moduleName,
44-
const std::string& cacheKey);
4542
static v8::ScriptCompiler::CachedData* LoadScriptCache(
4643
const std::string& path);
4744
static void SaveScriptCache(const v8::Local<v8::Script> script,

‎NativeScript/runtime/ModuleInternal.mm‎

Lines changed: 2 additions & 101 deletions
Original file line numberDiff line numberDiff line change
@@ -17,16 +17,6 @@
1717

1818
namespace tns {
1919

20-
// Helper function to check if a module name looks like an optional external module
21-
bool IsLikelyOptionalModule(const std::string& moduleName) {
22-
// Check if it's a bare module name (no path separators) that could be an npm package
23-
if (moduleName.find('/') == std::string::npos && moduleName.find('\\') == std::string::npos &&
24-
moduleName[0] != '.' && moduleName[0] != '~' && moduleName[0] != '/') {
25-
return true;
26-
}
27-
return false;
28-
}
29-
3020
// Helper function to check if a file path is an ES module (.mjs) but not a source map (.mjs.map)
3121
bool IsESModule(const std::string& path) {
3222
return path.size() >= 4 && path.compare(path.size() - 4, 4, ".mjs") == 0 &&
@@ -411,21 +401,7 @@ bool IsESModule(const std::string& path) {
411401
}
412402

413403
if (path.empty()) {
414-
// For absolute paths (where baseDir is "/"), always throw an error instead of creating
415-
// placeholder
416-
bool isAbsolutePath = (baseDir == "/");
417-
418-
// Create placeholder module only for likely optional modules that aren't absolute paths
419-
if (!isAbsolutePath && IsLikelyOptionalModule(moduleName)) {
420-
return this->CreatePlaceholderModule(isolate, moduleName, cacheKey);
421-
}
422-
423-
// For absolute paths or non-optional modules, throw an error
424-
std::string errorMsg = "Module not found: '" + moduleName + "'";
425-
if (isAbsolutePath) {
426-
errorMsg = "Cannot find module '" + baseDir + moduleName + "'";
427-
}
428-
throw NativeScriptException(isolate, errorMsg, "Error");
404+
throw NativeScriptException(isolate, "Cannot find module '" + moduleName + "'", "Error");
429405
}
430406

431407
NSString* pathStr = [NSString stringWithUTF8String:path.c_str()];
@@ -1067,14 +1043,8 @@ throw NativeScriptException(
10671043
}
10681044

10691045
if (exists == NO) {
1070-
// Check if this looks like an optional module
1071-
if (IsLikelyOptionalModule(moduleName)) {
1072-
// Return empty string to indicate optional module not found
1073-
return std::string();
1074-
}
1075-
10761046
// Create a detailed error message with context
1077-
std::string errorMsg = "Module not found: '" + moduleName + "'";
1047+
std::string errorMsg = "Cannot find module '" + moduleName + "'";
10781048
errorMsg += "\n Base directory: " + baseDir;
10791049
errorMsg += "\n Attempted paths:";
10801050

@@ -1176,75 +1146,6 @@ throw NativeScriptException(
11761146
return std::string([[basePath stringByAppendingPathExtension:@"js"] UTF8String]);
11771147
}
11781148

1179-
Local<Object> ModuleInternal::CreatePlaceholderModule(Isolate* isolate,
1180-
const std::string& moduleName,
1181-
const std::string& cacheKey) {
1182-
Local<Context> context = isolate->GetCurrentContext();
1183-
1184-
// Create a module object with exports that throws when accessed
1185-
Local<Object> moduleObj = Object::New(isolate);
1186-
1187-
// Create a Proxy that throws an error when any property is accessed
1188-
std::string errorMessage =
1189-
"Module '" + moduleName + "' is not available. This is an optional module.";
1190-
std::string proxyCode = "(function() {"
1191-
" const error = new Error('" +
1192-
errorMessage +
1193-
"');"
1194-
" return new Proxy({}, {"
1195-
" get: function(target, prop) {"
1196-
" throw error;"
1197-
" },"
1198-
" set: function(target, prop, value) {"
1199-
" throw error;"
1200-
" },"
1201-
" has: function(target, prop) {"
1202-
" return false;"
1203-
" },"
1204-
" ownKeys: function(target) {"
1205-
" return [];"
1206-
" },"
1207-
" getPrototypeOf: function(target) {"
1208-
" return null;"
1209-
" }"
1210-
" });"
1211-
"})()";
1212-
1213-
Local<Script> proxyScript;
1214-
if (Script::Compile(context, tns::ToV8String(isolate, proxyCode.c_str())).ToLocal(&proxyScript)) {
1215-
Local<Value> proxyObject;
1216-
if (proxyScript->Run(context).ToLocal(&proxyObject)) {
1217-
// Set the exports to the proxy object
1218-
bool success = moduleObj->Set(context, tns::ToV8String(isolate, "exports"), proxyObject)
1219-
.FromMaybe(false);
1220-
if (!success) {
1221-
Log(@"Warning: Failed to set exports property on proxy module object");
1222-
}
1223-
}
1224-
}
1225-
1226-
// Set up the module object
1227-
bool success = moduleObj
1228-
->Set(context, tns::ToV8String(isolate, "id"),
1229-
tns::ToV8String(isolate, moduleName.c_str()))
1230-
.FromMaybe(false);
1231-
if (!success) {
1232-
Log(@"Warning: Failed to set id property on module object");
1233-
}
1234-
1235-
success =
1236-
moduleObj->Set(context, tns::ToV8String(isolate, "loaded"), v8::Boolean::New(isolate, true))
1237-
.FromMaybe(false);
1238-
if (!success) {
1239-
Log(@"Warning: Failed to set loaded property on module object");
1240-
}
1241-
1242-
// Cache the placeholder module
1243-
this->loadedModules_[cacheKey] = std::make_shared<Persistent<Object>>(isolate, moduleObj);
1244-
1245-
return moduleObj;
1246-
}
1247-
12481149
ScriptCompiler::CachedData* ModuleInternal::LoadScriptCache(const std::string& path) {
12491150
std::string canonicalPath = NormalizePath(path);
12501151
if (RuntimeConfig.IsDebug) {

‎NativeScript/runtime/ModuleInternalCallbacks.mm‎

Lines changed: 12 additions & 92 deletions
Original file line numberDiff line numberDiff line change
@@ -25,21 +25,6 @@
2525

2626
namespace tns {
2727

28-
// Helper function to check if a module name looks like an optional external module
29-
static bool IsLikelyOptionalModule(const std::string& moduleName) {
30-
// Skip Node.js built-in modules (they should be handled separately)
31-
if (moduleName.rfind("node:", 0) == 0) {
32-
return false;
33-
}
34-
35-
// Check if it's a bare module name (no path separators) that could be an npm package
36-
if (moduleName.find('/') == std::string::npos && moduleName.find('\\') == std::string::npos &&
37-
moduleName[0] != '.' && moduleName[0] != '~' && moduleName[0] != '/') {
38-
return true;
39-
}
40-
return false;
41-
}
42-
4328
// Helper function to check if a module name is a Node.js built-in module
4429
static bool IsNodeBuiltinModule(const std::string& moduleName) {
4530
return moduleName.rfind("node:", 0) == 0;
@@ -1375,68 +1360,13 @@ static bool IsDocumentsPath(const std::string& path) {
13751360
}
13761361
}
13771362

1378-
std::string msg = "Cannot find module " + spec + " (failed to create in-memory polyfill)";
1379-
if (RuntimeConfig.IsDebug) {
1380-
Log(@"Debug mode - Node.js polyfill creation failed: %s", msg.c_str());
1381-
return v8::MaybeLocal<v8::Module>();
1382-
} else {
1383-
isolate->ThrowException(v8::Exception::Error(tns::ToV8String(isolate, msg)));
1384-
return v8::MaybeLocal<v8::Module>();
1385-
}
1386-
} else if (IsLikelyOptionalModule(spec)) {
1387-
// Treat bare specifiers as optional modules with an in-memory placeholder ES module
1388-
// that throws on property access. This avoids bundle writes in iOS release builds.
1389-
1390-
std::string key = std::string("optional:") + spec;
1391-
auto itExisting = g_moduleRegistry.find(key);
1392-
if (itExisting != g_moduleRegistry.end()) {
1393-
v8::Local<v8::Module> existing = itExisting->second.Get(isolate);
1394-
if (!existing.IsEmpty() && existing->GetStatus() != v8::Module::kErrored) {
1395-
return v8::MaybeLocal<v8::Module>(existing);
1396-
}
1397-
RemoveModuleFromRegistry(key);
1398-
}
1399-
1400-
std::string placeholderContent = "const error = new Error(\"Module '" + spec +
1401-
"' is not available. This is an optional module.\");\n"
1402-
"const proxy = new Proxy({}, {\n"
1403-
" get: function(target, prop) { throw error; },\n"
1404-
" set: function(target, prop, value) { throw error; },\n"
1405-
" has: function(target, prop) { return false; },\n"
1406-
" ownKeys: function(target) { return []; },\n"
1407-
" getPrototypeOf: function(target) { return null; }\n"
1408-
"});\n"
1409-
"export default proxy;\n";
1410-
1411-
v8::MaybeLocal<v8::Module> m =
1412-
CompileModuleForResolveRegisterOnly(isolate, context, placeholderContent, key);
1413-
if (!m.IsEmpty()) {
1414-
v8::Local<v8::Module> mod;
1415-
if (m.ToLocal(&mod)) {
1416-
return m;
1417-
}
1418-
}
1419-
1420-
std::string msg =
1421-
"Cannot find module " + spec + " (failed to create in-memory optional placeholder)";
1422-
if (RuntimeConfig.IsDebug) {
1423-
Log(@"Debug mode - Optional module placeholder creation failed: %s", msg.c_str());
1424-
return v8::MaybeLocal<v8::Module>();
1425-
} else {
1426-
isolate->ThrowException(v8::Exception::Error(tns::ToV8String(isolate, msg)));
1427-
return v8::MaybeLocal<v8::Module>();
1428-
}
1363+
std::string msg = "Cannot find module '" + spec + "' (failed to create in-memory polyfill)";
1364+
isolate->ThrowException(v8::Exception::Error(tns::ToV8String(isolate, msg)));
1365+
return v8::MaybeLocal<v8::Module>();
14291366
} else {
1430-
// Not an optional module, throw the original error
1431-
std::string msg = "Cannot find module " + spec + " (tried " + absPath + ")";
1432-
if (RuntimeConfig.IsDebug) {
1433-
Log(@"Debug mode - Module not found: %s", msg.c_str());
1434-
// Return empty instead of crashing in debug mode
1435-
return v8::MaybeLocal<v8::Module>();
1436-
} else {
1437-
isolate->ThrowException(v8::Exception::Error(tns::ToV8String(isolate, msg)));
1438-
return v8::MaybeLocal<v8::Module>();
1439-
}
1367+
std::string msg = "Cannot find module '" + spec + "' (tried " + absPath + ")";
1368+
isolate->ThrowException(v8::Exception::Error(tns::ToV8String(isolate, msg)));
1369+
return v8::MaybeLocal<v8::Module>();
14401370
}
14411371
}
14421372

@@ -1467,14 +1397,9 @@ static bool IsDocumentsPath(const std::string& path) {
14671397
v8::Local<v8::String> urlString;
14681398
if (!v8::String::NewFromUtf8(isolate, url.c_str(), v8::NewStringType::kNormal)
14691399
.ToLocal(&urlString)) {
1470-
if (RuntimeConfig.IsDebug) {
1471-
Log(@"Debug mode - Failed to create URL string for JSON module");
1472-
return v8::MaybeLocal<v8::Module>();
1473-
} else {
1474-
isolate->ThrowException(v8::Exception::Error(
1475-
tns::ToV8String(isolate, "Failed to create URL string for JSON module")));
1476-
return v8::MaybeLocal<v8::Module>();
1477-
}
1400+
isolate->ThrowException(v8::Exception::Error(
1401+
tns::ToV8String(isolate, "Failed to create URL string for JSON module")));
1402+
return v8::MaybeLocal<v8::Module>();
14781403
}
14791404

14801405
v8::ScriptOrigin origin(urlString, 0, 0, false, -1, v8::Local<v8::Value>(), false, false,
@@ -1484,14 +1409,9 @@ static bool IsDocumentsPath(const std::string& path) {
14841409

14851410
v8::Local<v8::Module> jsonModule;
14861411
if (!v8::ScriptCompiler::CompileModule(isolate, &src).ToLocal(&jsonModule)) {
1487-
if (RuntimeConfig.IsDebug) {
1488-
Log(@"Debug mode - Failed to compile JSON module");
1489-
return v8::MaybeLocal<v8::Module>();
1490-
} else {
1491-
isolate->ThrowException(
1492-
v8::Exception::SyntaxError(tns::ToV8String(isolate, "Failed to compile JSON module")));
1493-
return v8::MaybeLocal<v8::Module>();
1494-
}
1412+
isolate->ThrowException(
1413+
v8::Exception::SyntaxError(tns::ToV8String(isolate, "Failed to compile JSON module")));
1414+
return v8::MaybeLocal<v8::Module>();
14951415
}
14961416

14971417
// No imports inside this module, so instantiate directly

0 commit comments

Comments
 (0)