feat: Node-style primordials for runtime builtins - #415
Conversation
|
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 |
e31477b to
bce9ece
Compare
|
Orchestrator review pass (folded into the commit):
Deferred (pre-existing, unchanged by this PR): |
The runtime's builtins install globals and leave closures behind that keep running for the lifetime of the app: event dispatch, the Promise proxy traps, console's smart-stringify, __extends. Those closures reached for intrinsics (Array.prototype.slice, JSON.stringify, Object.create, ...) through the live globals, so app code replacing one could break or observe runtime internals. primordials.js captures the intrinsics the other builtins need into a frozen, null-prototype namespace and BuiltinLoader passes it to every builtin as a second fixed parameter next to `binding`. It is built lazily on the first RunBuiltin of an isolate — during runtime init, before user code — and cached in Caches, so workers snapshot their own realm and late-compiling builtins still see pristine intrinsics. Instance methods are uncurried Node-style, `uncurryThis(fn)` being Function.prototype.bind.bind(Function.prototype.call): ArrayPrototypeSlice(list, 1) rather than list.slice(1). Benchmarked under jitless (which is how the runtime always runs on device) uncurried calls cost +5-12% per op over a raw method call but beat the captured-and-.call() alternative, so they are used uniformly. ESLint gains no-restricted-properties for the captured statics and no-restricted-globals for the captured constructors, each pointing at the replacement; uncurried instance-method use stays a review rule since the receiver cannot be matched.
bce9ece to
1c9f00c
Compare
Stacked on #411 — review that one first; this PR targets
feat/js-builtins.What this adds
The runtime's builtin JavaScript installs globals and leaves closures behind that run for the lifetime of the app: event dispatch, the
Promiseproxy traps, console's smart-stringify,__extends, theURL.searchParamsaccessor. Until now those closures reached for intrinsics (Array.prototype.slice,JSON.stringify,Object.create, …) through the live globals, so app code replacing one could break runtime internals or observe them.NativeScript/runtime/js/primordials.jsis a new builtin that captures exactly the intrinsics the other builtins need into a frozen, null-prototype namespace, Node-style.The
(binding, primordials)contractBuiltins are now compiled as function bodies with two fixed parameters:
primordialsis built lazily on the firstRunBuiltinof an isolate — which happens during runtime init, before any user code — and cached inCaches::Primordials. Consequences:smart-stringifyis only compiled on the first object logged, which can be well after user code has run.bindingis unchanged; most builtins still receiveundefinedfor it.Why uncurrying, uniformly
Instance methods are exposed uncurried, exactly as Node does it:
The runtime always runs V8 jitless on device, so the usual "uncurrying is free once optimized" argument does not apply and the cost had to be measured. Benchmarked on Node 24 / V8 13.6, arm64 host,
--jitless:captured.call(recv, …)alternative, which is why uncurrying is used uniformly rather than mixing styles.JSON.stringifyetc.) are free.Reflect.applyis measurably worse and is not used;FunctionPrototypeApplycovers those sites.Plain constructor calls made once at init time (
new Map()while bootstrapping) may stay direct — the rule targets code in closures that outlive init.Enforcement
eslint.config.mjsdeclaresprimordialsand adds two rules overNativeScript/runtime/js, both verified to fire:no-restricted-propertiesfor every captured static (JSON.stringify,Object.defineProperty/assign/freeze/create/keys/setPrototypeOf/getOwnPropertyDescriptor,Array.from,Reflect.construct,Reflect.apply).no-restricted-globalsfor every captured constructor (Error,Map,Proxy,TypeError). Aconst { Proxy } = primordialsdestructure shadows the global, so the rule only fires on an unguarded reference.Each message names the replacement.
primordials.jsitself is exempted via a per-file override. Uncurried instance-method use cannot be matched by receiver, so that stays a review rule — documented as such inNativeScript/runtime/js/README.md.Deliberately left as live global lookups:
Reflect.decorateininline-functions.js(reflect-metadatainstalls it after runtime init, and the TypeScript__decorateshim must see it),Blob/Fileinblob-url.js(provided by the app layer, not the runtime), andString(x)/instanceof Arrayconversions where tamper-immunity is not affected.ts-helpersbuilds native super calls with the capturedReflect.constructrather thanbind-then-new, which keeps that path off the array iterator protocol as well.Tests
New
TestRunner/app/tests/PrimordialsTests.js(7 specs) tampers with the intrinsics and checks the runtime keeps working. Each spec keeps the tampered window synchronous and assertion-free, restoring the originals in afinally:dispatchEventdelivers to every listener (function andhandleEvent) withArray.prototype.slice/indexOf/push/spliceandFunction.prototype.callreplacedaddEventListener/removeEventListenerandoncesemantics under the same tamperingreportErrorstill reaches anerrorlistener with a correctErrorEventconsole.logof a circular object stays non-fatal withJSON.stringifyand the replacer'sArray.prototype.indexOf/pushreplaced (the smart-stringify path; its output is not reachable from JS andJsonStringifyObjectswallows a throwing stringify, so this spec is a crash/throw check rather than an output assertion — noted in the test)_super.call(this, x)over an Objective-C base withArray.prototype.slice/concat,Function.prototype.bindandReflect.constructreplaced__extendsand__tsEnumwith theObjectstatics replacedResult: 912 specs, 0 failures (905 baseline + 7), independently reproduced on a second simulator. ESLint clean; every builtin verified to compile as a
(binding, primordials)function body.Known follow-ups (not in this PR)
Pre-existing live-intrinsic reads that primordials does not yet cover:
for...of ObjectKeys(...)in__tsEnumandArrayFrom(arguments)ininline-functions.jsstill go through the array iterator protocol, andString(x)conversions inevents.js/error-events.jsread the live globalString.