feat: ns:util builtin module (inspect, format) - #1992
Open
edusperoni wants to merge 1 commit into
Open
Conversation
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, so `console.log("%d apples", 3)` prints `3 apples`.
v1 exports `inspect(value[, options])` — the console formatter, now reachable
from app code — and `format(fmt, ...args)`, Node's `util.format`.
`docs/ns-builtin-modules.md` is the cross-runtime contract both runtimes
implement against.
Resolution rules:
- `ns:`/`node:` specifiers are intercepted before any filesystem or npm
resolution, in all three entry points: the CommonJS require path
(ModuleInternal::RequireCallbackImpl), the ES module resolve callback and
the dynamic-import host callback (ModuleInternalCallbacks.cpp). 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`, instantiated and cached per realm.
- 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. Android has no Caches class, so every per-realm
cache is an isolate-keyed map released from disposeIsolate; the
process-global g_moduleRegistry deliberately holds none of it.
- Bare specifiers are untouched: `require("util")` still resolves through npm.
- Exports are frozen.
The builtin function wrapper becomes `(exports, require, module, binding,
primordials)`. That `require` resolves builtin specifiers only — no path, no
package, no filesystem — and is how `node:util` consumes `ns:util`, keeping
all Node adaptation in the shim and out of `ns-util.js`.
Behavior changes:
- The generic fallback for unshimmed `node:` imports (a `console.warn` plus an
empty default export) is gone; those imports now fail with the contract
message instead of silently succeeding and breaking at first use.
- The `node:fs` stub, whose members threw when called, is gone for the same
reason: the contract requires unimplemented members to be absent, never
present-but-throwing.
- The `node:url`, `node:module` and `node:path` polyfills provide real
functionality and are kept unchanged. They predate the registry and stay
import-only, so `require("node:path")` reports the builtin as missing; this
is documented in the spec's non-normative Android section.
Mirrors NativeScript/ios#418.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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. Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Android mirror of NativeScript/ios#418. Stacked on #1991 (
feat/inspect) — review only the last commit.Runtime-provided modules get a resolution scheme.
require("ns:util"),import util, { inspect } from "ns:util"andawait import("ns:util")all resolve to the same per-realm module, andconsole.*gains Node's%-substitution.docs/ns-builtin-modules.md(added here, same document as the iOS PR) is the cross-runtime contract both runtimes implement, with a non-normative Android section.v1 exports —
ns:util:inspect(value[, options])(the console formatter from the inspect PR, same instance) andformat(fmt, ...args)(Node's%s %d %i %f %j %o %O %%).Resolution rules
ns:/node:specifiers are intercepted before any filesystem or npm resolution in all three entry points: the CommonJS require path (ModuleInternal::RequireCallbackImpl), the ES module resolve callback, and the dynamic-import host callback (ModuleInternalCallbacks.cpp). A builtin can never be shadowed by a file or a package.v8::Module::CreateSyntheticModule, exporting every member by name plusdefault; instantiated, evaluated and cached per realm.No such built-in module: <specifier>— synchronously forrequire, as a rejection forimport().Cachesclass, so per-realm caches are isolate-keyed maps released on isolate disposal; the process-globalg_moduleRegistrydeliberately holds none of this state.require("util")still resolves through npm exactly as before (there is a test asserting it does not return the builtin).The internal
require(new wrapper parameter) — the builtin wrapper becomes(exports, require, module, binding, primordials), Node's order. Thatrequireis created once per realm and resolves builtin specifiers only — no path, no package, no filesystem — with an in-progress guard so cycles throw instead of recursing. It's hownode:util(node-util.js, its own lazy builtin) consumesns:utiland re-exports: all Node-compat adaptation lives in the shim,ns-util.jscarries none, and the two module objects are distinct and separately frozen (nodeUtil.inspect === nsUtil.inspect, objects differ).console integration — the arg-joining routine calls the cached
formatwith the full argument list, soconsole.log("%d apples", 3)prints3 appleswhileconsole.log("100%"), unknown specifiers and a dangling%stay verbatim. Non-format calls keep joining with spaces; if the builtin is unavailable the old per-argument loop still runs, so logging can never be taken down by the formatter.node:polyfills with the contract):node:imports (console.warn+export default {}, which broke at first use) is removed — they now fail withNo such built-in module: node:<name>.node:fsstub whose every member threw on use is removed for the same reason (the contract requires missing members to be absent, never present-but-throwing) —node:fsnow fails with the not-found message.node:url(fileURLToPath/pathToFileURL),node:module(createRequire) andnode:pathpolyfills are kept unchanged, import-only —require("node:path")reports not-found whileimportworks, mirroring iOS's pre-registrynode:urlasymmetry. Documented in the doc's Android non-normative section.Related Pull Requests
Does your pull request have unit tests?
Yes — 21 new device specs (
tests/testNsUtil.js+ a static-import.mjsfixture covering the resolve callback, + a worker spec asserting a worker builds its ownns:utilinstance): frozen exports, require/static/dynamic-import singleton identity, every format specifier incl.%%/unknown/dangling%/%jon a circular value/extras/non-string first arg,node:utildistinctness with shared members, exact unknown-ns:/node:messages in both schemes, and the bare-utilnon-change. Full suite: 648 specs, 0 failures (627 baseline + 21).