Skip to content

js: wire onload/onerror on elements; RunPending sees timer-injected scripts - #160

Merged
tannevaled merged 1 commit into
mainfrom
js-onload-runpending-pacing
Sep 9, 2026
Merged

tannevaled merged 1 commit into
mainfrom
js-onload-runpending-pacing

Conversation

@tannevaled

Copy link
Copy Markdown
Contributor

Summary

Two independent gaps in dynamic script loading, both found tracing react.dev's own ChunkLoadError via direct instrumentation:

  1. onload/onerror did not exist as properties on any element. webpack/Turbopack's classic chunk-loading helper sets both directly on a dynamically-created <script> (never via addEventListener), so even a successful load could never resolve its own promise. Fixed by wiring both generically in defineElement, backed by a new per-node handler-slot map so a later assignment replaces (not stacks with) an earlier one. runScripts now dispatches "load" after a script fetches and runs, "error" when the fetch itself fails.
  2. Session.RunPending decided its "did anything happen" signal from runScripts alone, called BEFORE drainTimers — but the standard chunk-loading idiom creates its <script> from exactly a timer callback, so a freshly-appended, never-executed script could sit in the DOM while the caller's settle loop read a false "ran" as "nothing left to do" and stopped iterating. Fixed by having RunPending check for pending unexecuted scripts after draining timers (an earlier attempt that ran scripts twice per call broke TestSettleFixpointCap's one-generation-per-pass guarantee — fixed by reporting "pending" without running inline).

Honestly, neither fix alone closes react.dev's own ChunkLoadError — confirmed directly that the chunk still never fetches, for a separate, deeper reason: this engine's setTimeout binding ignores its own delay argument, draining every timer job in pure FIFO order, so a bundler's internal "give up after 120s" safety-net timeout structurally races ahead of (and always wins against) the chunk's own near-instant real fetch. Flagged for a future round, not attempted here.

Test plan

  • TestSessionInjectedScriptOnloadAndOnerrorFire added — covers both outcomes (fetch succeeds → onload; fetch fails → onerror), never the wrong one
  • TestSessionRunPendingReportsPendingScriptAppendedDuringDrainTimers added — a focused white-box test for the exact "runScripts finds nothing, but this call's own drainTimers appends a script" ordering
  • git-stash revert-and-confirm-fail: both new tests fail with the exact predicted symptoms when their respective mechanism is reverted
  • TestSettleFixpointCap (broken by an earlier over-corrected attempt) re-confirmed green
  • go test ./... green; css/layout/paint/dom/paginate coverage floors re-checked (99.5%/100.0%/100.0%/98.1%/100.0%, unaffected — this fix lives in js, not itself coverage-gated)
  • bench/cmd/compare run against real headless Chrome; react.dev SSIM exactly unchanged (0.6126→0.6126), correctly predicted in advance given the confirmed separate remaining cause

🤖 Generated with Claude Code

…cripts

Two independent gaps in dynamic script loading, both found tracing
react.dev's own ChunkLoadError via direct instrumentation:

1. onload/onerror did not exist as properties on any element.
   webpack/Turbopack's classic chunk-loading helper sets both
   directly on a dynamically-created <script> (never via
   addEventListener), so even a successful load could never resolve
   its own promise. Fixed by wiring both generically in
   defineElement, backed by a new per-node handler-slot map so a
   later assignment replaces (not stacks with) an earlier one --
   real IDL-event-handler-attribute semantics. runScripts now
   dispatches "load" after a script fetches and runs, "error" when
   the fetch itself fails.

2. Session.RunPending decided its "did anything happen" signal from
   runScripts alone, called BEFORE drainTimers -- but the standard
   chunk-loading idiom creates its <script> from exactly a timer
   callback, so a freshly-appended, never-executed script could sit
   in the DOM while the caller's settle loop read a false "ran" as
   "nothing left to do" and stopped iterating. Fixed by having
   RunPending check for pending unexecuted scripts after draining
   timers, reporting there is more to do without running it inline
   (an earlier attempt that ran scripts twice per call broke
   TestSettleFixpointCap's one-generation-per-pass guarantee).

Neither fix alone closes react.dev's own ChunkLoadError -- confirmed
directly that the chunk still never fetches, for a separate, deeper
reason (this engine's setTimeout ignores its own delay, so a
bundler's internal safety-net timeout always wins the FIFO race
against the chunk's real fetch) flagged for a future round.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@tannevaled
tannevaled merged commit 7b7169e into main Sep 9, 2026
7 checks passed
@tannevaled
tannevaled deleted the js-onload-runpending-pacing branch September 9, 2026 20:07
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.

1 participant