Skip to content

Commit 27911e4

Browse files
authored
fix: pass domRef to onDestroyed / onBeforeDestroyed hooks (#1636)
* fix: pass domRef to onDestroyed / onBeforeDestroyed hooks callDestroyed and callBeforeDestroyed invoked the destroy hooks with no argument, so the DOMRef parameter of onDestroyedWith / onBeforeDestroyedWith (Miso.Event) was always undefined on the Haskell side. onBeforeDestroyedWith is documented as receiving the element's DOMRef for teardown of third-party widgets, mirroring onCreatedWith, which does receive it. Pass c.domRef at both call sites (the element is still alive at before-destroy time and just-detached at destroy time, matching the documented semantics), update the VNode hook signatures in types.ts, and rebuild js/miso.js and js/miso.prod.js. Zero-argument hook users are unaffected. Adds a regression test asserting both hooks receive the element that was created. * feat: add onDestroyedWith Completes the lifecycle hook family: onCreated / onCreatedWith, onBeforeDestroyed / onBeforeDestroyedWith existed, but onDestroyed had no DOMRef-receiving variant. The docs note the element is already detached from the DOM when the hook fires (no parent, zero bounding rect), so it is suited to cleaning up out-of-band references rather than in-document teardown, which belongs in onBeforeDestroyedWith.
1 parent 07a0e3d commit 27911e4

6 files changed

Lines changed: 48 additions & 7 deletions

File tree

js/miso.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -449,7 +449,7 @@ function callDestroyedRecursive(c) {
449449
}
450450
function callDestroyed(c) {
451451
if (c.type === 1 /* VNode */ && c.onDestroyed)
452-
c.onDestroyed();
452+
c.onDestroyed(c.domRef);
453453
if (c.type === 0 /* VComp */)
454454
unmountComponent(c);
455455
}
@@ -459,7 +459,7 @@ function callBeforeDestroyed(c) {
459459
break;
460460
case 1 /* VNode */:
461461
if (c.onBeforeDestroyed)
462-
c.onBeforeDestroyed();
462+
c.onBeforeDestroyed(c.domRef);
463463
break;
464464
default:
465465
break;

js/miso.prod.js

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/Miso/Event.hs

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ module Miso.Event
4040
, onCreatedWith
4141
, onBeforeCreated
4242
, onDestroyed
43+
, onDestroyedWith
4344
, onBeforeDestroyed
4445
, onBeforeDestroyedWith
4546
-- *** Exports
@@ -287,6 +288,25 @@ onDestroyed action =
287288
callback <- FFI.syncCallback (sink action)
288289
FFI.set "onDestroyed" callback object
289290
-----------------------------------------------------------------------------
291+
-- | Like 'onDestroyed' but also receives the element's 'DOMRef'.
292+
--
293+
-- The element has already been detached from the DOM when this fires: the
294+
-- 'DOMRef' has no parent and reports a zero bounding rect. Use it to clean
295+
-- up out-of-band references to the element (unregister it from a JS
296+
-- library, drop it from a lookup table) — for teardown that needs the
297+
-- element still live in the document, use 'onBeforeDestroyedWith'.
298+
--
299+
-- @since 1.13.0.0
300+
--
301+
onDestroyedWith
302+
:: (DOMRef -> action)
303+
-- ^ Callback receiving the element's (detached) 'DOMRef' after it is removed from the DOM
304+
-> Attribute model action
305+
onDestroyedWith action =
306+
On $ \_model sink (VTree object) _ _ -> do
307+
callback <- FFI.syncCallback1 (sink . action)
308+
FFI.set "onDestroyed" callback object
309+
-----------------------------------------------------------------------------
290310
-- | Fire an action just before the DOM element is removed from the document.
291311
--
292312
-- The element is still present in the DOM when this fires, making it suitable

ts/miso/dom.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,7 @@ function callDestroyedRecursive<T>(c: VNode<T> | VComp<T> | VFrag<T>): void {
155155
}
156156

157157
function callDestroyed<T>(c: VNode<T> | VComp<T>): void {
158-
if (c.type === VTreeType.VNode && c.onDestroyed) c.onDestroyed();
158+
if (c.type === VTreeType.VNode && c.onDestroyed) c.onDestroyed(c.domRef);
159159
if (c.type === VTreeType.VComp) unmountComponent(c);
160160
}
161161

@@ -164,7 +164,7 @@ function callBeforeDestroyed<T>(c: VNode<T> | VComp<T>): void {
164164
case VTreeType.VComp:
165165
break;
166166
case VTreeType.VNode:
167-
if (c.onBeforeDestroyed) c.onBeforeDestroyed();
167+
if (c.onBeforeDestroyed) c.onBeforeDestroyed(c.domRef);
168168
break;
169169
default:
170170
break;

ts/miso/types.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,8 +58,8 @@ export type VNode<T> = {
5858
* component events that don't bubble). Absent on browser/WASM. */
5959
directEvents?: Array<string>;
6060
children: Array<VTree<T>>;
61-
onDestroyed: () => void;
62-
onBeforeDestroyed: () => void;
61+
onDestroyed: (domRef: T) => void;
62+
onBeforeDestroyed: (domRef: T) => void;
6363
onCreated: (domRef: T) => void;
6464
onBeforeCreated: () => void;
6565
draw?: (T) => void;

ts/spec/dom.spec.ts

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -468,6 +468,27 @@ describe('DOM tests', () => {
468468
expect(destroy).toBe(1);
469469
});
470470

471+
test('Should pass the domRef to onDestroyed and onBeforeDestroyed', () => {
472+
let beforeRef: DOMRef | undefined;
473+
let destroyedRef: DOMRef | undefined;
474+
const currentNode = vnode<DOMRef>({
475+
onBeforeDestroyed: (domRef) => {
476+
beforeRef = domRef;
477+
},
478+
onDestroyed: (domRef) => {
479+
destroyedRef = domRef;
480+
},
481+
});
482+
483+
diff<DOMRef>(null, currentNode, document.body, drawingContext);
484+
const el = currentNode.domRef;
485+
expect(el).not.toBeNull();
486+
487+
diff<DOMRef>(currentNode, null, document.body, drawingContext);
488+
expect(beforeRef).toBe(el);
489+
expect(destroyedRef).toBe(el);
490+
});
491+
471492
test('Should call onDestroyed recursively', () => {
472493
let destroy = 0,
473494
childDestroy = 0;

0 commit comments

Comments
 (0)