mirror of
https://github.com/got-feedBack/feedBack.git
synced 2026-08-11 03:09:57 +00:00
refactor(ui): defer every classic script, keep boot() on DOMContentLoaded (R3a) (#872)
Puts every external `<script>` in the v3 shell into the deferred queue, and
keeps each script's boot() firing at DOMContentLoaded exactly as it does today.
Behaviourally a no-op; it is what makes the ES-module flips safe.
WHY. `type="module"` defers execution to after HTML parse. Classic-`defer` and
module scripts share ONE "execute after parsing" list and run in DOCUMENT ORDER,
but a plain classic script runs DURING parse — ahead of all of them. So the
moment capabilities.js becomes a module while app.js is still plain, app.js runs
FIRST, and its 11 top-level `window.feedBack.on(...)` calls (app.js:6245-6722)
hit a bare `{}` — `_ensureFeedBackEventBus()` (capabilities.js:33), which
attaches .on/.emit/.off, would not have run yet. TypeError, app.js dies
mid-parse. Deferring everything now keeps document order == execution order
through the rest of the migration.
THE CATCH (Codex preflight caught this — a real ordering change). 22 scripts
guard their boot with `if (document.readyState === 'loading')`. A deferred
script runs at readyState 'interactive', so that test is FALSE and the else-branch
fires boot() immediately, at the script's position in document order — instead of
at DOMContentLoaded, after every script has evaluated.
That matters far more than one call site: a scan of the shell's scripts found
**43 forward references** where a script's boot() reads a global that a LATER
script defines (shell.js -> profile.js's window.v3Onboarding, songs.js ->
settings.js's window._confirmDialog, badges.js -> songs.js's
window.displayTuningName, ...). Every one of them resolves today only because
all boots happen at DOMContentLoaded. So the guards now treat 'interactive' as
not-ready (`!== 'complete'`), restoring that exactly.
Codex's specific finding (first-run onboarding silently skipped) did NOT
reproduce — shell.js's boot() awaits /api/profile, and that yield lets the
remaining deferred scripts run first. But the race it described is real, the
guard is silent when it fails (`&& window.v3Onboarding`), and the other 42
forward refs have no such await protecting them. Fixed at the root rather than
at the one site.
VERIFIED. A/B against origin/main on a fresh profile, 13 probes (onboarding
overlay, v3Onboarding/v3Songs/v3Profile/fbNotify/v3Badges/uiPrompt/showScreen,
bus, capabilities.version, createHighway, plugin scripts, mounted screens):
IDENTICAL, zero console/page errors on both. pytest 2396, node 1028/1028,
ESLint 0 errors, Codex 0.
New guard: test_every_external_script_defers_so_document_order_is_execution_order
fails if any external tag is plain classic — verified to fail on a single
reverted tag, so it actually bites.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
9d0bf95716
commit
4b4c156fce
@@ -1,3 +1,4 @@
|
||||
import re
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
@@ -71,13 +72,39 @@ def test_capability_visualizer_waits_for_registry_instead_of_hard_error():
|
||||
def test_app_shell_loads_capability_registry_before_app_runtime():
|
||||
source = (ROOT / "static" / "v3" / "index.html").read_text(encoding="utf-8")
|
||||
|
||||
assert '<script src="/static/capabilities.js"></script>' in source
|
||||
assert '<script src="/static/capabilities/library.js"></script>' in source
|
||||
assert re.search(r'<script[^>]+src="/static/capabilities\.js"', source)
|
||||
assert re.search(r'<script[^>]+src="/static/capabilities/library\.js"', source)
|
||||
assert source.index('/static/diagnostics.js') < source.index('/static/capabilities.js')
|
||||
assert source.index('/static/capabilities.js') < source.index('/static/capabilities/library.js')
|
||||
assert source.index('/static/capabilities/library.js') < source.index('/static/app.js')
|
||||
|
||||
|
||||
def test_every_external_script_defers_so_document_order_is_execution_order():
|
||||
"""The shell's scripts must all execute in document order.
|
||||
|
||||
`capabilities.js` builds the `window.feedBack` bus and must run before
|
||||
`app.js`, which calls `window.feedBack.on(...)` at top level. Today document
|
||||
order gives that for free, because every script is a parse-time classic one.
|
||||
|
||||
That guarantee survives the ES-module migration ONLY while no script is a
|
||||
*plain* classic script: `defer` and `type="module"` scripts share one
|
||||
"execute after parsing" list and run in document order, but a plain classic
|
||||
script runs DURING parse — ahead of every deferred one. So the moment
|
||||
capabilities.js becomes a module while app.js is still plain, app.js runs
|
||||
first and `window.feedBack.on` is undefined.
|
||||
|
||||
Pinning "no plain external scripts" is what keeps that from silently
|
||||
regressing as tags flip to type="module" one at a time.
|
||||
"""
|
||||
source = (ROOT / "static" / "v3" / "index.html").read_text(encoding="utf-8")
|
||||
|
||||
plain = [
|
||||
tag for tag in re.findall(r'<script\b[^>]*\bsrc=[^>]*>', source)
|
||||
if 'defer' not in tag and 'async' not in tag and 'type="module"' not in tag
|
||||
]
|
||||
assert not plain, f"external scripts that would jump the deferred queue: {plain}"
|
||||
|
||||
|
||||
def test_capability_registry_exposes_claim_dispatch_and_ready_contracts():
|
||||
source = (ROOT / "static" / "capabilities.js").read_text(encoding="utf-8")
|
||||
|
||||
|
||||
Reference in New Issue
Block a user