feat: add DOMException and CustomEvent as lazy globals - #2027
Merged
Conversation
Port of NativeScript/ios#452, the next two items of the web-globals plan, both behind the lazy-global tier. DOMException (Web IDL §4.3) is a new lazy builtin (dom-exception.js, shared verbatim with iOS): a class grafted onto Error.prototype with branded enumerable name/message/code prototype accessors, the full legacy code table, the 25 constants on interface object and prototype, @@toStringTag and stack capture. LazyGlobals places it on first read; until then nothing runs or allocates. Sibling builtins construct DOMExceptions through a new internal-only specifier tier: kRegistry rows flagged internalOnly resolve through the require builtins receive and nowhere else (the module system refuses them, and a canary pins that app code cannot name them). All five stand-in throw sites now produce real DOMExceptions, required at first throw so a clean path never runs the builtin: abort-signal.js (AbortError/TimeoutError reasons), performance.js (SyntaxError/InvalidModificationError), structured-clone.js and StructuredSerialization.cpp (DataCloneError, the native serializer keeping the name-patched-Error shape as a teardown fallback), and base64.js (InvalidCharacterError). With the tier in place the interim `internals` wrapper parameter had exactly two users left; both moved into events.js's exports behind internal/events (kListenerChanged for abort-signal's GC accounting, setListenerErrorReporter for error-events). The builtin wrapper is back to Node's five parameters (exports, require, module, binding, primordials). CustomEvent (DOM §2.4) is defined in events.js next to the Event it extends, exported rather than installed: Events::Init now runs the file through BuiltinLoader::GetExports and reads the backing EventTarget from the exports bag, so the lazy CustomEvent row is a cache hit — only the placement is deferred. Tests: shared submodule bumped to 9cc46c06 (self-gating DOMException and CustomEvent suites plus integration specs), both suites wired into mainpage.js, and unguarded canaries added so this runtime regressing the globals fails instead of skipping. Full suite: 1203 specs, 0 failures on arm64 API 33.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (28)
Included review availability: Your plan provides up to 4 included reviews per hour; 3 remain after this review. 📝 WalkthroughWalkthroughThe runtime replaces the per-runtime ChangesRuntime builtin architecture
Estimated code review effort: 4 (Complex) | ~45 minutes Merge Risk: ⚪ Minimal · up to The PR adds lazy DOMException and CustomEvent globals and updates related error paths; no actionable merge-blocking risk remains after normal checks and review. Sequence Diagram(s)sequenceDiagram
participant Runtime
participant BuiltinLoader
participant InternalModules
participant DOMException
Runtime->>BuiltinLoader: Request builtin exports
BuiltinLoader->>InternalModules: Resolve internal/dom-exception
InternalModules->>DOMException: Load and cache constructor
DOMException-->>Runtime: Return DOMException constructor
Runtime->>DOMException: Construct named exception
DOMException-->>Runtime: Throw DOMException
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 21.21% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 33 functions across 21 files. (7 skipped: 7 unsupported.) ✨ Finishing Touches 💡 1📝 Generate docstrings 💡
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 |
NathanWalker
approved these changes
Aug 25, 2026
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
2 participants
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.
Mirrors NativeScript/ios#452 — the follow-up to #2026 promised there ("a follow-up PR will introduce
DOMExceptionand upgrade these plusAbortSignal's reasons"):DOMExceptionandCustomEvent, both behind the lazy-global tier.DOMException
New lazy builtin
dom-exception.js(Web IDL §4.3), byte-identical with the iOS copy:Error.prototype—instanceof Errorholds andError.prototype.toStringrendersname: message— withname/message/codeas branded, enumerable prototype accessors (private fields double as the Web IDL brand check), the full legacy code table, the 25 constants on interface object and prototype,@@toStringTag, and stack capture.LazyGlobalson first read; until then nothing runs or allocates.Internal require tier
Sibling builtins construct DOMExceptions lazily via a new internal-only specifier tier:
kRegistryrows flaggedinternalOnlyresolve through therequirebuiltins receive and nowhere else — the module system refuses them (GetModuleguard for ES imports; the CommonJS path was already prefix-gated byIsBuiltinScheme), and a canary test pins that app code cannot name them. This is the Node internal-module idiom the js README had planned.All five existing stand-in throw sites now produce real DOMExceptions, with the builtin required at first throw so a clean path never runs it:
abort-signal.js— default abort ("AbortError") and timeout ("TimeoutError") reasonsperformance.js— SyntaxError / InvalidModificationError failuresstructured-clone.js— transfer-list DataCloneErrorbase64.js— atob/btoa InvalidCharacterErrorStructuredSerialization.cpp— the native serializer's DataCloneError, built through the same exports cache (the name-patched-Errorshape kept as a teardown fallback)internals parameter removed
With the tier in place, the interim
internalsobject (introduced with AbortSignal in #2025) had exactly two users left, and both moved intoevents.js's exports behindinternal/events(kListenerChangedfor abort-signal's GC accounting,setListenerErrorReporterfor error-events). The builtin wrapper is back to Node's five parameters (exports, require, module, binding, primordials), and a consumer resolves the capability explicitly at therequire— a cache hit for consumers of eager producers, an on-demand run otherwise — so it can never observe a missing key the way the shared object allowed.CustomEvent
Defined in
events.jsnext to theEventit extends (same ES5 idiom), exported rather than installed:Events::Initnow runs the file throughBuiltinLoader::GetExportsand reads the backing EventTarget from the exports bag, so the lazyCustomEventrow is a cache hit — only the placement is deferred.Not implemented
The spec's
[Serializable]slot for DOMException:structuredClone/workerpostMessagego throughv8::ValueSerializer, which has no hook for a plain JS class, so a DOMException inside a cloned graph degrades like any custom Error subclass.Tests
mainpage.js): self-gatingDOMExceptionandCustomEventsuites that skip with a visible pending spec where the APIs are absent, plus integration specs — gated per collaborating API — asserting AbortSignal reasons, atob failures and structuredClone failures are real DOMExceptions.testRuntimeImplementedAPIs.jsso this runtime regressing the globals fails instead of skipping, plus the app-code-cannot-require-internal pin.runtestsAndVerifyResults, arm64 API 33): 1203 specs, 0 failures.Summary by CodeRabbit
New Features
DOMExceptionsupport, including legacy properties and error codes.CustomEventsupport with read-only event details.DOMExceptiontypes.Documentation
Tests