mirror of
https://github.com/got-feedBack/feedBack.git
synced 2026-07-21 12:21:49 +00:00
756588678b
A plugin reload silently did nothing for scriptType:"module" plugins. ES modules are evaluated ONCE PER URL PER DOCUMENT, so re-inserting a <script type="module"> whose src the module map has already seen fires `load` without re-running the body — and the loader then recorded the reload as applied. A no-op that reported success. THE ISSUE UNDERSTATES IT. #879 says "upgrades are fine — a new version yields a new URL". That is true of screen.js and FALSE of the plugin. I drove a real browser through install(1.0.0) -> upgrade(1.1.0) -> rollback(1.0.0), counting evaluations of src/main.js: ONE. Not three, not two. The upgrade re-runs the one-line screen.js shim at its new ?v= URL; the shim does `import './src/main.js'`; a relative specifier resolves against the base URL WITH THE QUERY DROPPED; that is the same URL as before; the module map hands back the already-evaluated v1.0.0 module. The plugin's own code never re-ran. Busting the entry point cannot fix this, whatever token you hang off it. So the token goes in the PATH: /api/plugins/<id>/g/<n>/screen.js. From there './src/main.js' resolves to /api/plugins/<id>/g/<n>/src/main.js — every relative import inherits it, at every depth, for free. No import-specifier rewriting (which could never see `import(expr)` anyway). Same browser drive after the fix: THREE evaluations. Keyed on the plugin ID, not id@version: EVERY re-load of a module plugin needs a fresh path, not just a rollback. First load keeps the stable ?v= URL, so the ETag/304 live-edit caching the R0 rails depend on is untouched. Classic-script plugins are not affected and never take a /g/ path. ━━━ A PATH REWRITE, NOT TWO MIRRORED ROUTES ━━━ Codex caught this, and it was right. The token shifts the BASE URL, so EVERYTHING the module graph resolves relatively moves with it — not only imports. `new URL('../assets/worklet.js', import.meta.url)` from /api/plugins/x/g/1/src/main.js resolves to /api/plugins/x/g/1/assets/worklet.js. Mirroring only screen.js and src/ would have fixed imports and 404'd every asset, worklet and wasm file the graph reaches — and would have broken again the next time someone added a plugin route. So the /g/<token> segment is STRIPPED BEFORE ROUTING. Every plugin route, present and future, works under the prefix with no extra wiring. The token is opaque and never joined into a filesystem path, so containment still rests entirely on the same safe_join. Codex then caught a [P3] in that: eagerly re-encoding raw_path with latin-1 raises UnicodeEncodeError on a valid plugin file like src/工具.js, 500ing a request the plain route serves fine. raw_path is informational and Starlette routes on scope["path"], so the mutation is simply gone — and leaving raw_path as the client sent it is more truthful for logs anyway. TESTS. tests/js/plugin_module_rollback.test.js (5) + 8 in test_plugin_src_route.py: identical bytes under the prefix, the whole graph one and two levels deep, ASSETS (the Codex [P2]), every plugin route, non-ASCII filenames (the [P3]), an opaque token, and containment asserted as PARITY with the un-prefixed route rather than a guessed 404 — `../screen.js` legitimately 200s on both, because the URL normalises before routing. All bite-tested: reverting the fix fails the rollback tests, disabling the rewrite fails the asset tests. Two harnesses re-anchored on `script.src = _pluginScriptUrl(` — the URL literal they keyed on now lives in the helper, further down the file, so their slice ran off the end. node 1045, pytest 2404, ESLint 0, Codex 0. Closes #879 Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
84 lines
4.1 KiB
JavaScript
84 lines
4.1 KiB
JavaScript
// #879 — a plugin ROLLBACK must actually re-evaluate a module plugin.
|
|
//
|
|
// ES modules are evaluated once per URL per document. Re-inserting a
|
|
// <script type="module"> whose src the module map has already seen fires `load` but
|
|
// does NOT re-run the body — so rolling back to a version already evaluated this
|
|
// session left the OLD module live while the loader recorded success.
|
|
//
|
|
// The fix puts a generation token in the PATH (/api/plugins/x/g/7/screen.js), not the
|
|
// query, because a relative specifier resolves against the base URL with the query
|
|
// DROPPED — so './src/main.js' would otherwise keep resolving to the same cached URL
|
|
// and the plugin's actual code would never re-run.
|
|
|
|
const { test } = require('node:test');
|
|
const assert = require('node:assert/strict');
|
|
const fs = require('node:fs');
|
|
const path = require('node:path');
|
|
const vm = require('node:vm');
|
|
|
|
const { extractFunction } = require('./test_utils');
|
|
const LOADER = path.join(__dirname, '..', '..', 'static', 'js', 'plugin-loader.js');
|
|
|
|
function makeUrlBuilder() {
|
|
const src = fs.readFileSync(LOADER, 'utf8');
|
|
const sandbox = { _evaluatedModules: new Set(), _moduleReloadSeq: 0 };
|
|
vm.createContext(sandbox);
|
|
vm.runInContext(`
|
|
${extractFunction(src, 'function _pluginScriptUrl(')}
|
|
globalThis.url = _pluginScriptUrl;
|
|
`, sandbox);
|
|
return sandbox.url;
|
|
}
|
|
|
|
const MOD = { id: 'editor', script_type: 'module' };
|
|
const CLASSIC = { id: 'legacy', script_type: 'classic' };
|
|
|
|
test('a module plugin first load uses the stable ?v= URL (ETag/304 stays intact)', () => {
|
|
const url = makeUrlBuilder();
|
|
assert.equal(url(MOD, '1.0.0', '?v=1.0.0'), '/api/plugins/editor/screen.js?v=1.0.0');
|
|
});
|
|
|
|
// An UPGRADE has to bust the graph too, and this is the part #879 got wrong. It says
|
|
// "upgrades are fine — a new version yields a new URL". True of screen.js; FALSE of the
|
|
// plugin. Driving a real browser through install -> upgrade -> rollback and counting
|
|
// evaluations of src/main.js gives ONE: the upgrade re-runs the one-line screen.js shim
|
|
// at its new ?v= URL, the shim imports './src/main.js', that resolves to the SAME url,
|
|
// and the module map hands back the already-evaluated old module. So the key here is the
|
|
// plugin ID, not id@version — every re-load of a module plugin needs a fresh path.
|
|
test('an UPGRADE also gets a fresh /g/<n>/ path — a new ?v= does NOT reach the graph', () => {
|
|
const url = makeUrlBuilder();
|
|
url(MOD, '1.0.0', '?v=1.0.0');
|
|
assert.equal(url(MOD, '1.1.0', '?v=1.1.0'), '/api/plugins/editor/g/1/screen.js?v=1.1.0');
|
|
});
|
|
|
|
test('a ROLLBACK to an already-evaluated version gets a fresh /g/<n>/ PATH', () => {
|
|
const url = makeUrlBuilder();
|
|
url(MOD, '1.0.0', '?v=1.0.0'); // installed
|
|
url(MOD, '1.1.0', '?v=1.1.0'); // upgraded -> /g/1/
|
|
const back = url(MOD, '1.0.0', '?v=1.0.0'); // rolled back -> /g/2/
|
|
assert.equal(back, '/api/plugins/editor/g/2/screen.js?v=1.0.0');
|
|
|
|
// The token must be in the PATH so a relative import INHERITS it — the whole point.
|
|
// A query token is dropped by URL resolution and never reaches src/main.js.
|
|
const resolved = new URL('./src/main.js', `http://h${back}`).pathname;
|
|
assert.equal(resolved, '/api/plugins/editor/g/2/src/main.js',
|
|
'the token must reach the module GRAPH, not just the entry point');
|
|
});
|
|
|
|
test('every re-load gets a distinct URL (no reuse across a bounce)', () => {
|
|
const url = makeUrlBuilder();
|
|
url(MOD, '1.0.0', '?v=1.0.0');
|
|
const seen = new Set();
|
|
for (const v of ['1.1.0', '1.0.0', '1.1.0', '1.0.0']) seen.add(url(MOD, v, `?v=${v}`));
|
|
assert.equal(seen.size, 4, 'each re-load must be a URL the module map has never seen');
|
|
});
|
|
|
|
test('classic-script plugins are untouched — they always re-run on re-insert', () => {
|
|
const url = makeUrlBuilder();
|
|
const first = url(CLASSIC, '1.0.0', '?v=1.0.0');
|
|
url(CLASSIC, '1.1.0', '?v=1.1.0');
|
|
const back = url(CLASSIC, '1.0.0', '?v=1.0.0');
|
|
assert.equal(first, '/api/plugins/legacy/screen.js?v=1.0.0');
|
|
assert.equal(back, first, 'a classic script needs no cache-busting and must not get a /g/ path');
|
|
});
|