From 0997252432029fed9309bddda5bed9f5029ce4ba Mon Sep 17 00:00:00 2001 From: Doxa Maps Date: Sun, 27 Sep 2026 04:29:29 +0000 Subject: [PATCH] check:setup-order arm C: getCurrentInstance() read from a watcher, and the early-out that hid it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Arm C catches the live RTL defect in the two plus-grid composites: a top-level function that reads getCurrentInstance() and is handed to a watcher or an addEventListener runs with no current instance, so the read returns null. IT MODELS VUE RATHER THAN BANNING THE CALL. Inside setup, and inside a lifecycle-hook callback, the call is CORRECT — Vue sets the instance for the duration of a hook — so research-map-plus-grid.vue's onMounted(() => { getCurrentInstance()... }) is deliberately not reported. A gate that flags working code gets ignored, and this one has already had to drop five such hits. ★ AND ITS OWN EARLY-OUT MADE IT USELESS FIRST, WHICH IS THE SECOND TIME TODAY FOR THIS EXACT HOLE. The file filter only ran the arms on sources containing `immediate: true` or `async` — arm A's and arm B's triggers. Neither of the two files carrying the RTL bug contains either word, so both were skipped before a single arm ran, and the gate reported the tree clean while the self-test stayed green. A fixture is text and never meets the file filter, so no self-test could have caught it; only running the gate against the un-fixed code did, which is why dx-shepherd asked for exactly that. An hour earlier I diagnosed the same shape in check:i18n-keys-exist — it skipped any file that never wrote the word "tStatic", which is how a planted poison key passed — and wrote up the rule that a cheap early-out is part of a gate's COVERAGE, not an optimisation. Then I wrote the hole again here. So the fix is structural rather than a correction: the precondition is now DERIVED from a per-arm ARM_TRIGGERS table, so adding a fourth arm forces you to extend the filter in the same edit. One more false positive the self-test caught before it shipped: `watch(textDir, reflectDir)` has no inline callback, so brace-matching forward from the call walked past it into the NEXT statement's onMounted body and reported a getCurrentInstance() nowhere near a watcher. A call's callback is now bounded by that call's own parenthesis span. Verified both directions, on the real files and not only on fixtures: FAILS on the shipped code naming both composites, PASSES with the fix, and does not flag the legal onMounted read. Gate only, no product code, no rebuild — rule 5B because it lives in internal/. --- internal/check-setup-order.mjs | 129 +++++++++++++++++++++++++++++++-- 1 file changed, 124 insertions(+), 5 deletions(-) diff --git a/internal/check-setup-order.mjs b/internal/check-setup-order.mjs index 619c431bb..054c3899d 100644 --- a/internal/check-setup-order.mjs +++ b/internal/check-setup-order.mjs @@ -114,6 +114,22 @@ function scriptOf(src, file) { return out.join('') } +/* Paren-match forward from the first `(` at or after `from`. Returns [open, close] or null. + * Arm C needs this because `watch(textDir, reflectDir)` has NO inline callback, and searching for the + * next `{` from there walks past the call entirely — the first version read the NEXT statement's + * onMounted body and reported a getCurrentInstance() that was nowhere near a watcher. Caught by the + * self-test, on the fixture written to prove the arm was quiet on correct code. */ +function parenSpan(src, from) { + const open = src.indexOf('(', from) + if (open < 0) return null + let d = 0 + for (let i = open; i < src.length; i++) { + if (src[i] === '(') d++ + else if (src[i] === ')' && --d === 0) return [open, i] + } + return null +} + /* Brace-match forward from the first `{` at or after `from`. Returns [open, close] or null. */ function braceSpan(src, from) { const open = src.indexOf('{', from) @@ -245,12 +261,76 @@ function armB(code) { return hits } +// ── ARM C: getCurrentInstance() ANSWERS ONLY WHILE SETUP OR A HOOK IS ON THE STACK ───────────── +// dashboard-plus-grid.vue:303 and research-map-plus-grid.vue:152 — a live goal-(d) RTL bug. +// +// `function reflectDir() { getCurrentInstance()?.proxy?.$el… }` was called from TWO places: +// onMounted(reflectDir), where Vue has set the current instance and it works, and +// watch(textDir, reflectDir), where nothing has, so the read returns null. A host that set lang=ar +// BEFORE mount got dir=rtl; a host that switched to ar AFTERWARDS got nothing at all, and Arabic +// rendered in a left-to-right layout. +// +// THAT SPLIT IS WHY IT SURVIVED REVIEW. The function is not wrong, and neither caller is wrong; +// the pairing is. Reading either line alone shows working code, and first paint — the thing anyone +// would check — was correct. Only the later switch was broken. +// +// SO THIS ARM MODELS VUE RATHER THAN BANNING THE CALL. Inside setup, or inside a lifecycle-hook +// callback, getCurrentInstance() is CORRECT and must not be reported — research-map-plus-grid.vue's +// onMounted(() => { getCurrentInstance()… }) is fine and is deliberately left alone. A gate that +// flags working code gets ignored, and this file has already dropped five such hits today. +// What it reports is the call reached from a watcher or an event handler, directly or through a +// top-level function. The fix is always the same: capture the instance in setup (_selfInstance). +const HOOK_SET = new RegExp('^(?:' + HOOKS.join('|') + ')$') +function armC(code) { + const hits = [] + const fns = topLevelFunctions(code) + // Which top-level functions are handed to a watcher or an event registration rather than called + // from setup? Those are the ones whose body runs with no current instance. + const deferred = new Set() + for (const m of code.matchAll(/\b(?:watch|watchEffect)\s*\(([\s\S]{0,400}?)\)/g)) { + for (const id of m[1].matchAll(/\b([A-Za-z_$][\w$]*)\b/g)) if (fns.has(id[1])) deferred.add(id[1]) + } + for (const m of code.matchAll(/addEventListener\s*\(\s*'[^']*'\s*,\s*([A-Za-z_$][\w$]*)/g)) { + if (fns.has(m[1])) deferred.add(m[1]) + } + for (const [name, fn] of fns) { + if (!deferred.has(name) || !/\bgetCurrentInstance\s*\(/.test(fn.body)) continue + hits.push({ + line: lineOf(code, fn.at), + why: `${name}() calls getCurrentInstance(), and is handed to a watcher or an event listener — there is no current instance then, so the call returns null. Capture it in setup instead (const _selfInstance = getCurrentInstance()).`, + }) + } + // And the call written straight inside a watcher callback, with no function to name. + for (const m of code.matchAll(/\b(?:watch|watchEffect)\s*\(/g)) { + const args = parenSpan(code, m.index) + if (!args) continue + const brace = braceSpan(code, m.index) + // The callback must be INSIDE this call's own argument list. Without that test the search runs on + // to the next block in the file, which is how the first version invented a hit. + if (!brace || brace[0] > args[1]) continue + const span = brace + const body = code.slice(span[0], span[1] + 1) + // A lifecycle hook nested inside would re-establish the instance; nothing else does. + const idx = body.search(/\bgetCurrentInstance\s*\(/) + if (idx < 0) continue + const before = body.slice(0, idx) + const nestedHook = [...before.matchAll(/\b(on[A-Z]\w+)\s*\(/g)].some((h) => HOOK_SET.test(h[1])) + if (!nestedHook) { + hits.push({ + line: lineOf(code, span[0] + idx), + why: 'getCurrentInstance() is called inside a watcher callback, where there is no current instance, so it returns null. Capture it in setup instead.', + }) + } + } + return hits +} + const run = (src, file) => { /* Strings are blanked HERE, for both arms. Arm B's first version flagged useMapInstance.js:430 — * where the only `onBeforeUnmount` on the line is inside the text of a console.error advising a * caller to use one. A gate that reads prose as code cries wolf, and a crying gate gets ignored. */ const code = stripStrings(stripComments(scriptOf(src, file))) - return [...armA(code).map((h) => ({ ...h, arm: 'IMMEDIATE-TDZ' })), ...armB(code).map((h) => ({ ...h, arm: 'HOOK-AFTER-AWAIT' }))] + return [...armA(code).map((h) => ({ ...h, arm: 'IMMEDIATE-TDZ' })), ...armB(code).map((h) => ({ ...h, arm: 'HOOK-AFTER-AWAIT' })), ...armC(code).map((h) => ({ ...h, arm: 'INSTANCE-OUTSIDE-SETUP' }))] } // ── self-test: both defects as they actually shipped, and the fixes, which must stay quiet ─────── @@ -299,6 +379,18 @@ function applyFitSlugs(slugs) { _lastFitKey = String(slugs) fit({ maxZoom: FIT_MAX_ZOOM, padding: 70 }) } +// ARM C's three legal shapes. The first is the fix; the second and third are CORRECT as written and +// are the reason this arm models Vue instead of banning the call — Vue sets the current instance for +// the duration of a lifecycle hook, so a call inside one resolves. research-map-plus-grid.vue:422 is +// the third shape, and flagging it would have been this gate crying wolf on working code. +const _selfInstance = getCurrentInstance() +function reflectDir() { + const host = _selfInstance?.proxy?.$el?.getRootNode?.()?.host + if (host) host.setAttribute('dir', 'rtl') +} +watch(textDir, reflectDir) +const inst = getCurrentInstance() +onMounted(() => { rootEl = getCurrentInstance()?.proxy?.$el?.getRootNode?.() || null }) let scrollRO = null onBeforeUnmount(() => { if (scrollRO) scrollRO.disconnect() }) onMounted(async () => { @@ -306,19 +398,45 @@ onMounted(async () => { scrollRO = new ResizeObserver(() => sync(map.value)) }) ` +const PLANTED_C = `` const blindA = run(PLANTED_A, 'x.vue').filter((h) => h.arm === 'IMMEDIATE-TDZ').length === 0 const blindB = run(PLANTED_B, 'x.vue').filter((h) => h.arm === 'HOOK-AFTER-AWAIT').length === 0 +const blindC = run(PLANTED_C, 'x.vue').filter((h) => h.arm === 'INSTANCE-OUTSIDE-SETUP').length === 0 const noisy = run(CLEAN, 'x.vue') -if (blindA || blindB || noisy.length) { +if (blindA || blindB || blindC || noisy.length) { console.error('SETUP-ORDER: SELF-TEST FAILED — ' + (blindA ? 'arm A is blind to the shipped TDZ. ' : '') + (blindB ? 'arm B is blind to the shipped late hook. ' : '') + + (blindC ? 'arm C is blind to the shipped getCurrentInstance-in-a-watcher. ' : '') + (noisy.length ? 'false positives on the fixed code: ' + noisy.map((h) => `${h.arm} ${h.why}`).join('; ') : '')) process.exit(3) } -if (POISON) { console.log('SETUP-ORDER: self-test passed (both defects caught, both fixes quiet).'); process.exit(0) } +if (POISON) { console.log('SETUP-ORDER: self-test passed (all three defects caught, every fix and legal shape quiet).'); process.exit(0) } // ── scan ───────────────────────────────────────────────────────────────────────────────────────── +// ★ ONE TRIGGER PER ARM, DERIVED — NOT A HAND-KEPT LIST, because a stale precondition voids every +// careful thing below it and does so SILENTLY. This is the second time in one day: check:i18n-keys-exist +// skipped any file that never wrote the word "tStatic", which is how a planted poison key passed it, +// and I diagnosed that and then wrote the same hole here within the hour — arm C went in and the +// early-out still only knew about arms A and B, so BOTH files carrying the live RTL bug were skipped +// before a single arm ran. The gate reported the tree clean and the self-test stayed green, because a +// fixture is text and never meets the file filter. +// The rule this encodes: a cheap early-out is part of a gate's COVERAGE, not an optimisation, so it +// has to be written where adding an arm forces you to extend it. +const ARM_TRIGGERS = [ + /\bimmediate\s*:\s*true\b/, // A: an immediate watcher runs during setup + /\basync\b/, // B: only an async body can put a hook after an await + /\bgetCurrentInstance\s*\(/, // C: the instance read that answers only during setup or a hook +] +const SCANNABLE = new RegExp(ARM_TRIGGERS.map((r) => r.source).join('|')) + const ROOTS = ['plugin-designs', 'library'] const SKIP = /(^|\/)(node_modules|dist|public|\.git|fixtures)(\/|$)/ function files(dir, acc = []) { @@ -340,17 +458,18 @@ for (const root of ROOTS) { for (const f of files(resolve(REPO, root))) { let src try { src = readFileSync(f, 'utf8') } catch (_) { continue } - if (!/\bimmediate\s*:\s*true\b|\basync\b/.test(src)) continue + if (!SCANNABLE.test(src)) continue for (const h of run(src, f)) found.push({ file: relative(REPO, f), ...h }) } } if (!found.length) { - console.log('SETUP-ORDER: OK — no immediate watcher reads below itself, and no lifecycle hook is registered after an await.') + console.log('SETUP-ORDER: OK — no immediate watcher reads below itself, no lifecycle hook is registered after an await, and no getCurrentInstance() is read from a watcher or a listener.') process.exit(0) } console.error(`SETUP-ORDER: ${found.length} defect(s) —`) for (const h of found) console.error(` ${h.arm} ${h.file}:${h.line} ${h.why}`) console.error('\nARM A: move the declaration above the watcher (a hoisted function resolves its NAME early, not its BODY).') console.error('ARM B: declare the handle at setup scope and tear it down in a hook registered BEFORE the first await.') +console.error('ARM C: capture the instance in setup (const _selfInstance = getCurrentInstance()); a call inside a hook is fine, inside a watcher it is null.') process.exit(1)