fix: anchor relative dynamic imports at the file:// referrer's directory

[adrian-niculescu]

A dynamic import() with a relative specifier (./x, ../x) from a module loaded over file:// resolves against the application root instead of the importing module's own directory. A module that lives in a subdirectory then fails to resolve its relative dependencies.

ImportModuleDynamicallyCallback reads the referrer URL but only uses it when the referrer is http(s). For file:// referrers it passes an empty referrer to ResolveModuleCallback, which falls back to anchoring relative specifiers at the app root. Static imports are unaffected because V8 passes the real parent module.

This anchors the relative specifier at the referrer's directory, taken from resource_name, and passes the resolver an absolute file:// URL. It is a no-op when the importing module already sits at the app root. The iOS runtime already resolves this case the same way.

Test: added test-app ES module specs for a relative ./ import from a subdirectory, a ../ import from a nested directory, and an app-root control. The first two fail before the change and pass after.

Summary by CodeRabbit

Release Notes

• New Features

  • Enhanced dynamic module import resolution with improved support for relative paths across different directory structures, including parent-sibling and nested module scenarios.

• Tests

  • Added comprehensive test coverage for dynamic ES module imports to verify correct path resolution in various directory contexts.

[coderabbitai]

📝 Walkthrough

Walkthrough

Adds relative dynamic import() resolution for file:// modules in the Android runtime. A new NormalizeDotSegments() path helper and ResolveFileRelative() function are added to ModuleInternalCallbacks.cpp, and ImportModuleDynamicallyCallback is updated to resolve relative specifiers to absolute file:// URLs before delegating to ResolveModuleCallback. Three new .mjs fixture files and matching Jasmine tests validate same-directory, parent-directory, and app-root relative import scenarios.

Changes

Relative Dynamic Import Resolution

Layer / File(s)	Summary
NormalizeDotSegments, ResolveFileRelative, and ImportModuleDynamicallyCallback wiring test-app/runtime/src/main/cpp/ModuleInternalCallbacks.cpp	Adds NormalizeDotSegments() to collapse . and .. path segments; refactors ResolveHttpRelative to use it; introduces ResolveFileRelative() to resolve ./ and ../ specifiers against file:// referrer URLs; changes ImportModuleDynamicallyCallback to call ResolveFileRelative() and pass the absolute specifier to ResolveModuleCallback for relative imports.
ESM test fixtures and Jasmine relative-import tests test-app/app/src/main/assets/app/esm-subdir/sibling.mjs, test-app/app/src/main/assets/app/esm-subdir/parent.mjs, test-app/app/src/main/assets/app/esm-subdir/nested/child.mjs, test-app/app/src/main/assets/app/testRelativeDynamicImport.mjs, test-app/app/src/main/assets/app/tests/testESModules.mjs	Adds sibling.mjs exporting "sibling-loaded", parent.mjs with loadSibling() using ./sibling.mjs, child.mjs with loadParentSibling() using ../sibling.mjs, testRelativeDynamicImport.mjs with loadRootSibling() using ./testSimpleESModule.mjs, and three new Jasmine tests verifying all three relative resolution paths.

Sequence Diagram(s)

