mirror of
https://github.com/got-feedBack/feedBack.git
synced 2026-08-11 03:09:57 +00:00
refactor(app): carve the plugin loader out of app.js into static/js/ (R3a) (#878)
The first carve, and deliberately the riskiest: app.js IS the plugin loader (the
R0 host rails), so it goes first while the module graph is still one edge deep.
static/js/plugin-loader.js (829 lines) — bodies VERBATIM. app.js 12,217 → 11,439.
Core's first `static/js/` module, exactly as constitution II anticipates.
CLOSURE (measured with acorn, not regex — brace-matching stripped source drifted):
the block at app.js:11246-12031 is contiguous and self-contained. It needs only
TWO things from the rest of app.js, and exports only TWO:
exports: loadPlugins (the window contract), bootstrapPluginsAndUi (boot)
inbound: window.showScreen — already the public host contract (constitution II),
so it is called through `window`, not re-coupled as an import
_populateVizPicker — injected via configurePluginLoader()
WHY A SEAM, NOT AN IMPORT. plugin-loader must not import app.js: app.js imports
it, so that would close a cycle. I checked whether _populateVizPicker could just
move into the module instead (which would delete the seam entirely) — it drags 9
further symbols (_canRun3D, _autoMatchViz, _showPromotionNag, …), i.e. a whole
viz cluster. That is its own carve, so the seam stays.
THE SEAM'S DEFAULT IS LOUD, ON PURPOSE. A no-op stub is the classic silent
failure for this pattern (see the editor's setHostHooks trap, hit twice): drop the
wiring call and the loader keeps working while the viz picker quietly stops
refreshing — no test, no boot check says a word. The default now console.errors,
so the smoke harness catches it. VERIFIED BY BITE TEST: removing
configurePluginLoader() from app.js surfaces
"[plugin-loader] host seam not configured" at boot. The seam IS exercised on the
plugin-startup path, so an unwired hook cannot pass silently.
no-cycle is now LIVE on core's own graph for the first time. eslint.config.js
gains `static/app.js` + `static/js/**` to the module block — app.js now `import`s,
so parsing it as a script would be a syntax error. VERIFIED BY BITE TEST: making
plugin-loader import app.js back fails with "Dependency cycle detected".
HARNESSES (the R3a note said budget one conversion per carve — it was five):
retargeted capability_inspector_nav, plugin_hydration_wipe,
plugin_loader_script_type, plugin_style_injection, legacy_shim_hits (SPLIT — one
test needs the loader, one still needs app.js) + test_plugin_runtime_idempotence.
legacy_shim_hits was missed by a symbol-name grep because it greps for a code
STRING; only the failing run found it. test_capability_events' NEGATIVE asserts
now span app.js + the loader — carving code out of app.js would otherwise make
them vacuous instead of failing.
VERIFIED: A/B against origin/main in two browsers — mounted plugin screens, 14
loaded plugin scripts, the 3 module plugins injected as <script type="module">,
37 capability participants, 14 shims, window.loadPlugins: IDENTICAL, zero
console/page errors on both. /static/js/plugin-loader.js serves 200; R0 rails
intact (src/main.js 200, conditional GET 304, script_type passthrough).
pytest 2396, node 1032/1032, ESLint 0, Codex 0.
Codex preflight caught a REAL [P1] first pass: static/js/plugin-loader.js was
untracked, so a checkout would have served an app.js importing a nonexistent
module — a failed static import kills the whole module and every window handler
with it. Now tracked.
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
92c86f5393
commit
38772f604a
@@ -7,6 +7,10 @@ import pytest
|
||||
ROOT = Path(__file__).resolve().parents[1]
|
||||
WORKSPACE_ROOT = ROOT.parent
|
||||
|
||||
# The plugin loader was carved out of static/app.js into its own module (R3a).
|
||||
# These tests assert on its source text, so they read it from its new home.
|
||||
PLUGIN_LOADER = ROOT / "static" / "js" / "plugin-loader.js"
|
||||
|
||||
|
||||
def _sibling_file(plugin_dir: str, filename: str) -> Path:
|
||||
path = WORKSPACE_ROOT / plugin_dir / filename
|
||||
@@ -23,7 +27,7 @@ def _sibling_text(plugin_dir: str, filename: str, required_token: str | None = N
|
||||
|
||||
|
||||
def test_plugin_loader_guards_duplicate_hydration_and_scripts():
|
||||
source = (ROOT / "static" / "app.js").read_text(encoding="utf-8")
|
||||
source = PLUGIN_LOADER.read_text(encoding="utf-8")
|
||||
|
||||
assert "let _loadPluginsInFlight = false" in source
|
||||
assert "window.feedBack._loadedPluginScripts" in source
|
||||
@@ -31,7 +35,7 @@ def test_plugin_loader_guards_duplicate_hydration_and_scripts():
|
||||
|
||||
|
||||
def test_plugin_loader_unmounts_previous_ui_contributions_before_reregistering():
|
||||
source = (ROOT / "static" / "app.js").read_text(encoding="utf-8")
|
||||
source = PLUGIN_LOADER.read_text(encoding="utf-8")
|
||||
|
||||
assert "const _pluginUiContributions = new Map()" in source
|
||||
assert "await _commandUiDomain(contribution.domain, 'unmount', plugin, contribution)" in source
|
||||
@@ -48,7 +52,7 @@ def test_plugin_loader_does_not_treat_response_absence_as_uninstall():
|
||||
# (plugin scripts don't re-run), and the DOM/style wipes forced a
|
||||
# mid-session screen.js re-evaluation that duplicated the desktop
|
||||
# audio_engine's native signal chain.
|
||||
source = (ROOT / "static" / "app.js").read_text(encoding="utf-8")
|
||||
source = PLUGIN_LOADER.read_text(encoding="utf-8")
|
||||
|
||||
# The absence-triggered sweep is gone (rationale comment in its place)...
|
||||
assert "const livePluginIds" not in source
|
||||
@@ -154,7 +158,13 @@ def test_deferred_runtime_domains_remain_reserved_not_bridged():
|
||||
|
||||
|
||||
def test_capability_events_do_not_bridge_deferred_surfaces():
|
||||
app_source = (ROOT / "static" / "app.js").read_text(encoding="utf-8")
|
||||
# These are NEGATIVE assertions, so they must span every file the code could
|
||||
# have moved to — otherwise carving a function out of app.js turns the guard
|
||||
# vacuous instead of failing.
|
||||
app_source = (
|
||||
(ROOT / "static" / "app.js").read_text(encoding="utf-8")
|
||||
+ PLUGIN_LOADER.read_text(encoding="utf-8")
|
||||
)
|
||||
capability_source = (ROOT / "static" / "capabilities.js").read_text(encoding="utf-8")
|
||||
|
||||
for token in ["return 'ui.navigation'", "return 'note-detection'", "eventName.startsWith('viz:') || eventName.startsWith('highway:')"]:
|
||||
@@ -164,7 +174,7 @@ def test_capability_events_do_not_bridge_deferred_surfaces():
|
||||
|
||||
|
||||
def test_plugin_loader_registers_manifest_capability_declarations():
|
||||
source = (ROOT / "static" / "app.js").read_text(encoding="utf-8")
|
||||
source = PLUGIN_LOADER.read_text(encoding="utf-8")
|
||||
|
||||
assert "const capabilityPlugins = fetchedPlugins.slice().sort((a, b) => String(a.id || '').localeCompare(String(b.id || '')))" in source
|
||||
assert "capabilityApi.registerParticipants(capabilityPlugins)" in source
|
||||
|
||||
Reference in New Issue
Block a user