Skip to content

Commit 314d7d5

Browse files
committed
chore: pr feedback
1 parent a2635d2 commit 314d7d5

File tree

5 files changed

+48
-29
lines changed

5 files changed

+48
-29
lines changed

NativeScript/runtime/HMRSupport.mm

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -273,6 +273,7 @@ bool HttpFetchText(const std::string& url, std::string& out, std::string& conten
273273
NSURLSessionConfiguration* cfg = [NSURLSessionConfiguration defaultSessionConfiguration];
274274
cfg.HTTPAdditionalHeaders = @{ @"Accept": @"application/javascript, text/javascript, */*;q=0.1",
275275
@"Accept-Encoding": @"identity" };
276+
// Note: this could be made configurable if needed
276277
cfg.timeoutIntervalForRequest = 5.0;
277278
cfg.timeoutIntervalForResource = 5.0;
278279
NSURLSession* session = [NSURLSession sessionWithConfiguration:cfg];

NativeScript/runtime/ModuleInternalCallbacks.mm

Lines changed: 29 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -421,6 +421,11 @@ void UpdateModuleFallback(v8::Isolate* isolate, const std::string& canonicalPath
421421
static thread_local std::unordered_map<std::string, std::string> g_modulePrimaryImporters;
422422
static thread_local std::unordered_set<std::string> g_modulesInFlight;
423423
static thread_local std::unordered_set<std::string> g_modulesPendingReset;
424+
// The threshold for detecting circular dependencies during module resolution.
425+
// 256 was chosen as a high enough value to allow deep but legitimate module graphs,
426+
// but low enough to catch runaway recursion or infinite circular imports.
427+
// If a module is re-entered more than this limit, module loading is aborted and
428+
// an error is reported to prevent stack overflow or infinite loops.
424429
static constexpr size_t kMaxModuleReentryCount = 256;
425430
// Waiters: module path -> list of Promise resolvers waiting for completion (instantiated/evaluated or errored)
426431
static std::unordered_map<std::string, std::vector<v8::Global<v8::Promise::Resolver>>> g_moduleWaiters;
@@ -680,40 +685,44 @@ static bool IsDocumentsPath(const std::string& path) {
680685

681686
// 1) Turn the specifier literal into a std::string:
682687
v8::String::Utf8Value specUtf8(isolate, specifier);
683-
std::string spec = *specUtf8 ? *specUtf8 : "";
684-
if (spec.empty()) {
688+
const std::string rawSpec = *specUtf8 ? *specUtf8 : "";
689+
if (rawSpec.empty()) {
685690
return v8::MaybeLocal<v8::Module>();
686691
}
687692

693+
std::string normalizedSpec = rawSpec;
694+
688695
// Normalize malformed HTTP(S) schemes that sometimes appear as 'http:/host' (single slash)
689696
// due to upstream path joins or standardization. This ensures our HTTP loader fast-path
690697
// is used and avoids filesystem fallback attempts like '/app/http:/host'.
691-
if (spec.rfind("http:/", 0) == 0 && spec.rfind("http://", 0) != 0) {
692-
spec.insert(5, "/"); // http:/ -> http://
693-
} else if (spec.rfind("https:/", 0) == 0 && spec.rfind("https://", 0) != 0) {
694-
spec.insert(6, "/"); // https:/ -> https://
698+
if (normalizedSpec.rfind("http:/", 0) == 0 && normalizedSpec.rfind("http://", 0) != 0) {
699+
normalizedSpec.insert(5, "/"); // http:/ -> http://
700+
} else if (normalizedSpec.rfind("https:/", 0) == 0 && normalizedSpec.rfind("https://", 0) != 0) {
701+
normalizedSpec.insert(6, "/"); // https:/ -> https://
695702
}
696703

697704
if (IsScriptLoadingLogEnabled()) {
698-
Log(@"[resolver][spec] %s", spec.c_str());
705+
Log(@"[resolver][spec] %s", normalizedSpec.c_str());
699706
}
700707

701708
// Normalize '@/' alias to '/src/' for static imports (mirrors client dynamic import normalization)
702-
if (spec.rfind("@/", 0) == 0) {
703-
std::string orig = spec;
704-
spec = std::string("/src/") + spec.substr(2);
709+
if (normalizedSpec.rfind("@/", 0) == 0) {
710+
std::string orig = normalizedSpec;
711+
normalizedSpec = std::string("/src/") + normalizedSpec.substr(2);
705712
if (IsScriptLoadingLogEnabled()) {
706-
Log(@"[resolver][normalize] %@ -> %@", [NSString stringWithUTF8String:orig.c_str()], [NSString stringWithUTF8String:spec.c_str()]);
713+
Log(@"[resolver][normalize] %@ -> %@", [NSString stringWithUTF8String:orig.c_str()], [NSString stringWithUTF8String:normalizedSpec.c_str()]);
707714
}
708715
}
709716
// Guard against a bare '@' spec showing up (invalid); return empty to avoid poisoning registry with '@'
710-
if (spec == "@") {
717+
if (normalizedSpec == "@") {
711718
if (IsScriptLoadingLogEnabled()) {
712719
Log(@"[resolver][normalize] ignoring invalid '@' static spec");
713720
}
714721
return v8::MaybeLocal<v8::Module>();
715722
}
716723

724+
const std::string& spec = normalizedSpec; // use normalized spec for the rest of the resolution logic
725+
717726
// ── Early absolute-HTTP fast path ─────────────────────────────
718727
// If the specifier itself is an absolute HTTP(S) URL, resolve it immediately via
719728
// the HTTP dev loader and return before any filesystem candidate logic runs.
@@ -1199,8 +1208,13 @@ static bool IsDocumentsPath(const std::string& path) {
11991208
if (qpos != std::string::npos) baseNoQuery = baseNoQuery.substr(0, qpos);
12001209
// Strip a terminal .ts/.js when constructing mirror .mjs candidate
12011210
std::string noExt = baseNoQuery;
1202-
if (EndsWith(noExt, ".ts") || EndsWith(noExt, ".js")) {
1203-
noExt = noExt.substr(0, noExt.size() - 3);
1211+
// Handle variable-length extensions: .ts, .js, .tsx, .jsx, .mts, .cts
1212+
const std::vector<std::string> knownExts = {".ts", ".js", ".tsx", ".jsx", ".mts", ".cts"};
1213+
for (const auto& ext : knownExts) {
1214+
if (EndsWith(noExt, ext)) {
1215+
noExt = noExt.substr(0, noExt.size() - ext.size());
1216+
break;
1217+
}
12041218
}
12051219
// Use cached Documents directory (generic dynamic fetch mirror fallback)
12061220
const std::string& docsRootBase = GetDocumentsDirectory();
@@ -1498,7 +1512,7 @@ static bool IsDocumentsPath(const std::string& path) {
14981512
if (parentKey == primaryImporter) {
14991513
parentAlreadyRecorded = false; // Owner re-entry is expected during evaluation.
15001514
} else {
1501-
// NEW: gating block—only applied for dynamic Documents modules when unfinished.
1515+
// gating block—only applied for dynamic Documents modules when unfinished.
15021516
if (!gatingDisabled && unfinished) {
15031517
g_hmrModuleGatedCount.fetch_add(1, std::memory_order_relaxed);
15041518
if (IsScriptLoadingLogEnabled()) {

NativeScript/runtime/Runtime.mm

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,9 +30,7 @@
3030
#include "URLSearchParamsImpl.h"
3131
#include <mutex>
3232
#include "HMRSupport.h"
33-
#include "HMRSupport.mm"
3433
#include "DevFlags.h"
35-
#include "DevFlags.mm"
3634

3735
#define STRINGIZE(x) #x
3836
#define STRINGIZE_VALUE_OF(x) STRINGIZE(x)

TestRunner/app/tests/HttpEsmLoaderTests.js

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,8 @@ describe("HTTP ESM Loader", function() {
126126

127127
it("should handle network timeouts", function(done) {
128128
// Attempt to import from an unreachable address to test timeout
129+
// 192.0.2.1 is a TEST-NET-1 address reserved by RFC 5737 for documentation and testing purposes.
130+
// It is intentionally used here to trigger a network timeout scenario.
129131
import("http://192.0.2.1:5173/timeout-test.js").then(function(module) {
130132
fail("Should not have succeeded for unreachable server");
131133
done();
@@ -185,16 +187,4 @@ describe("HTTP ESM Loader", function() {
185187
});
186188
});
187189

188-
// Helper function to create a test dev server module (if dev server is running)
189-
function createTestDevModule() {
190-
return `
191-
// Test ES module for HTTP ESM loader
192-
export const testValue = "http-esm-loaded";
193-
export default function testFunction() {
194-
return "HTTP ESM loader working";
195-
}
196-
export { testFunction as namedExport };
197-
`;
198-
}
199-
200190
console.log("HTTP ESM Loader tests loaded");

v8ios.xcodeproj/project.pbxproj

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,10 @@
4949
9160C065291ED41F000641C0 /* SpinLock.h in Headers */ = {isa = PBXBuildFile; fileRef = 9160C064291ED41F000641C0 /* SpinLock.h */; };
5050
91B25A0A29DAC83D00E3CE04 /* ns-v8-tracing-agent-impl.mm in Sources */ = {isa = PBXBuildFile; fileRef = 91B25A0829DAC83D00E3CE04 /* ns-v8-tracing-agent-impl.mm */; };
5151
91B25A0B29DAC83D00E3CE04 /* ns-v8-tracing-agent-impl.h in Headers */ = {isa = PBXBuildFile; fileRef = 91B25A0929DAC83D00E3CE04 /* ns-v8-tracing-agent-impl.h */; };
52+
AA5DBFD32EC191F8008D12F9 /* HMRSupport.mm in Sources */ = {isa = PBXBuildFile; fileRef = AA5DBFD22EC191F8008D12F9 /* HMRSupport.mm */; };
53+
AA5DBFD42EC191F8008D12F9 /* HMRSupport.h in Headers */ = {isa = PBXBuildFile; fileRef = AA5DBFD12EC191F8008D12F9 /* HMRSupport.h */; };
54+
AA5DBFD72EC19216008D12F9 /* DevFlags.h in Headers */ = {isa = PBXBuildFile; fileRef = AA5DBFD52EC19216008D12F9 /* DevFlags.h */; };
55+
AA5DBFD82EC19216008D12F9 /* DevFlags.mm in Sources */ = {isa = PBXBuildFile; fileRef = AA5DBFD62EC19216008D12F9 /* DevFlags.mm */; };
5256
AA8C47B12E27114300649BF5 /* ModuleInternalCallbacks.h in Headers */ = {isa = PBXBuildFile; fileRef = AA8C47AF2E27114300649BF5 /* ModuleInternalCallbacks.h */; };
5357
AA8C47B22E27114300649BF5 /* ModuleInternalCallbacks.mm in Sources */ = {isa = PBXBuildFile; fileRef = AA8C47B02E27114300649BF5 /* ModuleInternalCallbacks.mm */; };
5458
C205257F2577D6F900C12A5C /* NativeScript.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = C2DDEB32229EAB3B00345BFE /* NativeScript.framework */; };
@@ -486,6 +490,10 @@
486490
9160C064291ED41F000641C0 /* SpinLock.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; name = SpinLock.h; path = NativeScript/runtime/SpinLock.h; sourceTree = "<group>"; };
487491
91B25A0829DAC83D00E3CE04 /* ns-v8-tracing-agent-impl.mm */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.objcpp; path = "ns-v8-tracing-agent-impl.mm"; sourceTree = "<group>"; };
488492
91B25A0929DAC83D00E3CE04 /* ns-v8-tracing-agent-impl.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = "ns-v8-tracing-agent-impl.h"; sourceTree = "<group>"; };
493+
AA5DBFD12EC191F8008D12F9 /* HMRSupport.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = HMRSupport.h; sourceTree = "<group>"; };
494+
AA5DBFD22EC191F8008D12F9 /* HMRSupport.mm */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.objcpp; path = HMRSupport.mm; sourceTree = "<group>"; };
495+
AA5DBFD52EC19216008D12F9 /* DevFlags.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = DevFlags.h; sourceTree = "<group>"; };
496+
AA5DBFD62EC19216008D12F9 /* DevFlags.mm */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.objcpp; path = DevFlags.mm; sourceTree = "<group>"; };
489497
AA8C47AF2E27114300649BF5 /* ModuleInternalCallbacks.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = ModuleInternalCallbacks.h; sourceTree = "<group>"; };
490498
AA8C47B02E27114300649BF5 /* ModuleInternalCallbacks.mm */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.objcpp; path = ModuleInternalCallbacks.mm; sourceTree = "<group>"; };
491499
C2003F9E23FA78CD0043B815 /* TNSDerivedClass.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = TNSDerivedClass.h; sourceTree = "<group>"; };
@@ -1409,6 +1417,8 @@
14091417
C2DDEB79229EAC8200345BFE /* Console.h */,
14101418
C2DDEB6B229EAC8200345BFE /* Console.cpp */,
14111419
C2DDEB72229EAC8200345BFE /* DataWrapper.h */,
1420+
AA5DBFD52EC19216008D12F9 /* DevFlags.h */,
1421+
AA5DBFD62EC19216008D12F9 /* DevFlags.mm */,
14121422
C2DDEB85229EAC8300345BFE /* DictionaryAdapter.h */,
14131423
C2DDEB8B229EAC8300345BFE /* DictionaryAdapter.mm */,
14141424
C2A5F8692359AEB600074AFA /* ExtVector.h */,
@@ -1421,6 +1431,8 @@
14211431
C266569C22B282BA00EE15CC /* FunctionReference.cpp */,
14221432
C2DDEB6D229EAC8200345BFE /* Helpers.h */,
14231433
C2DDEB7D229EAC8200345BFE /* Helpers.mm */,
1434+
AA5DBFD12EC191F8008D12F9 /* HMRSupport.h */,
1435+
AA5DBFD22EC191F8008D12F9 /* HMRSupport.mm */,
14241436
C2FEA16E22A3C75C00A5C0FC /* InlineFunctions.h */,
14251437
C2FEA16D22A3C75C00A5C0FC /* InlineFunctions.cpp */,
14261438
C2DDEB65229EAC8100345BFE /* Interop.h */,
@@ -1622,6 +1634,7 @@
16221634
C2D71D7623E18C41000CD5F0 /* robin_hood.h in Headers */,
16231635
C2F4D0CB2334A6E70008A2EB /* RuntimeConfig.h in Headers */,
16241636
C247C16B22F82842001D2CA2 /* v8.h in Headers */,
1637+
AA5DBFD72EC19216008D12F9 /* DevFlags.h in Headers */,
16251638
C22536B6241A318900192740 /* ffi.h in Headers */,
16261639
C2DDEB8F229EAC8300345BFE /* ArgConverter.h in Headers */,
16271640
3CBFF7442971C1C200C5DE36 /* ArcMacro.h in Headers */,
@@ -1634,6 +1647,7 @@
16341647
AA8C47B12E27114300649BF5 /* ModuleInternalCallbacks.h in Headers */,
16351648
C2DDEBA7229EAC8300345BFE /* ClassBuilder.h in Headers */,
16361649
6573B9ED291FE5B700B0ED7C /* JSIRuntime.h in Headers */,
1650+
AA5DBFD42EC191F8008D12F9 /* HMRSupport.h in Headers */,
16371651
C2DDEB98229EAC8300345BFE /* ArrayAdapter.h in Headers */,
16381652
C2DDEB9B229EAC8300345BFE /* DataWrapper.h in Headers */,
16391653
C2DDEBB2229EAC8300345BFE /* ModuleInternal.h in Headers */,
@@ -2197,6 +2211,7 @@
21972211
C2A5F86A2359AEB600074AFA /* ExtVector.cpp in Sources */,
21982212
AA8C47B22E27114300649BF5 /* ModuleInternalCallbacks.mm in Sources */,
21992213
C2DDEBAF229EAC8300345BFE /* ObjectManager.mm in Sources */,
2214+
AA5DBFD82EC19216008D12F9 /* DevFlags.mm in Sources */,
22002215
F6191AB029C0FCE8003F588F /* JsV8InspectorClient.mm in Sources */,
22012216
C2DDEB9C229EAC8300345BFE /* ClassBuilder.cpp in Sources */,
22022217
C266569322AFFF7E00EE15CC /* Pointer.cpp in Sources */,
@@ -2231,6 +2246,7 @@
22312246
C26656B322B3768C00EE15CC /* InteropTypes.mm in Sources */,
22322247
3C78BA5C2A0D600100C20A88 /* ModuleBinding.cpp in Sources */,
22332248
C2F4D0AD232F85E20008A2EB /* SymbolIterator.mm in Sources */,
2249+
AA5DBFD32EC191F8008D12F9 /* HMRSupport.mm in Sources */,
22342250
C2DDEBA5229EAC8300345BFE /* Runtime.mm in Sources */,
22352251
6573B9CF291FE29F00B0ED7C /* V8Runtime.cpp in Sources */,
22362252
6573B9E3291FE2A700B0ED7C /* jsilib-posix.cpp in Sources */,

0 commit comments

Comments
 (0)