Skip to content

feat: ns:util builtin module (inspect, format) - #418

Merged
NathanWalker merged 2 commits into
mainfrom
feat/ns-util
Jul 31, 2026
Merged

feat: ns:util builtin module (inspect, format)#418
NathanWalker merged 2 commits into
mainfrom
feat/ns-util

Conversation

@edusperoni

@edusperoni edusperoni commented Jul 30, 2026

Copy link
Copy Markdown
Collaborator

Stacked on #416 (inspect). Review only the last commit; the base branch is feat/inspect.

What

Runtime-provided modules get a resolution scheme. require("ns:util"), import util, { inspect } from "ns:util" and await import("ns:util") all resolve to the same per-realm module, and console.* gains Node's %-substitution.

v1 exports:

export
inspect(value[, options]) the console formatter from #416, now reachable from app code
format(fmt, ...args) Node's util.format: %s %d %i %f %j %o %O %%, extras appended space-separated

docs/ns-builtin-modules.md (added here) is the cross-runtime contract — the same document the Android runtime implements against, so a capability behaves identically on both platforms before it ships in a stable release.

Resolution rules

  • ns:/node: specifiers are intercepted before any filesystem or npm resolution, in all three entry points: the CommonJS require path (ModuleInternal.mm:219), the ES module resolve callback (ModuleInternalCallbacks.mm:719) and the dynamic-import host callback (ModuleInternalCallbacks.mm:1878). A builtin can never be shadowed by a file or a package.
  • ESM is served by v8::Module::CreateSyntheticModule, exporting every member by name plus default (the exports object). The module is instantiated, evaluated and cached per realm, so repeated imports return the identical namespace.
  • Unknown names in either scheme fail with exactly No such built-in module: <specifier> — synchronously for require, as a rejection for import().
  • Builtins are singletons per realm: the main context and every worker build their own exports object (the caches live in Caches, not in a process-global).
  • Exports are frozen.

node: shims

The same registry serves node: (spec: "node: compatibility shims"), so npm packages requiring Node builtins by their prefixed name can run where a shim exists.

  • One source file per specifier, one registry entry. node:util is its own builtin (node-util.js) that consumes ns:util through the internal require and re-exports its members. The shim owns all the Node adaptation — argument shapes, option names, aliases — so ns-util.js contains no compatibility code and does not know a shim exists. This is the reference pattern every future ns:* module and shim copies.
  • Shims are lazy: node-util.js is only compiled the first time node:util is resolved, so an app that never touches the node: scheme never pays for it.
  • node:util is a distinct, separately frozen module object from ns:util, even though every member is re-exported unchanged (nodeUtil.inspect === nsUtil.inspect) — per the spec bullet, mirroring how Bun (bun:*), Deno and Cloudflare (cloudflare:*) keep their own surface apart from their Node compat layer. ns:util can grow runtime-specific exports without widening the Node compatibility surface.
  • Missing members are simply absent (typeof util.promisify === "undefined"), never present-but-throwing, so feature checks work.
  • Bare specifiers are completely untouched: require("util") still resolves through npm exactly as before (many apps bundle the util polyfill package). Only the explicit prefix reaches the registry. There is a test asserting require("util") does not return the builtin.

⚠️ Behavior change: unshimmed node: ES imports previously got a generic in-memory polyfill (console.warn(...); export default {}) that broke at first use; they now fail with No such built-in module: node:<name>. The pre-existing node:url polyfill (fileURLToPath/pathToFileURL) is kept as-is and is still import-only; it is noted in the doc's non-normative iOS section.

The internal require (new wrapper parameter)

The builtin function wrapper becomes (exports, require, module, binding, primordials) — Node's order. require is created once per realm by BuiltinLoader and resolves builtin specifiers only: no path, no package, no filesystem. An unregistered name throws exactly No such built-in module: <specifier>, and requiring a module that is still loading throws instead of recursing (per-realm in-progress guard). It is documented as normative in the spec, since Android needs it to build shims the same way. C++ call sites are unchanged; builtins that don't need it simply ignore the parameter.

console integration

Console::BuildStringFromArgs calls the cached format with the full argument list, so console.log("%d apples", 3) prints 3 apples while console.log("100%"), unknown specifiers and a dangling % stay verbatim. Non-format calls keep joining with spaces exactly as before. If the builtin is unavailable the old per-argument loop still runs, so logging can never be taken down by the formatter.

