Skip to content

Commit 05cffd1

Browse files
committed
fix: bare specifier handling
1 parent d31b72e commit 05cffd1

File tree

2 files changed

+27
-16
lines changed

2 files changed

+27
-16
lines changed

NativeScript/runtime/ModuleInternalCallbacks.mm

Lines changed: 12 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1291,16 +1291,9 @@ static bool IsDocumentsPath(const std::string& path) {
12911291
absPath = builtinPath;
12921292
}
12931293
} else if (IsLikelyOptionalModule(spec)) {
1294-
// In debug/test builds, surface a clear unresolved bare specifier error instead of
1295-
// synthesizing a placeholder module; more helpful signal during development.
1296-
if (RuntimeConfig.IsDebug) {
1297-
std::string msg = "Cannot resolve bare specifier '" + spec + "'";
1298-
isolate->ThrowException(v8::Exception::Error(tns::ToV8String(isolate, msg.c_str())));
1299-
return v8::MaybeLocal<v8::Module>();
1300-
}
1301-
1302-
// In release builds, create a placeholder that throws on use,
1303-
// so apps can guard optional imports without crashing at import time.
1294+
// Treat bare specifiers as optional modules by creating a placeholder ES module that
1295+
// throws on property access. This lets applications guard optional imports at runtime
1296+
// without crashing during startup, especially in development.
13041297
std::string appPath = RuntimeConfig.ApplicationPath;
13051298
std::string placeholderPath = NormalizePath(appPath + "/" + spec + ".mjs");
13061299

@@ -1330,10 +1323,16 @@ static bool IsDocumentsPath(const std::string& path) {
13301323
// File created successfully, now resolve it normally
13311324
absPath = placeholderPath;
13321325
} else {
1333-
// Failed to create file, fall back to throwing error
1326+
// Failed to create file. In debug, avoid throwing to keep dev sessions alive; in release
1327+
// throw to surface the missing optional module.
13341328
std::string msg = "Cannot find module " + spec + " (tried " + absPath + ")";
1335-
isolate->ThrowException(v8::Exception::Error(tns::ToV8String(isolate, msg)));
1336-
return v8::MaybeLocal<v8::Module>();
1329+
if (RuntimeConfig.IsDebug) {
1330+
Log(@"Debug mode - Optional module placeholder creation failed: %s", msg.c_str());
1331+
return v8::MaybeLocal<v8::Module>();
1332+
} else {
1333+
isolate->ThrowException(v8::Exception::Error(tns::ToV8String(isolate, msg)));
1334+
return v8::MaybeLocal<v8::Module>();
1335+
}
13371336
}
13381337
} else {
13391338
// Placeholder file already exists, use it

TestRunner/app/tests/HttpEsmLoaderTests.js

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,24 @@ describe("HTTP ESM Loader", function() {
1616
});
1717
});
1818
it("should surface helpful errors for unresolved bare specifiers", function(done) {
19-
import("bare-spec-example").then(function() {
20-
(done.fail ? done.fail : fail)("Bare specifier should not have resolved successfully");
19+
import("bare-spec-example").then(function(mod) {
20+
// Placeholder modules export a default Proxy. Accessing a property on that proxy
21+
// should throw with a helpful error message containing the specifier.
22+
let threw = false;
23+
try {
24+
// Trigger the proxy's get trap by accessing a property on the default export
25+
// eslint-disable-next-line no-unused-expressions
26+
mod && mod.default && mod.default.__touch__;
27+
} catch (useErr) {
28+
threw = true;
29+
const msg = (useErr && useErr.message) ? useErr.message : String(useErr);
30+
expect(msg).toContain("bare-spec-example");
31+
}
32+
expect(threw).toBe(true);
2133
done();
2234
}).catch(function(error) {
35+
// Other runtimes throw on import; assert message includes the specifier name.
2336
const message = (error && error.message) ? error.message : String(error);
24-
// Expect our thrown helpful message containing the specifier name
2537
expect(message).toContain("bare-spec-example");
2638
done();
2739
});

0 commit comments

Comments
 (0)