Release only the hooks this instance installed

251606460 dropped each component's window hooks on unmount, but deleted them
unconditionally, and that is wrong as soon as two of the same component share a
page -- which research-map-plus-grid and the Big Picture composite both arrange.
B mounts and overwrites A's hooks; A unmounts and deletes them; B is still on
screen with its verify hooks gone. Not a leak, and not visible to a reader: the
cost lands on rega, as a hook reported missing on a map that is perfectly
healthy, which is a more expensive thing to chase than this is to write.

people-groups-grid already compared identity before releasing. research-map (4
hooks) and the dashboard map panel (3) did not, so the commit was uneven with
itself; gospel's d3597f3d1 settled on the same identity-checked pattern, and one
pattern across the fleet is what lets the check enforce it later.

Each hook is now held in a local as it is installed -- window.__x = _hookX = (…)
-- declared outside the block that assigns it, because a const in there is
invisible to onBeforeUnmount. The release compares before deleting, one key per
line, so the check can read both halves.

The check reports how many releases are identity-checked and enforces it under
STRICT_IDENTITY=1, which is off only because other owners have not adopted the
pattern yet. Flipping it later is a smaller change than re-deriving this.

Two things it taught me while I wrote it. Enforcement first flagged the grid,
which WAS identity-checked -- the comparison simply sat on the line above the
delete, and looking only at the delete's own line dictated formatting instead of
recognising correct code; the window now covers the lines leading up to it. And
the pattern is tested for BEHAVIOUR, not presence: two hooks installed in turn,
the first unmount must leave the second working, the second must leave nothing.
This commit is contained in:
Doxa Maps 2026-09-28 20:02:57 +00:00
parent ba54af0113
commit 3ea48ebb42
3 changed files with 69 additions and 19 deletions

View file

