diff --git a/CHANGELOG.md b/CHANGELOG.md index 6ac5e4eb..a26dbaed 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,15 @@ The format is loosely based on [Keep a Changelog](https://keepachangelog.com). ## [Unreleased] +### Fixed + +- Exit-animated unmounts (e.g. every Toast dismissal) no longer leak a + driver listener entry per handler: the `animateExit` opcode now runs the + same listener-eviction sweep as `destroyNode`, and detaches immediately so + clicks during the exit window can't dispatch into evicted handlers. +- The dev-mode `window.__swiflow` API no longer re-creates its four closures + on every render/unmount (each re-install pinned the prior set forever). + ### Removed - **BREAKING:** the `HTTP` static facade (`HTTP.get/post/put/patch/delete/send`) diff --git a/Sources/SwiflowCLI/EmbeddedDriver.swift b/Sources/SwiflowCLI/EmbeddedDriver.swift index 49a2676a..42bbe162 100644 --- a/Sources/SwiflowCLI/EmbeddedDriver.swift +++ b/Sources/SwiflowCLI/EmbeddedDriver.swift @@ -148,6 +148,27 @@ enum EmbeddedDriver { "stop", "marker", "clipPath", "mask", "pattern", "use", "symbol", ]); + /** + * Detach every tracked listener for `handle` from `node` and drop the + * `listeners` map entries. Shared by `destroyNode` and `animateExit` — + * both destruction paths must run the same sweep or entries leak. The + * handle prefix is parsed numerically: a startsWith match would hit + * handle 1 against keys for 10/11/etc. + */ + function detachListeners(handle, node) { + for (const key of Array.from(listeners.keys())) { + const sep = key.indexOf(":"); + if (sep < 0) continue; + if (Number(key.slice(0, sep)) !== handle) continue; + const event = key.slice(sep + 1); + const fn = listeners.get(key); + if (node !== undefined && fn !== undefined) { + node.removeEventListener(event, fn); + } + listeners.delete(key); + } + } + /** * Apply a single patch. The opcode is `p.op`; field names match the * Swift-side `PatchSerializer.encode(...)` contract. @@ -175,24 +196,7 @@ enum EmbeddedDriver { case "destroyNode": { // Symmetric with removeHandler: detach every tracked listener for // this handle from the DOM node AND drop the map entries. - // Previously this case only deleted from the map, leaving the DOM - // bindings until GC -- mostly harmless but inconsistent with - // removeHandler and falsified the comment that claimed "Detach any - // listeners". Also: parse the handle prefix numerically (was - // startsWith-based, which would match handle 1 against keys for - // 10/11/etc. once handles cross 10). - const node = nodes.get(p.handle); - for (const key of Array.from(listeners.keys())) { - const sep = key.indexOf(":"); - if (sep < 0) continue; - if (Number(key.slice(0, sep)) !== p.handle) continue; - const event = key.slice(sep + 1); - const fn = listeners.get(key); - if (node !== undefined && fn !== undefined) { - node.removeEventListener(event, fn); - } - listeners.delete(key); - } + detachListeners(p.handle, nodes.get(p.handle)); nodes.delete(p.handle); return; } @@ -200,6 +204,12 @@ enum EmbeddedDriver { const node = nodes.get(p.handle); const parent = nodes.get(p.parentHandle); if (!node) return; + // The Swift side evicts this node's handler ids when it emits the + // patch (destroyNode is suppressed for the animated root), so its + // listeners must be detached NOW — kept through the animation + // window they dispatch into evicted ids and their map entries + // outlive the node. + detachListeners(p.handle, node); node.style.animation = p.animation; setTimeout(function () { if (parent && node.parentNode === parent) { @@ -893,13 +903,13 @@ enum EmbeddedDriver { """# static let javascriptSourceMinified: String = #""" -(function(){"use strict";const o=new Map,m=new Map;let y=null;const x=new Map,_="./.build/plugins/PackageToJS/outputs/Package/App.wasm";function b(e){const t=document.createElement("template");if(t.innerHTML=e,t.content.childNodes.length===1)return t.content.firstChild;const n=document.createElement("span");for(;t.content.firstChild;)n.appendChild(t.content.firstChild);return n}function k(e){const t=e.target,n=t instanceof Element?t:t&&t.parentElement||null;if(!n||!n.closest||!e.currentTarget)return!1;const r=n.closest("a[href],button,input,select,textarea,summary,label,[contenteditable]:not([contenteditable='false'])");return!!(r&&r!==e.currentTarget&&e.currentTarget.contains&&e.currentTarget.contains(r))}function C(e){const t=e.target,n=t&&"value"in t?String(t.value):null,r=t&&"checked"in t?!!t.checked:null,l=typeof e.key=="string"?e.key:null;return{type:e.type,targetValue:n,targetChecked:r,isSelfTarget:t===e.currentTarget,fromInteractiveDescendant:k(e),key:l,shiftKey:!!e.shiftKey,ctrlKey:!!e.ctrlKey,altKey:!!e.altKey,metaKey:!!e.metaKey,detail:e.detail!==null&&typeof e.detail=="object"?JSON.stringify(e.detail):null}}const E=new Set(["svg","path","g","circle","ellipse","rect","line","polyline","polygon","text","tspan","defs","linearGradient","radialGradient","stop","marker","clipPath","mask","pattern","use","symbol"]);function T(e){switch(e.op){case"createElement":o.set(e.handle,E.has(e.tag)?document.createElementNS("http://www.w3.org/2000/svg",e.tag):document.createElement(e.tag));return;case"createText":o.set(e.handle,document.createTextNode(e.text));return;case"createRawHTML":{o.set(e.handle,b(e.html));return}case"destroyNode":{const t=o.get(e.handle);for(const n of Array.from(m.keys())){const r=n.indexOf(":");if(r<0||Number(n.slice(0,r))!==e.handle)continue;const l=n.slice(r+1),w=m.get(n);t!==void 0&&w!==void 0&&t.removeEventListener(l,w),m.delete(n)}o.delete(e.handle);return}case"animateExit":{const t=o.get(e.handle),n=o.get(e.parentHandle);if(!t)return;t.style.animation=e.animation,setTimeout(function(){n&&t.parentNode===n?n.removeChild(t):t.parentNode&&t.parentNode.removeChild(t),o.delete(e.handle)},e.durationMs);return}case"appendChild":o.get(e.parent).appendChild(o.get(e.child));return;case"insertBefore":o.get(e.parent).insertBefore(o.get(e.child),o.get(e.beforeChild));return;case"removeChild":o.get(e.parent).removeChild(o.get(e.child));return;case"setAttribute":o.get(e.handle).setAttribute(e.name,e.value);return;case"removeAttribute":o.get(e.handle).removeAttribute(e.name);return;case"setProperty":if(e.name==="innerHTML")throw new Error("swiflow: setProperty refuses to write the innerHTML property; use VNode.rawHTML(_:) instead");o.get(e.handle)[e.name]=e.value;return;case"removeProperty":delete o.get(e.handle)[e.name];return;case"setStyle":e.name.startsWith("--")?o.get(e.handle).style.setProperty(e.name,e.value):o.get(e.handle).style[e.name]=e.value;return;case"removeStyle":o.get(e.handle).style.removeProperty(e.name);return;case"setText":{const t=o.get(e.handle);t.data!==void 0?t.data=e.text:t.textContent=e.text;return}case"setRawHTML":{const t=b(e.html),n=o.get(e.handle);n&&n.parentNode&&n.parentNode.replaceChild(t,n),o.set(e.handle,t);return}case"addHandler":{const t=e.handlerId,n=e.handle+":"+e.event,r=m.get(n);r!==void 0&&o.get(e.handle).removeEventListener(e.event,r);const l=function(w){window.__swiflowDispatch(t,C(w))};o.get(e.handle).addEventListener(e.event,l),m.set(n,l);return}case"removeHandler":{const t=e.handle+":"+e.event,n=m.get(t);n!==void 0&&(o.get(e.handle).removeEventListener(e.event,n),m.delete(t));return}case"replaceMount":{const t=document.querySelector(e.selector);if(t===null)throw new Error("swiflow-driver: replaceMount target '"+e.selector+"' not found");const n=x.get(e.selector);n!==void 0&&n.parentNode===t&&t.removeChild(n);const r=o.get(e.newHandle);x.set(e.selector,r),t.appendChild(r);return}default:console.error("swiflow-driver: unknown opcode",e.op,e);return}}async function v(e){const t=await fetch(e);if(!t.ok)throw new Error("swiflow: fetch "+e+" failed ("+t.status+")");const n=parseInt(t.headers.get("Content-Length")||"",10),r=t.body&&t.body.getReader?t.body.getReader():null;if(!r)return document.documentElement.dataset.swiflowProgress="100",t;const l=[];let w=0;const h=Number.isFinite(n)&&n>0;try{for(;;){const{done:i,value:s}=await r.read();if(i)break;if(l.push(s),w+=s.byteLength,h){const a=Math.floor(w/n*100);document.documentElement.dataset.swiflowProgress=String(Math.min(a,99))}}}catch(i){try{await r.cancel()}catch{}throw i}document.documentElement.dataset.swiflowProgress="100";const g=new Blob(l,{type:t.headers.get("Content-Type")||"application/wasm"});return new Response(g,{headers:t.headers,status:t.status})}if(window.swiflow={applyPatches:function(e){let t=!0;for(let n=0;n0;try{for(;;){const{done:i,value:s}=await r.read();if(i)break;if(l.push(s),u+=s.byteLength,h){const a=Math.floor(u/n*100);document.documentElement.dataset.swiflowProgress=String(Math.min(a,99))}}}catch(i){try{await r.cancel()}catch{}throw i}document.documentElement.dataset.swiflowProgress="100";const g=new Blob(l,{type:t.headers.get("Content-Type")||"application/wasm"});return new Response(g,{headers:t.headers,status:t.status})}if(window.swiflow={applyPatches:function(e){let t=!0;for(let n=0;nimport(d)),{init:u}=await c(i.jsURL);await u({module:v(i.wasmURL)});const f=(performance.now()-s).toFixed(1);console.log("[swiflow] hmr-swap took "+f+"ms")}catch(a){console.warn("[swiflow] HMR swap failed, falling back to full reload:",a),location.reload();return}finally{l=!1}if(w!==null){const a=w;w=null,g(a)}}h()}window.swiflow.__boot=async function({swiflowDev:t}){if("serviceWorker"in navigator){if(t){const n=await navigator.serviceWorker.getRegistrations();for(const r of n)if(((r.active||r.installing||r.waiting)?.scriptURL??"").endsWith("/swiflow-service-worker.js"))try{await r.unregister()}catch{}if(typeof caches<"u")try{const r=await caches.keys();await Promise.all(r.filter(l=>l.startsWith("swiflow-")).map(l=>caches.delete(l)))}catch{}return}try{await navigator.serviceWorker.register("swiflow-service-worker.js")}catch(n){console.warn("swiflow: service worker registration failed",n)}}},window.swiflow.__bootForTest=window.swiflow.__boot,window.swiflow.__test_fetchWithProgress=v,(async()=>{if(!window.__SWIFLOW_SKIP_BOOT&&(await window.swiflow.__boot({swiflowDev:!!window.SWIFLOW_DEV}),!window.swiflow.__inited)){window.swiflow.__inited=!0;try{const{init:e}=await import("./.build/plugins/PackageToJS/outputs/Package/index.js");await e({module:v(_)})}catch(e){console.error("swiflow: WASM init failed",e)}}})()})(); +`+(i&&i.stack?i.stack:String(i)));const s=document.getElementById("__swiflow-error-overlay");s&&s.remove();const a=document.createElement("div");a.id="__swiflow-error-overlay",a.style.cssText="position:fixed;inset:0;z-index:999999;background:rgba(0,0,0,0.85);color:#fff;font-family:monospace;font-size:14px;padding:24px;overflow:auto;white-space:pre-wrap;word-break:break-word;";const c=document.createElement("div");c.style.cssText="font-size:18px;font-weight:bold;margin-bottom:16px;color:#ff6b6b;",c.textContent="\u26A0 Swiflow render error \u2014 WASM execution stopped";const w=document.createElement("pre");w.style.cssText="margin:0 0 16px;",w.textContent=i&&i.stack?i.stack:String(i);const f=document.createElement("div");f.style.cssText="color:#aaa;font-size:12px;margin-bottom:4px;",f.textContent="Install the Chrome C/C++ DevTools Extension to see Swift file:line in the stack above:";const d=document.createElement("a");d.href="https://goo.gle/wasm-debugging-extension",d.target="_blank",d.style.cssText="color:#4dabf7;font-size:12px;display:block;margin-bottom:16px;",d.textContent="https://goo.gle/wasm-debugging-extension";const p=document.createElement("button");p.style.cssText="padding:8px 16px;background:#444;color:#fff;border:none;cursor:pointer;font-size:14px;border-radius:4px;",p.textContent="Dismiss (app is frozen \u2014 reload to continue)",p.onclick=function(){a.remove()},a.appendChild(c),a.appendChild(w),a.appendChild(f),a.appendChild(d),a.appendChild(p),(document.body||document.documentElement).appendChild(a)},typeof window.requestAnimationFrame=="function"){var L=window.requestAnimationFrame.bind(window);window.requestAnimationFrame=function(i){return L(function(s){try{i(s)}catch(a){window.__swiflowDevError(a)}})}}let n=250;const r=5e3;let l=!1,u=null;async function g(i){if(l){u=i;return}l=!0;const s=performance.now();try{const a=window.__swiflow&&window.__swiflow.hmrSnapshot?window.__swiflow.hmrSnapshot():null;window.__swiflowPendingSnapshot=a;try{window.__swiflow&&typeof window.__swiflow.hmrTeardown=="function"&&window.__swiflow.hmrTeardown()}catch(d){console.warn("[swiflow] hmr teardown of previous module failed:",d)}if(o.clear(),m.clear(),y){const d=document.querySelector(y);d&&d.replaceChildren()}document.querySelectorAll('style[id^="swiflow-"]').forEach(function(d){d.remove()});const c=window.swiflow&&window.swiflow.__importOverride||(d=>import(d)),{init:w}=await c(i.jsURL);await w({module:v(i.wasmURL)});const f=(performance.now()-s).toFixed(1);console.log("[swiflow] hmr-swap took "+f+"ms")}catch(a){console.warn("[swiflow] HMR swap failed, falling back to full reload:",a),location.reload();return}finally{l=!1}if(u!==null){const a=u;u=null,g(a)}}h()}window.swiflow.__boot=async function({swiflowDev:t}){if("serviceWorker"in navigator){if(t){const n=await navigator.serviceWorker.getRegistrations();for(const r of n)if(((r.active||r.installing||r.waiting)?.scriptURL??"").endsWith("/swiflow-service-worker.js"))try{await r.unregister()}catch{}if(typeof caches<"u")try{const r=await caches.keys();await Promise.all(r.filter(l=>l.startsWith("swiflow-")).map(l=>caches.delete(l)))}catch{}return}try{await navigator.serviceWorker.register("swiflow-service-worker.js")}catch(n){console.warn("swiflow: service worker registration failed",n)}}},window.swiflow.__bootForTest=window.swiflow.__boot,window.swiflow.__test_fetchWithProgress=v,(async()=>{if(!window.__SWIFLOW_SKIP_BOOT&&(await window.swiflow.__boot({swiflowDev:!!window.SWIFLOW_DEV}),!window.swiflow.__inited)){window.swiflow.__inited=!0;try{const{init:e}=await import("./.build/plugins/PackageToJS/outputs/Package/index.js");await e({module:v(k)})}catch(e){console.error("swiflow: WASM init failed",e)}}})()})(); """# diff --git a/Sources/SwiflowDOM/DevAPI.swift b/Sources/SwiflowDOM/DevAPI.swift index b99abf60..3dfb59cb 100644 --- a/Sources/SwiflowDOM/DevAPI.swift +++ b/Sources/SwiflowDOM/DevAPI.swift @@ -31,9 +31,10 @@ enum DevAPI { // MARK: - Install - /// Installs (or re-installs) `window.__swiflow` commands pointing at all - /// currently mounted roots. Called after every `render(into:)` and - /// `unmount(into:)` so the API always reflects the live root set. + /// Installs `window.__swiflow` commands. Idempotent: called after every + /// `render(into:)` and `unmount(into:)`, but only the first call creates + /// the closures — they read the live `renderers` set on each invocation, + /// so the API reflects the current roots without re-installation. /// /// All four commands return JS objects keyed by selector when multiple /// roots are mounted, and return the same structure for a single root so @@ -41,6 +42,12 @@ enum DevAPI { @MainActor static func installAll() { guard JSObject.global.SWIFLOW_DEV.boolean == true else { return } + // Install once: every closure reads the live `renderers` static at + // call time, so one install stays correct as roots mount/unmount — + // and re-creating the closures would pin the prior four in + // JavaScriptKit's static sharedClosures table forever (nil-ing a + // JSClosure field never releases it). + guard treeClosure == nil else { return } let ns = swiflowDevNamespace() diff --git a/examples/AsyncFetch/swiflow-driver.js b/examples/AsyncFetch/swiflow-driver.js index e65e14f4..51ed49a2 100644 --- a/examples/AsyncFetch/swiflow-driver.js +++ b/examples/AsyncFetch/swiflow-driver.js @@ -138,6 +138,27 @@ "stop", "marker", "clipPath", "mask", "pattern", "use", "symbol", ]); + /** + * Detach every tracked listener for `handle` from `node` and drop the + * `listeners` map entries. Shared by `destroyNode` and `animateExit` — + * both destruction paths must run the same sweep or entries leak. The + * handle prefix is parsed numerically: a startsWith match would hit + * handle 1 against keys for 10/11/etc. + */ + function detachListeners(handle, node) { + for (const key of Array.from(listeners.keys())) { + const sep = key.indexOf(":"); + if (sep < 0) continue; + if (Number(key.slice(0, sep)) !== handle) continue; + const event = key.slice(sep + 1); + const fn = listeners.get(key); + if (node !== undefined && fn !== undefined) { + node.removeEventListener(event, fn); + } + listeners.delete(key); + } + } + /** * Apply a single patch. The opcode is `p.op`; field names match the * Swift-side `PatchSerializer.encode(...)` contract. @@ -165,24 +186,7 @@ case "destroyNode": { // Symmetric with removeHandler: detach every tracked listener for // this handle from the DOM node AND drop the map entries. - // Previously this case only deleted from the map, leaving the DOM - // bindings until GC -- mostly harmless but inconsistent with - // removeHandler and falsified the comment that claimed "Detach any - // listeners". Also: parse the handle prefix numerically (was - // startsWith-based, which would match handle 1 against keys for - // 10/11/etc. once handles cross 10). - const node = nodes.get(p.handle); - for (const key of Array.from(listeners.keys())) { - const sep = key.indexOf(":"); - if (sep < 0) continue; - if (Number(key.slice(0, sep)) !== p.handle) continue; - const event = key.slice(sep + 1); - const fn = listeners.get(key); - if (node !== undefined && fn !== undefined) { - node.removeEventListener(event, fn); - } - listeners.delete(key); - } + detachListeners(p.handle, nodes.get(p.handle)); nodes.delete(p.handle); return; } @@ -190,6 +194,12 @@ const node = nodes.get(p.handle); const parent = nodes.get(p.parentHandle); if (!node) return; + // The Swift side evicts this node's handler ids when it emits the + // patch (destroyNode is suppressed for the animated root), so its + // listeners must be detached NOW — kept through the animation + // window they dispatch into evicted ids and their map entries + // outlive the node. + detachListeners(p.handle, node); node.style.animation = p.animation; setTimeout(function () { if (parent && node.parentNode === parent) { diff --git a/examples/EdgeCases/swiflow-driver.js b/examples/EdgeCases/swiflow-driver.js index e65e14f4..51ed49a2 100644 --- a/examples/EdgeCases/swiflow-driver.js +++ b/examples/EdgeCases/swiflow-driver.js @@ -138,6 +138,27 @@ "stop", "marker", "clipPath", "mask", "pattern", "use", "symbol", ]); + /** + * Detach every tracked listener for `handle` from `node` and drop the + * `listeners` map entries. Shared by `destroyNode` and `animateExit` — + * both destruction paths must run the same sweep or entries leak. The + * handle prefix is parsed numerically: a startsWith match would hit + * handle 1 against keys for 10/11/etc. + */ + function detachListeners(handle, node) { + for (const key of Array.from(listeners.keys())) { + const sep = key.indexOf(":"); + if (sep < 0) continue; + if (Number(key.slice(0, sep)) !== handle) continue; + const event = key.slice(sep + 1); + const fn = listeners.get(key); + if (node !== undefined && fn !== undefined) { + node.removeEventListener(event, fn); + } + listeners.delete(key); + } + } + /** * Apply a single patch. The opcode is `p.op`; field names match the * Swift-side `PatchSerializer.encode(...)` contract. @@ -165,24 +186,7 @@ case "destroyNode": { // Symmetric with removeHandler: detach every tracked listener for // this handle from the DOM node AND drop the map entries. - // Previously this case only deleted from the map, leaving the DOM - // bindings until GC -- mostly harmless but inconsistent with - // removeHandler and falsified the comment that claimed "Detach any - // listeners". Also: parse the handle prefix numerically (was - // startsWith-based, which would match handle 1 against keys for - // 10/11/etc. once handles cross 10). - const node = nodes.get(p.handle); - for (const key of Array.from(listeners.keys())) { - const sep = key.indexOf(":"); - if (sep < 0) continue; - if (Number(key.slice(0, sep)) !== p.handle) continue; - const event = key.slice(sep + 1); - const fn = listeners.get(key); - if (node !== undefined && fn !== undefined) { - node.removeEventListener(event, fn); - } - listeners.delete(key); - } + detachListeners(p.handle, nodes.get(p.handle)); nodes.delete(p.handle); return; } @@ -190,6 +194,12 @@ const node = nodes.get(p.handle); const parent = nodes.get(p.parentHandle); if (!node) return; + // The Swift side evicts this node's handler ids when it emits the + // patch (destroyNode is suppressed for the animated root), so its + // listeners must be detached NOW — kept through the animation + // window they dispatch into evicted ids and their map entries + // outlive the node. + detachListeners(p.handle, node); node.style.animation = p.animation; setTimeout(function () { if (parent && node.parentNode === parent) { diff --git a/examples/HelloWorld/swiflow-driver.js b/examples/HelloWorld/swiflow-driver.js index e65e14f4..51ed49a2 100644 --- a/examples/HelloWorld/swiflow-driver.js +++ b/examples/HelloWorld/swiflow-driver.js @@ -138,6 +138,27 @@ "stop", "marker", "clipPath", "mask", "pattern", "use", "symbol", ]); + /** + * Detach every tracked listener for `handle` from `node` and drop the + * `listeners` map entries. Shared by `destroyNode` and `animateExit` — + * both destruction paths must run the same sweep or entries leak. The + * handle prefix is parsed numerically: a startsWith match would hit + * handle 1 against keys for 10/11/etc. + */ + function detachListeners(handle, node) { + for (const key of Array.from(listeners.keys())) { + const sep = key.indexOf(":"); + if (sep < 0) continue; + if (Number(key.slice(0, sep)) !== handle) continue; + const event = key.slice(sep + 1); + const fn = listeners.get(key); + if (node !== undefined && fn !== undefined) { + node.removeEventListener(event, fn); + } + listeners.delete(key); + } + } + /** * Apply a single patch. The opcode is `p.op`; field names match the * Swift-side `PatchSerializer.encode(...)` contract. @@ -165,24 +186,7 @@ case "destroyNode": { // Symmetric with removeHandler: detach every tracked listener for // this handle from the DOM node AND drop the map entries. - // Previously this case only deleted from the map, leaving the DOM - // bindings until GC -- mostly harmless but inconsistent with - // removeHandler and falsified the comment that claimed "Detach any - // listeners". Also: parse the handle prefix numerically (was - // startsWith-based, which would match handle 1 against keys for - // 10/11/etc. once handles cross 10). - const node = nodes.get(p.handle); - for (const key of Array.from(listeners.keys())) { - const sep = key.indexOf(":"); - if (sep < 0) continue; - if (Number(key.slice(0, sep)) !== p.handle) continue; - const event = key.slice(sep + 1); - const fn = listeners.get(key); - if (node !== undefined && fn !== undefined) { - node.removeEventListener(event, fn); - } - listeners.delete(key); - } + detachListeners(p.handle, nodes.get(p.handle)); nodes.delete(p.handle); return; } @@ -190,6 +194,12 @@ const node = nodes.get(p.handle); const parent = nodes.get(p.parentHandle); if (!node) return; + // The Swift side evicts this node's handler ids when it emits the + // patch (destroyNode is suppressed for the animated root), so its + // listeners must be detached NOW — kept through the animation + // window they dispatch into evicted ids and their map entries + // outlive the node. + detachListeners(p.handle, node); node.style.animation = p.animation; setTimeout(function () { if (parent && node.parentNode === parent) { diff --git a/examples/QueryDemo/swiflow-driver.js b/examples/QueryDemo/swiflow-driver.js index e65e14f4..51ed49a2 100644 --- a/examples/QueryDemo/swiflow-driver.js +++ b/examples/QueryDemo/swiflow-driver.js @@ -138,6 +138,27 @@ "stop", "marker", "clipPath", "mask", "pattern", "use", "symbol", ]); + /** + * Detach every tracked listener for `handle` from `node` and drop the + * `listeners` map entries. Shared by `destroyNode` and `animateExit` — + * both destruction paths must run the same sweep or entries leak. The + * handle prefix is parsed numerically: a startsWith match would hit + * handle 1 against keys for 10/11/etc. + */ + function detachListeners(handle, node) { + for (const key of Array.from(listeners.keys())) { + const sep = key.indexOf(":"); + if (sep < 0) continue; + if (Number(key.slice(0, sep)) !== handle) continue; + const event = key.slice(sep + 1); + const fn = listeners.get(key); + if (node !== undefined && fn !== undefined) { + node.removeEventListener(event, fn); + } + listeners.delete(key); + } + } + /** * Apply a single patch. The opcode is `p.op`; field names match the * Swift-side `PatchSerializer.encode(...)` contract. @@ -165,24 +186,7 @@ case "destroyNode": { // Symmetric with removeHandler: detach every tracked listener for // this handle from the DOM node AND drop the map entries. - // Previously this case only deleted from the map, leaving the DOM - // bindings until GC -- mostly harmless but inconsistent with - // removeHandler and falsified the comment that claimed "Detach any - // listeners". Also: parse the handle prefix numerically (was - // startsWith-based, which would match handle 1 against keys for - // 10/11/etc. once handles cross 10). - const node = nodes.get(p.handle); - for (const key of Array.from(listeners.keys())) { - const sep = key.indexOf(":"); - if (sep < 0) continue; - if (Number(key.slice(0, sep)) !== p.handle) continue; - const event = key.slice(sep + 1); - const fn = listeners.get(key); - if (node !== undefined && fn !== undefined) { - node.removeEventListener(event, fn); - } - listeners.delete(key); - } + detachListeners(p.handle, nodes.get(p.handle)); nodes.delete(p.handle); return; } @@ -190,6 +194,12 @@ const node = nodes.get(p.handle); const parent = nodes.get(p.parentHandle); if (!node) return; + // The Swift side evicts this node's handler ids when it emits the + // patch (destroyNode is suppressed for the animated root), so its + // listeners must be detached NOW — kept through the animation + // window they dispatch into evicted ids and their map entries + // outlive the node. + detachListeners(p.handle, node); node.style.animation = p.animation; setTimeout(function () { if (parent && node.parentNode === parent) { diff --git a/examples/RegionDemo/swiflow-driver.js b/examples/RegionDemo/swiflow-driver.js index e65e14f4..51ed49a2 100644 --- a/examples/RegionDemo/swiflow-driver.js +++ b/examples/RegionDemo/swiflow-driver.js @@ -138,6 +138,27 @@ "stop", "marker", "clipPath", "mask", "pattern", "use", "symbol", ]); + /** + * Detach every tracked listener for `handle` from `node` and drop the + * `listeners` map entries. Shared by `destroyNode` and `animateExit` — + * both destruction paths must run the same sweep or entries leak. The + * handle prefix is parsed numerically: a startsWith match would hit + * handle 1 against keys for 10/11/etc. + */ + function detachListeners(handle, node) { + for (const key of Array.from(listeners.keys())) { + const sep = key.indexOf(":"); + if (sep < 0) continue; + if (Number(key.slice(0, sep)) !== handle) continue; + const event = key.slice(sep + 1); + const fn = listeners.get(key); + if (node !== undefined && fn !== undefined) { + node.removeEventListener(event, fn); + } + listeners.delete(key); + } + } + /** * Apply a single patch. The opcode is `p.op`; field names match the * Swift-side `PatchSerializer.encode(...)` contract. @@ -165,24 +186,7 @@ case "destroyNode": { // Symmetric with removeHandler: detach every tracked listener for // this handle from the DOM node AND drop the map entries. - // Previously this case only deleted from the map, leaving the DOM - // bindings until GC -- mostly harmless but inconsistent with - // removeHandler and falsified the comment that claimed "Detach any - // listeners". Also: parse the handle prefix numerically (was - // startsWith-based, which would match handle 1 against keys for - // 10/11/etc. once handles cross 10). - const node = nodes.get(p.handle); - for (const key of Array.from(listeners.keys())) { - const sep = key.indexOf(":"); - if (sep < 0) continue; - if (Number(key.slice(0, sep)) !== p.handle) continue; - const event = key.slice(sep + 1); - const fn = listeners.get(key); - if (node !== undefined && fn !== undefined) { - node.removeEventListener(event, fn); - } - listeners.delete(key); - } + detachListeners(p.handle, nodes.get(p.handle)); nodes.delete(p.handle); return; } @@ -190,6 +194,12 @@ const node = nodes.get(p.handle); const parent = nodes.get(p.parentHandle); if (!node) return; + // The Swift side evicts this node's handler ids when it emits the + // patch (destroyNode is suppressed for the animated root), so its + // listeners must be detached NOW — kept through the animation + // window they dispatch into evicted ids and their map entries + // outlive the node. + detachListeners(p.handle, node); node.style.animation = p.animation; setTimeout(function () { if (parent && node.parentNode === parent) { diff --git a/examples/SwiflowUIDemo/swiflow-driver.js b/examples/SwiflowUIDemo/swiflow-driver.js index e65e14f4..51ed49a2 100644 --- a/examples/SwiflowUIDemo/swiflow-driver.js +++ b/examples/SwiflowUIDemo/swiflow-driver.js @@ -138,6 +138,27 @@ "stop", "marker", "clipPath", "mask", "pattern", "use", "symbol", ]); + /** + * Detach every tracked listener for `handle` from `node` and drop the + * `listeners` map entries. Shared by `destroyNode` and `animateExit` — + * both destruction paths must run the same sweep or entries leak. The + * handle prefix is parsed numerically: a startsWith match would hit + * handle 1 against keys for 10/11/etc. + */ + function detachListeners(handle, node) { + for (const key of Array.from(listeners.keys())) { + const sep = key.indexOf(":"); + if (sep < 0) continue; + if (Number(key.slice(0, sep)) !== handle) continue; + const event = key.slice(sep + 1); + const fn = listeners.get(key); + if (node !== undefined && fn !== undefined) { + node.removeEventListener(event, fn); + } + listeners.delete(key); + } + } + /** * Apply a single patch. The opcode is `p.op`; field names match the * Swift-side `PatchSerializer.encode(...)` contract. @@ -165,24 +186,7 @@ case "destroyNode": { // Symmetric with removeHandler: detach every tracked listener for // this handle from the DOM node AND drop the map entries. - // Previously this case only deleted from the map, leaving the DOM - // bindings until GC -- mostly harmless but inconsistent with - // removeHandler and falsified the comment that claimed "Detach any - // listeners". Also: parse the handle prefix numerically (was - // startsWith-based, which would match handle 1 against keys for - // 10/11/etc. once handles cross 10). - const node = nodes.get(p.handle); - for (const key of Array.from(listeners.keys())) { - const sep = key.indexOf(":"); - if (sep < 0) continue; - if (Number(key.slice(0, sep)) !== p.handle) continue; - const event = key.slice(sep + 1); - const fn = listeners.get(key); - if (node !== undefined && fn !== undefined) { - node.removeEventListener(event, fn); - } - listeners.delete(key); - } + detachListeners(p.handle, nodes.get(p.handle)); nodes.delete(p.handle); return; } @@ -190,6 +194,12 @@ const node = nodes.get(p.handle); const parent = nodes.get(p.parentHandle); if (!node) return; + // The Swift side evicts this node's handler ids when it emits the + // patch (destroyNode is suppressed for the animated root), so its + // listeners must be detached NOW — kept through the animation + // window they dispatch into evicted ids and their map entries + // outlive the node. + detachListeners(p.handle, node); node.style.animation = p.animation; setTimeout(function () { if (parent && node.parentNode === parent) { diff --git a/examples/TodoCRUD/swiflow-driver.js b/examples/TodoCRUD/swiflow-driver.js index e65e14f4..51ed49a2 100644 --- a/examples/TodoCRUD/swiflow-driver.js +++ b/examples/TodoCRUD/swiflow-driver.js @@ -138,6 +138,27 @@ "stop", "marker", "clipPath", "mask", "pattern", "use", "symbol", ]); + /** + * Detach every tracked listener for `handle` from `node` and drop the + * `listeners` map entries. Shared by `destroyNode` and `animateExit` — + * both destruction paths must run the same sweep or entries leak. The + * handle prefix is parsed numerically: a startsWith match would hit + * handle 1 against keys for 10/11/etc. + */ + function detachListeners(handle, node) { + for (const key of Array.from(listeners.keys())) { + const sep = key.indexOf(":"); + if (sep < 0) continue; + if (Number(key.slice(0, sep)) !== handle) continue; + const event = key.slice(sep + 1); + const fn = listeners.get(key); + if (node !== undefined && fn !== undefined) { + node.removeEventListener(event, fn); + } + listeners.delete(key); + } + } + /** * Apply a single patch. The opcode is `p.op`; field names match the * Swift-side `PatchSerializer.encode(...)` contract. @@ -165,24 +186,7 @@ case "destroyNode": { // Symmetric with removeHandler: detach every tracked listener for // this handle from the DOM node AND drop the map entries. - // Previously this case only deleted from the map, leaving the DOM - // bindings until GC -- mostly harmless but inconsistent with - // removeHandler and falsified the comment that claimed "Detach any - // listeners". Also: parse the handle prefix numerically (was - // startsWith-based, which would match handle 1 against keys for - // 10/11/etc. once handles cross 10). - const node = nodes.get(p.handle); - for (const key of Array.from(listeners.keys())) { - const sep = key.indexOf(":"); - if (sep < 0) continue; - if (Number(key.slice(0, sep)) !== p.handle) continue; - const event = key.slice(sep + 1); - const fn = listeners.get(key); - if (node !== undefined && fn !== undefined) { - node.removeEventListener(event, fn); - } - listeners.delete(key); - } + detachListeners(p.handle, nodes.get(p.handle)); nodes.delete(p.handle); return; } @@ -190,6 +194,12 @@ const node = nodes.get(p.handle); const parent = nodes.get(p.parentHandle); if (!node) return; + // The Swift side evicts this node's handler ids when it emits the + // patch (destroyNode is suppressed for the animated root), so its + // listeners must be detached NOW — kept through the animation + // window they dispatch into evicted ids and their map entries + // outlive the node. + detachListeners(p.handle, node); node.style.animation = p.animation; setTimeout(function () { if (parent && node.parentNode === parent) { diff --git a/js-driver/swiflow-driver.js b/js-driver/swiflow-driver.js index e65e14f4..51ed49a2 100644 --- a/js-driver/swiflow-driver.js +++ b/js-driver/swiflow-driver.js @@ -138,6 +138,27 @@ "stop", "marker", "clipPath", "mask", "pattern", "use", "symbol", ]); + /** + * Detach every tracked listener for `handle` from `node` and drop the + * `listeners` map entries. Shared by `destroyNode` and `animateExit` — + * both destruction paths must run the same sweep or entries leak. The + * handle prefix is parsed numerically: a startsWith match would hit + * handle 1 against keys for 10/11/etc. + */ + function detachListeners(handle, node) { + for (const key of Array.from(listeners.keys())) { + const sep = key.indexOf(":"); + if (sep < 0) continue; + if (Number(key.slice(0, sep)) !== handle) continue; + const event = key.slice(sep + 1); + const fn = listeners.get(key); + if (node !== undefined && fn !== undefined) { + node.removeEventListener(event, fn); + } + listeners.delete(key); + } + } + /** * Apply a single patch. The opcode is `p.op`; field names match the * Swift-side `PatchSerializer.encode(...)` contract. @@ -165,24 +186,7 @@ case "destroyNode": { // Symmetric with removeHandler: detach every tracked listener for // this handle from the DOM node AND drop the map entries. - // Previously this case only deleted from the map, leaving the DOM - // bindings until GC -- mostly harmless but inconsistent with - // removeHandler and falsified the comment that claimed "Detach any - // listeners". Also: parse the handle prefix numerically (was - // startsWith-based, which would match handle 1 against keys for - // 10/11/etc. once handles cross 10). - const node = nodes.get(p.handle); - for (const key of Array.from(listeners.keys())) { - const sep = key.indexOf(":"); - if (sep < 0) continue; - if (Number(key.slice(0, sep)) !== p.handle) continue; - const event = key.slice(sep + 1); - const fn = listeners.get(key); - if (node !== undefined && fn !== undefined) { - node.removeEventListener(event, fn); - } - listeners.delete(key); - } + detachListeners(p.handle, nodes.get(p.handle)); nodes.delete(p.handle); return; } @@ -190,6 +194,12 @@ const node = nodes.get(p.handle); const parent = nodes.get(p.parentHandle); if (!node) return; + // The Swift side evicts this node's handler ids when it emits the + // patch (destroyNode is suppressed for the animated root), so its + // listeners must be detached NOW — kept through the animation + // window they dispatch into evicted ids and their map entries + // outlive the node. + detachListeners(p.handle, node); node.style.animation = p.animation; setTimeout(function () { if (parent && node.parentNode === parent) { diff --git a/js-driver/test/opcodes.test.js b/js-driver/test/opcodes.test.js index 1a4685e2..1e466abe 100644 --- a/js-driver/test/opcodes.test.js +++ b/js-driver/test/opcodes.test.js @@ -61,6 +61,52 @@ describe("driver opcodes", () => { // entry only. removeChild is what removes from DOM. }); + test("animateExit detaches listeners immediately, removes the node after the window", async () => { + const { swiflow, window, document } = setupDriver(); + let callCount = 0; + window.__swiflowDispatch = () => { callCount += 1; }; + swiflow.applyPatches([ + { op: "createElement", handle: 1, tag: "div" }, + { op: "createElement", handle: 2, tag: "button" }, + { op: "appendChild", parent: 1, child: 2 }, + { op: "addHandler", handle: 2, event: "click", handlerId: 7 }, + ]); + swiflow.mount(1, "#app"); + const btn = document.querySelector("button"); + btn.click(); + assert.equal(callCount, 1); + + swiflow.applyPatches([ + { op: "animateExit", handle: 2, parentHandle: 1, animation: "out 0.2s", durationMs: 0 }, + ]); + assert.equal(btn.style.animation, "out 0.2s"); + // The Swift side evicted handler id 7 when it emitted the patch — a + // click inside the animation window must not dispatch, and the + // listeners entry must be gone (this was the per-toast leak). + btn.click(); + assert.equal(callCount, 1, "no dispatch during the exit window"); + + await new Promise((r) => setTimeout(r, 0)); + assert.equal(document.querySelector("button"), null, "removed after the window"); + // Map entry gone → re-destroying the handle is a no-op. + swiflow.applyPatches([{ op: "destroyNode", handle: 2 }]); + }); + + test("animateExit falls back to node.parentNode when parentHandle is unknown", async () => { + const { swiflow, document } = setupDriver(); + swiflow.applyPatches([ + { op: "createElement", handle: 1, tag: "div" }, + { op: "createElement", handle: 2, tag: "span" }, + { op: "appendChild", parent: 1, child: 2 }, + ]); + swiflow.mount(1, "#app"); + swiflow.applyPatches([ + { op: "animateExit", handle: 2, parentHandle: 99, animation: "out 0.1s", durationMs: 0 }, + ]); + await new Promise((r) => setTimeout(r, 0)); + assert.equal(document.querySelector("span"), null); + }); + test("insertBefore places a child before a reference child", () => { const { swiflow, document } = setupDriver(); swiflow.applyPatches([