allow reentrant onMount handlers (#69)

examples/kanban/package-lock.json

···
 			"name": "kanban",
 			"dependencies": {
 				"@tombl/router": "npm:@jsr/tombl__router@^0.1.2",
-				"dhtml": "file:../..",
+				"dhtml": "file:../../dist",
 				"sqlocal": "^0.14.1"
 			},
 			"devDependencies": {
···
 			}
 		},
 		"../../dist": {
-			"name": "dhtml",
-			"extraneous": true
+			"name": "dhtml"
 		},
 		"node_modules/@esbuild/aix-ppc64": {
 			"version": "0.25.0",
···
 			}
 		},
 		"node_modules/dhtml": {
-			"resolved": "../..",
+			"resolved": "../../dist",
 			"link": true
 		},
 		"node_modules/esbuild": {

examples/kanban/package.json

···
 	},
 	"dependencies": {
 		"@tombl/router": "npm:@jsr/tombl__router@^0.1.2",
-		"dhtml": "file:../..",
+		"dhtml": "file:../../dist",
 		"sqlocal": "^0.14.1"
 	}
 }

src/client/controller.ts

···
 export type Key = string | number | bigint | boolean | symbol | object | null
 
 export interface Controller {
-	_mount_callbacks?: Set<Cleanup> // undefined if mounted
-	_unmount_callbacks: Set<Cleanup>
+	_mounted: boolean
+	_mount_callbacks: Cleanup[]
+	_unmount_callbacks: Cleanup[]
 
 	_invalidate_queued?: Promise<void>
 	_invalidate?: () => void
···
 
 export function get_controller(renderable: Renderable): Controller {
 	let controller = controllers.get(renderable)
-	if (controller) return controller
-
-	controller = {
-		_mount_callbacks: new Set(),
-		_unmount_callbacks: new Set(),
-	}
-
-	controllers.set(renderable, controller)
+	if (!controller)
+		controllers.set(
+			renderable,
+			(controller = {
+				_mounted: false,
+				_mount_callbacks: [],
+				_unmount_callbacks: [],
+			}),
+		)
 	return controller
-}
-export function delete_controller(renderable: Renderable): void {
-	controllers.delete(renderable)
 }
 
 const keys: WeakMap<Displayable & object, Key> = new WeakMap()
···
 
 export function onMount(renderable: Renderable, callback: () => Cleanup): void {
 	assert(is_renderable(renderable), 'expected a renderable')
-
 	const controller = get_controller(renderable)
-	if (controller._mount_callbacks) {
-		controller._mount_callbacks.add(callback)
+	if (controller._mounted) {
+		controller._unmount_callbacks.push(callback())
 	} else {
-		controller._unmount_callbacks.add(callback())
+		controller._mount_callbacks.push(callback)
 	}
 }
 

src/client/parts.ts

···
 import type { Displayable, Renderable } from 'dhtml'
 import { assert, is_html, is_iterable, is_renderable, single_part_template } from '../shared.ts'
-import { delete_controller, get_controller, get_key } from './controller.ts'
+import { get_controller, get_key } from './controller.ts'
 import { create_root, create_root_after, type Root } from './root.ts'
 import { create_span, delete_contents, extract_contents, insert_node, type Span } from './span.ts'
 import type { Cleanup } from './util.ts'
···
 	function switch_renderable(next: Renderable | null) {
 		if (current_renderable && current_renderable !== next) {
 			const controller = get_controller(current_renderable)
-			controller._unmount_callbacks.forEach(callback => callback?.())
-			delete_controller(current_renderable)
+			if (controller._mounted) {
+				controller._mounted = false
+				controller._unmount_callbacks.forEach(callback => callback?.())
+			}
 		}
 		current_renderable = next
 	}
···
 
 			if (current_renderable) {
 				const controller = get_controller(current_renderable)
-				controller._mount_callbacks?.forEach(callback => controller._unmount_callbacks.add(callback?.()))
-				delete controller._mount_callbacks
+				if (!controller._mounted) {
+					controller._mounted = true
+					controller._unmount_callbacks = controller._mount_callbacks.map(callback => callback?.())
+				}
 			}
 
 			if (ends_were_equal) parent_span._end = span._end
···
 
 		if (current_renderable) {
 			const controller = get_controller(current_renderable)
-			controller._mount_callbacks?.forEach(callback => controller._unmount_callbacks.add(callback?.()))
-			delete controller._mount_callbacks
+			if (!controller._mounted) {
+				controller._mounted = true
+				controller._unmount_callbacks = controller._mount_callbacks.map(callback => callback?.())
+			}
 		}
 
 		if (ends_were_equal) parent_span._end = span._end

src/client/tests/renderable.test.ts

···
 	const sequence: string[] = []
 
 	const inner = {
-		attached: false,
 		render() {
 			sequence.push('inner render')
-			if (!this.attached) {
-				this.attached = true
-				onMount(this, () => {
-					sequence.push('inner mount')
-					return () => {
-						sequence.push('inner cleanup')
-					}
-				})
-			}
 			return 'inner'
 		},
 	}
+	onMount(inner, () => {
+		sequence.push('inner mount')
+		return () => {
+			sequence.push('inner cleanup')
+		}
+	})
 
 	const outer = {
-		attached: false,
 		show: true,
 		render() {
 			sequence.push('outer render')
-			if (!this.attached) {
-				this.attached = true
-				onMount(this, () => {
-					sequence.push('outer mount')
-					return () => {
-						sequence.push('outer cleanup')
-					}
-				})
-			}
 			if (!this.show) return null
 			return inner
 		},
 	}
+
+	onMount(outer, () => {
+		sequence.push('outer mount')
+		return () => {
+			sequence.push('outer cleanup')
+		}
+	})
 
 	outer.show = true
 	root.render(outer)
···
 	outer.show = true
 	root.render(outer)
 	assert.equal(el.innerHTML, 'inner')
-	assert.deepStrictEqual(sequence, ['outer render', 'inner render'])
+	// inner is mounted a second time because of the above cleanup
+	assert.deepStrictEqual(sequence, ['outer render', 'inner render', 'inner mount'])
 	sequence.length = 0
 })
 