@ -32,6 +32,11 @@ const ROOT = resolve(dirname(fileURLToPath(import.meta.url)), '..')
const fails = []
// Globals owned by somebody else: we may read or even set them, but we do not control their life.
// Identity checking is REPORTED by default and ENFORCED with STRICT_IDENTITY=1. It is not on by
// default only because other owners' plugins have not adopted it yet; flip it once they have, which
// is a smaller change than re-finding this reasoning later.
const STRICT = process.env.STRICT_IDENTITY === '1'
const NOT_OURS = new Set(['__VUE_INSTANCE_SETTERS__', '__VUE_DEVTOOLS_GLOBAL_HOOK__', '__vite_plugin_react_preamble_installed__'])
const walk = (dir, out = []) => {
@ -51,7 +56,7 @@ const walk = (dir, out = []) => {
const isCommentLine = (l) => /^\s*(\/\/|\*|\/\*)/.test(l)
const files = [...walk(resolve(ROOT, 'library')), ...walk(resolve(ROOT, 'plugin-designs'))]
let checked = 0, owned = 0
let checked = 0, owned = 0, identityChecked = 0
for (const file of files) {
const raw = readFileSync(file, 'utf8')
@ -60,6 +65,7 @@ for (const file of files) {
const lines = raw.split('\n')
const set = new Map() // name -> [lines]
const released = new Set()
const guarded = new Set()
lines.forEach((line, i) => {
if (isCommentLine(line)) return
// an assignment TO the global (not a comparison, not a read)
@ -82,11 +88,28 @@ for (const file of files) {
if (!set.has(name)) set.set(name, [])
set.get(name).push(i + 1)
}
for (const m of line.matchAll(/delete\s+window\s*\.\s*(__[\w$]+)/g)) released.add(m[1])
for (const m of line.matchAll(/delete\s+window\s*\.\s*(__[\w$]+)/g)) {
released.add(m[1])
// An identity-checked release only drops what THIS instance installed:
// if (window.__x === _hookX) delete window.__x
// Without it, unmounting one of two instances on a page strips the live one's hook, which
// reads as a missing hook on a healthy component rather than as a leak.
// The comparison need not share the line with the delete: an `if (…) {` on the line above is
// the same guard and just as readable. Looking only at the delete's own line reported a file
// that WAS identity-checked, so the window covers the few lines leading up to it. A check
// must not quietly dictate formatting in order to recognise correct code.
const near = lines.slice(Math.max(0, i - 3), i + 1).join('\n')
if (new RegExp(`window\\s*\\.\\s*${m[1]}\\s*===`).test(near)) guarded.add(m[1])
}
})
for (const [name, at] of set) {
owned++
if (released.has(name)) continue
if (released.has(name)) {
if (guarded.has(name)) identityChecked++
else if (STRICT) fails.push(`${relative(ROOT, file)}: releases window.${name} WITHOUT an identity check\n` +
` use \`if (window.${name} === _hook) delete window.${name}\` so one instance unmounting cannot strip another's hook`)
continue
}
fails.push(`${relative(ROOT, file)}: sets window.${name} (line${at.length > 1 ? 's' : ''} ${at.join(', ')}) and never releases it\n` +
` add \`delete window.${name}\` (or \`window.${name} = null\`) to this file's unmount path\n` +
` a window key holding a closure keeps the component, its shadow root and the whole app alive after unmount`)
@ -100,4 +123,5 @@ if (fails.length) {
for (const f of fails) console.error(` ✗ ${f}`)
process.exit(1)
}
console.log(`WINDOW-GLOBALS-RELEASED: PASS — ${owned} component-owned window global(s) across ${checked} file(s) are released by the file that sets them.`)
console.log(`WINDOW-GLOBALS-RELEASED: PASS — ${owned} component-owned window global(s) across ${checked} file(s) are released by the file that sets them; ` +
`${identityChecked}/${owned} release only what that instance installed` + (STRICT ? ' (identity checking ENFORCED).' : '. Run with STRICT_IDENTITY=1 to require it.'))

View file

@ -2408,8 +2408,20 @@ watch(() => profileConfig?.value?.externalFilters, (raw) => {
// resources-map's window.__grMap. A linked filter is a STATE claim ("the other half adopted it"),
// and every DOM proxy for it that I tried was either tab-specific or a selector guess: the honest
// way to check is to read the state itself. Reports the composite-driven facet only.
//
// ★ EACH HOOK IS ALSO HELD LOCALLY, so the unmount path can release ONLY WHAT IS STILL OURS.
// Two of these maps can share a page (research-map-plus-grid composes one beside a grid, and the
// gallery can show another): B mounts and overwrites A's hooks, then A unmounts. An unconditional
// delete would strip the hooks belonging to the map still on screen, and rega would report a verify
// hook missing on a map that is perfectly healthy — a false failure costing more to chase than this
// costs to write. Declared out here rather than inside the block below because a const inside it is
// invisible to onBeforeUnmount. Same pattern gospel used in d3597f3d1.
let _hookFilterState = null
let _hookPinOpacity = null
let _hookClusterDraw = null
let _hookVocabulary = null
if (typeof window !== 'undefined') {
window.__researchMapFilterState = () => ({
window.__researchMapFilterState = _hookFilterState = () => ({
selectedRegion: mapStore?.selectedRegion || '',
externalRegion: profileConfig?.value?.externalFilters?.region || '',
religionFamily: RELIGION_FAMILIES[uiStore?.religionFilter] || '',
@ -2433,11 +2445,11 @@ if (typeof window !== 'undefined') {
// and legend dimming COMPOSE into one paint property, and "the store value changed" is not evidence
// that they composed. Returns whatever GL is holding — a number when nothing is dimmed, a ['case',…]
// when something is.
window.__researchMapPinOpacity = () => {
window.__researchMapPinOpacity = _hookPinOpacity = () => {
try { return map.value?.getPaintProperty?.('language-family-pins', 'circle-opacity') ?? null } catch (_) { return null }
}
window.__researchMapClusterDraw = () => {
window.__researchMapClusterDraw = _hookClusterDraw = () => {
const m = map.value
const out = {}
try {
@ -2451,7 +2463,7 @@ if (typeof window !== 'undefined') {
return out
}
window.__researchMapVocabulary = (field) => {
window.__researchMapVocabulary = _hookVocabulary = (field) => {
const rows = mapData?.normalizedPeopleGroups?.value || []
const seen = new Set()
for (const r of rows) {
@ -3096,12 +3108,15 @@ onBeforeUnmount(() => {
// check-window-globals-released matches `delete window.__x`; a computed `delete window[k]` is
// invisible to it, and a release a checker cannot see is worth as much as no release at all the
// day someone deletes three of the four. Legibility to the tool is the point here, not brevity.
// Identity-checked: release only the hooks THIS instance installed, so unmounting one map on a
// two-map page cannot strip the hooks of the one still running. Written one key per line, not
// looped, so check-window-globals-released can read both the delete and the comparison.
step(() => {
if (typeof window === 'undefined') return
delete window.__researchMapFilterState
delete window.__researchMapPinOpacity
delete window.__researchMapClusterDraw
delete window.__researchMapVocabulary
if (window.__researchMapFilterState === _hookFilterState) delete window.__researchMapFilterState
if (window.__researchMapPinOpacity === _hookPinOpacity) delete window.__researchMapPinOpacity
if (window.__researchMapClusterDraw === _hookClusterDraw) delete window.__researchMapClusterDraw
if (window.__researchMapVocabulary === _hookVocabulary) delete window.__researchMapVocabulary
})
} finally {
destroy()

View file

@ -204,6 +204,15 @@ let containerRO = null
let scrollRO = null
let scrollResync = null
let offToggleTable = null
// Holders for the three verify hooks planted on window when the map is built, so the unmount path
// can release ONLY the ones this instance installed. Two dashboards can share a page (the Big
// Picture composite beside a gallery instance); B overwrites A's hooks, then A unmounts, and an
// unconditional delete would strip the hooks of the dashboard still on screen — rega would see a
// missing hook on a healthy map. Declared here because the assignments live inside the map-build
// function and a const there is invisible to onBeforeUnmount.
let _hookRdashMap = null
let _hookClickAt = null
let _hookPinAt = null
let offLayoutResizing = null
let _resizeRaf = 0
// BUG-RESIZE-STROBE (photosensitive-seizure risk): while a section divider is being DRAGGED the
@ -1065,16 +1074,16 @@ function addLayers() {
m.on('mouseenter', 'rd-bubble-layer', () => { m.getCanvas().style.cursor = 'pointer' })
m.on('mouseleave', 'rd-bubble-layer', () => { m.getCanvas().style.cursor = '' })
try {
window.__rdashMap = m
window.__rdashMap = _hookRdashMap = m
// verify hook: simulate a country click at a lng/lat (query the choropleth at that point)
window.__rdashClickAt = (lng, lat) => {
window.__rdashClickAt = _hookClickAt = (lng, lat) => {
const pt = m.project([lng, lat])
const feats = m.queryRenderedFeatures(pt, { layers: ['rd-choropleth'] })
if (feats && feats[0]) { onMapClick({ features: feats }); return feats[0].properties && feats[0].properties.iso_3166_1_alpha_3 }
return null
}
// verify hook: select the PG pin nearest a lng/lat (small bbox query around the projected point)
window.__rdashPinAt = (lng, lat) => {
window.__rdashPinAt = _hookPinAt = (lng, lat) => {
const pt = m.project([lng, lat])
const feats = m.queryRenderedFeatures([[pt.x - 7, pt.y - 7], [pt.x + 7, pt.y + 7]], { layers: ['rd-bubble-layer'] })
if (feats && feats[0]) { onBubbleClick({ features: [feats[0]] }); return feats[0].properties && feats[0].properties.slug }
@ -1294,12 +1303,14 @@ onBeforeUnmount(() => {
// Released as a set: __rdashPinAt was the one that showed up in the snapshot, but __rdashMap holds
// the map and retains strictly more, and clearing only the reported one would have left the larger
// retainer in place and looked fixed.
// One per line rather than a loop, so check-window-globals-released can actually read the release.
// Identity-checked, and one key per line rather than a loop, so check-window-globals-released can
// read both the delete and the comparison. Releasing only what this instance installed is what
// keeps a two-dashboard page from losing the live one's hooks when the other unmounts.
try {
if (typeof window !== 'undefined') {
delete window.__rdashPinAt
delete window.__rdashClickAt
delete window.__rdashMap
if (window.__rdashPinAt === _hookPinAt) delete window.__rdashPinAt
if (window.__rdashClickAt === _hookClickAt) delete window.__rdashClickAt
if (window.__rdashMap === _hookRdashMap) delete window.__rdashMap
}
} catch (_) { /* a locked-down window is not worth failing an unmount over */ }
// Drop the pending selection fly FIRST: its debounce timer outlives the component, and a