sequenceDiagram
  participant V8 as V8 Engine
  participant IMDC as ImportModuleDynamicallyCallback
  participant RFR as ResolveFileRelative
  participant NDS as NormalizeDotSegments
  participant RMC as ResolveModuleCallback

  V8->>IMDC: dynamic import("./sibling.mjs", referrer=file:///app/esm-subdir/parent.mjs)
  IMDC->>RFR: ResolveFileRelative(referrer, "./sibling.mjs")
  RFR->>RFR: strip fragment/query → base dir = /app/esm-subdir/
  RFR->>NDS: NormalizeDotSegments("/app/esm-subdir/sibling.mjs")
  NDS-->>RFR: "/app/esm-subdir/sibling.mjs"
  RFR-->>IMDC: "file:///app/esm-subdir/sibling.mjs"
  IMDC->>RMC: ResolveModuleCallback("file:///app/esm-subdir/sibling.mjs", empty referrer)
  RMC-->>V8: resolved sibling module

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐇 A dot-dot here, a slash right there,
The path was tangled beyond repair!
Now NormalizeDotSegments leads the way,
ResolveFileRelative saves the day.
Siblings found from parent and child—
Relative imports, finally reconciled! 🌿

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main fix: anchoring relative dynamic imports at the file:// referrer's directory, which is the core change across the ModuleInternalCallbacks.cpp file and test additions.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

🧹 Nitpick comments (1)

test-app/app/src/main/assets/app/tests/testESModules.mjs (1)

38-44: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Prefer terminal done.fail to avoid timeout-style failures in async specs.

A thrown assertion in these callbacks can skip done() and turn a direct assertion failure into a spec timeout. Using a terminal rejection handler gives cleaner failures.

Proposed pattern

   it("resolves a relative dynamic import from a subdirectory module", (done) => {
     import("~/esm-subdir/parent.mjs")
       .then((parent) => parent.loadSibling())
-      .then(
-        (value) => { expect(value).toBe("sibling-loaded"); done(); },
-        (err) => { expect(err).toBeUndefined(); done(); }
-      );
+      .then((value) => {
+        expect(value).toBe("sibling-loaded");
+      })
+      .then(() => done(), done.fail);
   });

Apply the same shape to the other two specs in this block.

Also applies to: 47-53, 56-62

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test-app/app/src/main/assets/app/tests/testESModules.mjs` around lines 38 -
44, The test "resolves a relative dynamic import from a subdirectory module" and
the two similar specs in this block use explicit error callbacks that can cause
timeout-style failures when assertions fail. Replace the second argument (the
rejection handler) in each of the three promise chain `.then()` calls with
`done.fail`, which properly handles assertion failures and provides cleaner test
failure messages. This pattern change should be applied to all three specs: the
one starting with "resolves a relative dynamic import from a subdirectory
module" and the two other specs mentioned in the comment.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@test-app/app/src/main/assets/app/tests/testESModules.mjs`:
- Around line 38-44: The test "resolves a relative dynamic import from a
subdirectory module" and the two similar specs in this block use explicit error
callbacks that can cause timeout-style failures when assertions fail. Replace
the second argument (the rejection handler) in each of the three promise chain
`.then()` calls with `done.fail`, which properly handles assertion failures and
provides cleaner test failure messages. This pattern change should be applied to
all three specs: the one starting with "resolves a relative dynamic import from
a subdirectory module" and the two other specs mentioned in the comment.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 6819f21a-2871-43cd-8d2b-94e764298eeb

📥 Commits

Reviewing files that changed from the base of the PR and between 4b5ab0a and 67442e2.

📒 Files selected for processing (6)

• test-app/app/src/main/assets/app/esm-subdir/nested/child.mjs
• test-app/app/src/main/assets/app/esm-subdir/parent.mjs
• test-app/app/src/main/assets/app/esm-subdir/sibling.mjs
• test-app/app/src/main/assets/app/testRelativeDynamicImport.mjs
• test-app/app/src/main/assets/app/tests/testESModules.mjs
• test-app/runtime/src/main/cpp/ModuleInternalCallbacks.cpp

[adrian-niculescu]

@coderabbitai, the test-app pins Jasmine 2.0.1, which has no done.fail: the async callback is a plain function with no .fail property, so done.fail is undefined and .then(() => done(), done.fail) would pass an undefined rejection handler, leaving a rejection to surface as the timeout it is meant to avoid.

This version's expectations also do not throw on failure (addExpectationResult records the result, it does not throw and there is no throwOnExpectationFailure), so the explicit (err) => { expect(err).toBeUndefined(); done(); } handler always reaches done(). Keeping the current form.

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

@adrian-niculescu: Thanks for the detailed explanation — that's a clear constraint from the pinned Jasmine 2.0.1 version. The current form is correct given that done.fail is absent and expectations don't throw in that version. I'm withdrawing my earlier concern.

---

✏️ Learnings added

Learnt from: adrian-niculescu
URL: https://github.com/NativeScript/android/pull/1976

Timestamp: 2026-06-23T22:07:53.313Z
Learning: The NativeScript Android test-app (NativeScript/android) pins Jasmine 2.0.1. In this version: (1) `done` is a plain function with no `.fail` property, so `done.fail` is `undefined` — using `.then(() => done(), done.fail)` silently passes `undefined` as the rejection handler and unhandled rejections surface as spec timeouts instead of failures. (2) `addExpectationResult` records failures without throwing and there is no `throwOnExpectationFailure`, so an explicit rejection handler `(err) => { expect(err).toBeUndefined(); done(); }` always reaches `done()`. The correct async test pattern for this version is the explicit two-handler form rather than `done.fail`.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

@adrian-niculescu: Understood — the concern was already withdrawn and the Jasmine 2.0.1 constraints have been noted in the knowledge base. No further action needed here; the current test form is the right one for the pinned version.

[NathanWalker]

Nice fix, ios may need similar @adrian-niculescu ?

[adrian-niculescu]

@NathanWalker, thanks and thank you for the quick review! iOS already handles this. In ImportModuleDynamicallyCallback, vanilla iOS anchors a relative .//../ specifier on the referrer's resource URL (referrer->GetResourceName()), derives the base directory, joins, and collapses the dot-segments:

https://github.com/NativeScript/ios/blob/6778dfdd5b436be0dad51f1f64df1e1408d8e76c/NativeScript/runtime/ModuleInternalCallbacks.mm#L2363-L2401

then passes the resulting absolute path to ResolveModuleCallback, so a file:// referrer's relative specifier never falls back to the app root:

https://github.com/NativeScript/ios/blob/6778dfdd5b436be0dad51f1f64df1e1408d8e76c/NativeScript/runtime/ModuleInternalCallbacks.mm#L2411-L2413

This Android change brings the file:// path to parity. Static imports were already fine on both platforms.