···
 
 	const app = {
 		render() {
-			onMount(this, () => {
-				sequence.push('mount 1')
-				return () => sequence.push('cleanup 1')
-			})
-
-			onMount(this, () => {
-				sequence.push('mount 2')
-				return () => sequence.push('cleanup 2')
-			})
-
 			return 'app'
 		},
 	}
 
+	onMount(app, () => {
+		sequence.push('mount 1')
+		return () => sequence.push('cleanup 1')
+	})
+
+	onMount(app, () => {
+		sequence.push('mount 2')
+		return () => sequence.push('cleanup 2')
+	})
+
 	root.render(app)
 	assert.deepStrictEqual(sequence, ['mount 1', 'mount 2'])
 	sequence.length = 0
···
 	assert.deepStrictEqual(sequence, ['cleanup 1', 'cleanup 2'])
 })
 
-test('onMount registers a fixed callback once', () => {
+test('onMount registers a fixed callback multiple times', () => {
 	const { root } = setup()
 
 	const sequence: string[] = []
···
 
 	const app = {
 		render() {
-			onMount(this, callback)
-			onMount(this, callback)
 			return 'app'
 		},
 	}
+
+	onMount(app, callback)
+	onMount(app, callback)
 
 	root.render(app)
-	assert.deepStrictEqual(sequence, ['mount'])
+	assert.deepStrictEqual(sequence, ['mount', 'mount'])
 	sequence.length = 0
 
 	root.render(null)
-	assert.deepStrictEqual(sequence, ['cleanup'])
+	assert.deepStrictEqual(sequence, ['cleanup', 'cleanup'])
 })
 
 test('onMount registers callbacks outside of render', () => {
···
 
 	const app = {
 		render() {
-			onMount(this, () => {
-				const parent = getParentNode(this) as Element
-				assert(parent.firstElementChild instanceof HTMLParagraphElement)
-			})
 			return html`<p>Hello, world!</p>`
 		},
 	}
 
+	onMount(app, () => {
+		const parent = getParentNode(app) as Element
+		assert(parent.firstElementChild instanceof HTMLParagraphElement)
+	})
+
 	root.render(app)
 })
 
-test('onMount works after render', () => {
+test('onMount is called immediately on a mounted renderable', () => {
 	const { root } = setup()
 
 	const app = {
···
 	const sequence: string[] = []
 
 	const inner = {
-		attached: false,
 		render() {
 			sequence.push('inner render')
-			if (!this.attached) {
-				this.attached = true
-				onUnmount(this, () => {
-					this.attached = false
-					sequence.push('inner abort')
-				})
-			}
 			return 'inner'
 		},
 	}
 
+	onUnmount(inner, () => {
+		sequence.push('inner abort')
+	})
+
 	const outer = {
-		attached: false,
 		show: true,
 		render() {
 			sequence.push('outer render')
-			if (!this.attached) {
-				this.attached = true
-				onUnmount(this, () => {
-					this.attached = false
-					sequence.push('outer abort')
-				})
-			}
 			if (!this.show) return null
 			return inner
 		},
 	}
+
+	onUnmount(outer, () => {
+		sequence.push('outer abort')
+	})
 
 	outer.show = true
 	root.render(outer)
···
 	const sequence: string[] = []
 
 	const inner = {
-		attached: false,
 		render() {
 			sequence.push('inner render')
-			if (!this.attached) {
-				this.attached = true
-				onUnmount(this, () => {
-					this.attached = false
-					sequence.push('inner abort')
-				})
-			}
 			return 'inner'
 		},
 	}
+
+	onUnmount(inner, () => {
+		sequence.push('inner abort')
+	})
 
 	const outer = {
 		attached: false,
···
 
 	root.render(null)
 	assert.equal(unmounted.mock.calls.length, 1)
+})
+
+test('onMount works for repeated mounts', () => {
+	const { root } = setup()
+	let mounted: boolean | null = null
+
+	const app = {
+		render() {
+			return html`${mounted}`
+		},
+	}
+	onMount(app, () => {
+		mounted = true
+		return () => {
+			mounted = false
+		}
+	})
+
+	assert.equal(mounted, null)
+
+	for (let i = 0; i < 10; i++) {
+		root.render(app)
+		assert.equal(mounted, true)
+
+		root.render(null)
+		assert.equal(mounted, false)
+	}
 })
 
 test('getParentNode works externally', () => {