Implementation

  • NativeScript/runtime/js/ns-util.js — the ns:util builtin, all intrinsic access through primordials (Number, NumberParseInt, NumberParseFloat, StringPrototypeCharCodeAt added, ESLint lists kept in sync). inspect is passed in via binding, so the module and console share one formatter instance.
  • NativeScript/runtime/js/node-util.js — the shim, four lines of re-export plus the rule about where adaptation code lives.
  • NativeScript/runtime/NsBuiltinModules.{h,cpp} — the registry (one specifier → one BuiltinId), lazy per-realm materialization with a cycle guard, and the synthetic-module evaluation steps. Adding a module is one row plus one file.

Stability caveat

Verbatim from Node's contract, and repeated in the doc: inspect's output (and therefore format's object rendering) may change between runtime versions for readability. It is for humans and must not be parsed programmatically.

Tests

TestRunner/app/tests/NsUtilTests.js (+ an .mjs fixture that statically imports both specifiers, so the resolve callback is covered and not just the dynamic-import fast path): frozen exports, singleton identity, every format specifier, %%/unknown/dangling %, %j on a circular value, extras, non-string first argument, console.log("%d apples", 3), dynamic + static import identity, node:util distinctness with shared members, unknown ns:/node: messages, and the bare-specifier non-change.

Full suite on the simulator: 941 tests, 0 failures (922 baseline + 19 new). format was additionally verified against Node's own util.format on the host for every non-object case.

Summary by CodeRabbit

  • New Features

    • Added built-in ns:util and node:util modules with inspection and formatting utilities.
    • Added support for static imports, dynamic imports, and require for registered built-in modules.
    • Added consistent errors for unknown or unsupported built-in modules.
    • Added formatting support for console output, including circular and specialized values.
  • Documentation

    • Documented built-in module behavior, compatibility rules, and usage examples.
  • Tests

    • Added comprehensive coverage for imports, formatting, caching, errors, and interoperability.

@coderabbitai

coderabbitai Bot commented Jul 30, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The runtime adds registered ns: and node: builtin modules, builtin-only require, synthetic ESM resolution, cached per-isolate state, and ns:util console formatting. Tests and documentation cover CommonJS, static and dynamic ESM imports, caching, errors, and formatting behavior.

Changes

Builtin module runtime

Layer / File(s) Summary
Builtin wrapper and cache contract
NativeScript/runtime/BuiltinLoader.*, NativeScript/runtime/Caches.h, NativeScript/runtime/NsBuiltinModules.h
Builtins receive a cached builtin-only require function. Per-isolate caches store builtin exports, synthetic modules, in-progress loads, and formatting functions.
Builtin registry and module materialization
NativeScript/runtime/NsBuiltinModules.cpp, NativeScript/runtime/js/ns-util.js, NativeScript/runtime/js/node-util.js, NativeScript/runtime/js/primordials.js, tools/*, v8ios.xcodeproj/project.pbxproj
The runtime registers ns:util and node:util, creates frozen exports, detects cycles, caches modules, and bundles the new JavaScript builtins.
Resolver and console integration
NativeScript/runtime/ModuleInternal.mm, NativeScript/runtime/ModuleInternalCallbacks.mm, NativeScript/runtime/Console.*
CommonJS, static ESM, dynamic ESM, and console formatting resolve registered builtins and report standardized errors.
Runtime contract and validation
NativeScript/runtime/js/README.md, docs/ns-builtin-modules.md, TestRunner/app/tests/*, eslint.config.mjs
Documentation, lint settings, and tests describe and validate builtin loading, formatting, caching, imports, shims, and failure cases.

Estimated code review effort: 4 (Complex) | ~60 minutes

Sequence Diagram(s)

sequenceDiagram
  participant BuiltinRequire
  participant RequireCallback
  participant NsBuiltinModules
  participant SyntheticModule
  BuiltinRequire->>RequireCallback: resolve builtin specifier
  RequireCallback->>NsBuiltinModules: GetExports(specifier)
  NsBuiltinModules->>SyntheticModule: create or retrieve builtin module
  SyntheticModule-->>NsBuiltinModules: module exports
  NsBuiltinModules-->>RequireCallback: cached exports
  RequireCallback-->>BuiltinRequire: return exports or throw not-found error
Loading

Possibly related PRs

  • NativeScript/ios#411: Extends the earlier builtin-loading infrastructure with builtin-only require support.
  • NativeScript/ios#415: Shares changes to the builtin wrapper contract, caches, runtime JavaScript, and primordials.
  • NativeScript/ios#375: Also modifies module-loading and error paths in ModuleInternal.mm and ModuleInternalCallbacks.mm.

Suggested reviewers: nathanwalker

Poem

A rabbit loads ns:util with care,
Caches its exports in a realm-safe lair.
node:util follows, frozen and bright,
Formatting hops through the console at night.
Cycles get errors, unknowns get none—
Builtin modules now safely run!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 28.57% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the primary change: adding the ns:util builtin module with inspect and format support.
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.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch

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

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

Base automatically changed from feat/inspect to main July 31, 2026 05:36
Runtime-provided modules now resolve under the URL-style `ns:` scheme,
ahead of any filesystem or npm resolution, with `node:` compatibility
shims served from the same registry. v1 ships `ns:util` (inspect, format)
and a `node:util` shim re-exporting its members from a distinct, frozen
module object.

Each specifier is one source file and one registry entry: a shim is its
own builtin that reaches the `ns:` module it adapts through a new fixed
wrapper parameter, `require`, which resolves builtin specifiers and
nothing else. Shims therefore compile only when first resolved and own
every bit of Node adaptation, keeping compatibility knowledge out of the
standard modules.

format() is Node's util.format (%s %d %i %f %j %o %O %%, extras appended
space-separated), and console.* routes its arguments through it, so
`console.log("%d apples", 3)` works while unknown and dangling percent
signs stay verbatim.

Resolution is wired into the CommonJS require path, the ES module resolve
callback and the dynamic-import callback; ESM consumption is served by a
per-realm synthetic module. An unknown name in either scheme fails with
`No such built-in module: <specifier>`, which replaces the warn-and-export-
nothing polyfill previously handed to unshimmed `node:` imports. Bare
specifiers are untouched.

docs/ns-builtin-modules.md is the cross-runtime contract both runtimes
implement.
…s inputs

Builtins stay classic function bodies on both runtimes; cross-builtin
sharing, if ever needed, starts with generation-time bundling. The js2c
guard turns an accidental .mjs into a build error pointing at the spec.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
NativeScript/runtime/Caches.h (1)

158-186: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Reset the new Persistent caches in ~Caches.

Caches::~Caches() currently only clears the preexisting prototype/cache maps and bound objects. The new entries in BuiltinModuleExports and BuiltinModules also hold v8::Persistents, and the single handles FormatFunc and BuiltinRequire need the same teardown. Add per-value Reset() cleanup for the maps and reset these single handles before isolate disposal. robin_hood::unordered_set is provided by the bundled robin_hood.h, so that part needs no change.

🤖 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 `@NativeScript/runtime/Caches.h` around lines 158 - 186, Update
Caches::~Caches() to reset every Persistent introduced by the new caches before
isolate disposal: iterate BuiltinModuleExports and BuiltinModules and call
Reset() on each stored handle, then reset the single handles FormatFunc and
BuiltinRequire. Preserve the existing cleanup for other caches and leave
BuiltinModulesInProgress unchanged.
🤖 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.

Inline comments:
In `@docs/ns-builtin-modules.md`:
- Around line 27-29: Update the builtin-module resolution documentation to
distinguish the APIs: state that require() throws synchronously for an unknown
builtin, while dynamic import() rejects its promise with the same Error message,
`No such built-in module: ns:<name>`. Preserve the exact message wording.

In `@NativeScript/runtime/ModuleInternal.mm`:
- Around line 218-236: Update the builtin-resolution predicate in
RequireCallback to match ESM behavior: use the new builtin path for registered
modules and ns:-prefixed specifiers, while allowing unregistered node:-prefixed
specifiers to continue through the legacy node:url fallback. Reuse the existing
ESM branch predicate or document and centralize the shared condition so CommonJS
and ESM resolve node: specifiers consistently.

In `@NativeScript/runtime/NsBuiltinModules.cpp`:
- Around line 272-301: Update the failure path in
NsBuiltinModules::GetFormatFunc so FormatFuncUnavailable is set only when
BuiltinModulesInProgress does not contain "ns:util"; preserve the existing error
logging and empty-function return, but avoid permanently caching failures caused
by re-entrant ns:util initialization.

In `@TestRunner/app/tests/NsUtilTests.js`:
- Around line 86-91: Update the “backs console.* formatting” test to spy on or
capture the supported console output sink instead of only asserting no throw.
Verify the first console.log call emits “3 apples” and the second emits “100%”
unchanged, using the test runner’s existing console-spy mechanism if available.

---

Outside diff comments:
In `@NativeScript/runtime/Caches.h`:
- Around line 158-186: Update Caches::~Caches() to reset every Persistent
introduced by the new caches before isolate disposal: iterate
BuiltinModuleExports and BuiltinModules and call Reset() on each stored handle,
then reset the single handles FormatFunc and BuiltinRequire. Preserve the
existing cleanup for other caches and leave BuiltinModulesInProgress unchanged.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: af47a8ca-636b-4ffb-8730-f207028b0a7c

📥 Commits

Reviewing files that changed from the base of the PR and between 5e40702 and dd7115d.

📒 Files selected for processing (21)
  • NativeScript/runtime/BuiltinLoader.cpp
  • NativeScript/runtime/BuiltinLoader.h
  • NativeScript/runtime/Caches.h
  • NativeScript/runtime/Console.cpp
  • NativeScript/runtime/Console.h
  • NativeScript/runtime/ModuleInternal.mm
  • NativeScript/runtime/ModuleInternalCallbacks.mm
  • NativeScript/runtime/NsBuiltinModules.cpp
  • NativeScript/runtime/NsBuiltinModules.h
  • NativeScript/runtime/js/README.md
  • NativeScript/runtime/js/node-util.js
  • NativeScript/runtime/js/ns-util.js
  • NativeScript/runtime/js/primordials.js
  • TestRunner/app/tests/NsUtilTests.js
  • TestRunner/app/tests/esm/ns-util-import.mjs
  • TestRunner/app/tests/index.js
  • docs/ns-builtin-modules.md
  • eslint.config.mjs
  • tools/js2c-inputs.xcfilelist
  • tools/js2c.mjs
  • v8ios.xcodeproj/project.pbxproj

Comment on lines +27 to +29
- Resolution of an unknown builtin fails synchronously with an `Error` whose
message is exactly `No such built-in module: ns:<name>` (matching Node's
wording for familiarity).

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Distinguish CommonJS errors from dynamic-import rejection.

The synchronous-error statement is not correct for import("ns:definitely-not-a-module"). Dynamic import returns a promise and reports this error by rejection, as TestRunner/app/tests/NsUtilTests.js lines 109-118 verify. State that require() throws synchronously and dynamic import() rejects with the same Error message.

🤖 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 `@docs/ns-builtin-modules.md` around lines 27 - 29, Update the builtin-module
resolution documentation to distinguish the APIs: state that require() throws
synchronously for an unknown builtin, while dynamic import() rejects its promise
with the same Error message, `No such built-in module: ns:<name>`. Preserve the
exact message wording.

Comment on lines +218 to +236
// Builtin modules resolve before any path handling, so they can never be
// shadowed by a file or a package, and an unknown one fails as a missing
// builtin rather than as a missing file. Only prefixed specifiers get here:
// a bare `util` still resolves through npm.
if (info.Length() > 0 && info[0]->IsString()) {
std::string specifier = tns::ToString(isolate, info[0].As<v8::String>());
if (NsBuiltinModules::IsBuiltinScheme(specifier)) {
Local<Context> context = isolate->GetCurrentContext();
Local<Object> exports;
if (NsBuiltinModules::GetExports(context, specifier).ToLocal(&exports)) {
info.GetReturnValue().Set(exports);
} else if (!NsBuiltinModules::IsRegistered(specifier)) {
isolate->ThrowException(Exception::Error(
tns::ToV8String(isolate, NsBuiltinModules::NotFoundMessage(specifier))));
}
return;
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: Inspect IsBuiltinScheme / IsNsScheme / IsRegistered definitions and the node:url legacy polyfill.
rg -n -B2 -A15 'bool.*IsBuiltinScheme|bool.*IsNsScheme|bool.*IsRegistered' NativeScript/runtime/NsBuiltinModules.h NativeScript/runtime/NsBuiltinModules.cpp 2>/dev/null
rg -n 'node:url|node:util' NativeScript/runtime/NsBuiltinModules.cpp docs/ns-builtin-modules.md 2>/dev/null

Repository: NativeScript/ios

Length of output: 4970


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== ModuleInternal RequireCallback context =="
sed -n '190,250p' NativeScript/runtime/ModuleInternal.mm 2>/dev/null || true

echo
echo "== ModuleInternalCallbacks relevant resolution callbacks =="
rg -n -B8 -A22 'ResolveModuleCallback|ImportModuleDynamicallyCallback|legacy|node:url|node:util|RequireCallback' NativeScript/runtime/ModuleInternalCallbacks.mm 2>/dev/null || true

echo
echo "== NsBuiltinModules constants and prefix check =="
rg -n -B4 -A6 'kNodePrefix|kNsPrefix|HasPrefix' NativeScript/runtime/NsBuiltinModules.cpp 2>/dev/null || true

echo
echo "== Deterministic predicate/probe from source text =="
python3 - <<'PY'
from pathlib import Path
import re

ns_cpp = Path('NativeScript/runtime/NsBuiltinModules.cpp').read_text(errors='replace')
m = re.search(r'bool NsBuiltinModules::IsBuiltinScheme\(const std::string& specifier\) \{\s*(.*?)\n\}', ns_cpp, re.S)
print('IsBuiltinScheme body:', m.group(0).strip() if m else 'NOT_FOUND')

m = re.search(r'bool NsBuiltinModules::IsNsScheme\(const std::string& specifier\) \{\s*(.*?)\n\}', ns_cpp, re.S)
print('IsNsScheme body:', m.group(0).strip() if m else 'NOT_FOUND')

m = re.search(r'bool.+kNodePrefix.+=(.+?);|kNodePrefix\s*=\s*([^;]+);', \
    '\n'.join(re.findall(r'k[a-zA-Z0-9_]+Prefix\s*=[^;]+;', ns_cpp)), re.S)
prefixes = re.findall(r'k[Nn]ode[Nn]Prefix(?:\s*=\s*\"([^\"]+)\"|:\s*\"([^\"]+)")', ns_cpp)
print('prefixes text:', prefixes)

def has_prefix(specifier, prefix):
    return specifier.startswith(prefix)
def predicate_new(specifier):
    # Derived from current IsBuiltinScheme body
    return has_prefix(specifier, 'ns:') or has_prefix(specifier, 'node:')
def predicate_esm_condition(specifier, registered):
    # Derived from IsRegistered(rawSpec) || IsNsScheme(rawSpec) branch
    return registered or has_prefix(specifier, 'ns:')

for specifier in ['node:url', 'node:util', 'ns:url', 'util', 'app/builtin']:
    registered = specifier in {'node:url', 'node:util'}  # registry lookup not provided; use documented presence
    print(specifier, 'new_predicate=', predicate_new(specifier),
          'esm_condition=', predicate_esm_condition(specifier, registered),
          'registered=', registered)
PY

Repository: NativeScript/ios

Length of output: 33628


Align RequireCallback builtin-prefixed resolution with ESM paths.

RequireCallback now treats every ns: or node: specifier as builtin and rejects unregistered keys as missing modules. ESM resolution only takes the new builtin path for registered nodes and ns:-prefixed specifiers, leaving unregistered node: specifiers to the legacy node:url fallback. Use or document the same branch predicate for both paths so CommonJS and ESM handle the same node: specifiers consistently.

🤖 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 `@NativeScript/runtime/ModuleInternal.mm` around lines 218 - 236, Update the
builtin-resolution predicate in RequireCallback to match ESM behavior: use the
new builtin path for registered modules and ns:-prefixed specifiers, while
allowing unregistered node:-prefixed specifiers to continue through the legacy
node:url fallback. Reuse the existing ESM branch predicate or document and
centralize the shared condition so CommonJS and ESM resolve node: specifiers
consistently.

Comment on lines +272 to +301
Local<v8::Function> NsBuiltinModules::GetFormatFunc(Local<Context> context) {
Isolate* isolate = v8::Isolate::GetCurrent();
std::shared_ptr<Caches> cache = Caches::Get(isolate);
if (cache->FormatFunc != nullptr) {
return cache->FormatFunc->Get(isolate);
}
if (cache->FormatFuncUnavailable) {
return Local<v8::Function>();
}

TryCatch tc(isolate);
Local<Object> exports;
Local<Value> format;
if (!GetExports(context, "ns:util").ToLocal(&exports) ||
!exports->Get(context, tns::ToV8String(isolate, "format"))
.ToLocal(&format) ||
!format->IsFunction()) {
if (tc.HasCaught()) {
tns::LogError(isolate, tc);
}
// One attempt per realm: a broken builtin must not make every log call
// recompile it.
cache->FormatFuncUnavailable = true;
return Local<v8::Function>();
}

cache->FormatFunc = std::make_unique<Persistent<v8::Function>>(
isolate, format.As<v8::Function>());
return format.As<v8::Function>();
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

fd -t f 'Console.cpp' --exec rg -n -C8 'InitInspect|GetFormatFunc|FormatFunc|Log|NSLog' {}

Repository: NativeScript/ios

Length of output: 11507


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Locate the relevant source files without executing repository code.
fd -t f '(NsBuiltinModules|Caches|BuiltinLoader|NativeScriptRuntime|RuntimeConfig)\.(cpp|h|hpp)$' .

printf '\n--- NsBuiltinModules outlines ---\n'
for f in $(fd -t f 'NsBuiltinModules\.(cpp|h|hpp)$' .); do
  echo "FILE $f"
  ast-grep outline "$f" --view compact || true
done

printf '\n--- Relevant NsBuiltinModules lines ---\n'
fd -t f 'NsBuiltinModules\.cpp$' . --exec sh -c 'echo "FILE $1"; sed -n "1,360p" "$1" | cat -n' sh {}

printf '\n--- Relevant Console.cpp section ---\n'
fd -t f 'Console\.cpp$' . --exec sh -c 'echo "FILE $1"; sed -n "60,290p" "$1" | cat -n' sh {}

printf '\n--- Searches for BuiltinModulesInProgress / InProgress / GetExports ---\n'
rg -n -C3 'BuiltinModulesInProgress|In.*(Progress|Loading)|Is.*Loading|LoadBuiltin|GetExports|FormatFuncUnavailable|FormatFunc' .

Repository: NativeScript/ios

Length of output: 50373


Do not treat re-entrant GetExports failures as permanent.

GetFormatFunc() can fail while ns:util is being instantiated because BuildBinding() calls Console::InitInspect() before returning exports. If this re-entry throws through GetExports("ns:util"), GetFormatFunc() sets FormatFuncUnavailable = true for the realm even though ns:util later completes successfully. Skip latching the flag when BuiltinModulesInProgress.count("ns:util") is non-zero.

🤖 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 `@NativeScript/runtime/NsBuiltinModules.cpp` around lines 272 - 301, Update the
failure path in NsBuiltinModules::GetFormatFunc so FormatFuncUnavailable is set
only when BuiltinModulesInProgress does not contain "ns:util"; preserve the
existing error logging and empty-function return, but avoid permanently caching
failures caused by re-entrant ns:util initialization.

Comment on lines +86 to +91
it("backs console.* formatting", function () {
expect(function () {
console.log("%d apples", 3);
console.log("100%");
}).not.toThrow();
});

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Assert the formatted console output.

This test passes if console.log does not use util.format. Capture the console sink or install the test runner’s supported console spy. Assert that the first call emits 3 apples rather than %d apples, and that 100% remains unchanged.

🤖 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 `@TestRunner/app/tests/NsUtilTests.js` around lines 86 - 91, Update the “backs
console.* formatting” test to spy on or capture the supported console output
sink instead of only asserting no throw. Verify the first console.log call emits
“3 apples” and the second emits “100%” unchanged, using the test runner’s
existing console-spy mechanism if available.

@NathanWalker
NathanWalker merged commit 22226ba into main Jul 31, 2026
9 checks passed
@NathanWalker
NathanWalker deleted the feat/ns-util branch July 31, 2026 16:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants