[BUG-26] with() chains discard hook cleanup functions

[shroy]

Severity: P2 · Batch: B12 (with() chain cleanups — depends on B3's cleanup mechanism)

Location

src/core/with.js:40-83

Root cause

Every chain method calls the hook and drops its returned cleanup → with(el).useEvents({...}) inside a component leaks listeners on teardown unless the author separately plumbs cleanups (the chainable API gives them no handle to do so).

Proposed fix

Collect cleanups into the active hook context (the BUG-19 mechanism) or return them from the chain.

Verification recipe

Spec: inside a component, with(el).useEvents({ click: spy }); tear the component down; dispatch click; assert spy not called. Expected on unmodified code: spy fires (listener leaked) → confirmed.

[shroy]

Phase 1 verification: CONFIRMED

Method: runtime repro (vitest + jsdom), corroborated by static reading of src/core/with.js.

Root cause (static)

Every chain method in src/core/with.js invokes the underlying hook but throws away the cleanup function the hook returns. For useEvents (lines 40-43):

useEvents: (eventMap) => {
  useEvents(element, eventMap)   // <- return value (cleanup fn) discarded
  return chain
},

useEvents returns a real teardown closure (src/hooks/useEvents.js:97-104) that calls removeEventListener, but with() never registers it into the component's hook context. Teardown (observer.removeElement -> lifecycleManager.executeTeardowns(el) + runCleanupFunctions(el) in src/core/observer.js:283-296) only runs cleanups tracked in componentCleanups / effect cleanups (src/core/hookContext.js:318-357), so nothing removes the listener. The same discard pattern applies to useClasses/useAttributes/useStyles/useText and to dynamically registered chainable hooks (lines 97-100).

Repro spec

/**
 * @vitest-environment jsdom
 */
import { describe, it, expect, vi, beforeEach } from 'vitest'
import { initializeComponents } from '../../core/scanComponents.js'
import * as registryModule from '../../core/registry.js'
import { lifecycleManager } from '../../core/initialization.js'
import { runCleanupFunctions } from '../../core/hookContext.js'
import { with as withEl } from '../../core/with.js'

describe('BUG-26 / issue #37 — with() chain hook cleanup on teardown', () => {
  beforeEach(() => {
    vi.restoreAllMocks()
    document.body.innerHTML = ''
  })

  it('removes with().useEvents listeners when the owning component is torn down', () => {
    const element = document.createElement('div')
    document.body.appendChild(element)

    const clickSpy = vi.fn()

    // A component that attaches a click listener via the chainable with() API.
    // It returns nothing — the author expects the framework to reclaim the
    // hook's cleanup, exactly as if useEvents were called directly.
    const componentFn = vi.fn(() => {
      withEl(element).useEvents({ click: clickSpy })
    })

    vi.spyOn(registryModule, 'getRegisteredComponent').mockReturnValue(componentFn)

    // Initialize the component inside a real hook context (production path).
    initializeComponents([{ element, componentName: 'LeakyComponent' }])

    // Sanity check: the listener really is attached while mounted.
    element.dispatchEvent(new Event('click'))
    expect(clickSpy).toHaveBeenCalledTimes(1)
    clickSpy.mockClear()

    // Tear the component down exactly like observer.removeElement does.
    lifecycleManager.executeTeardowns(element)
    runCleanupFunctions(element)

    // Dispatch a click AFTER teardown.
    element.dispatchEvent(new Event('click'))

    // Correct behavior: the listener was cleaned up, so the spy must NOT fire.
    // On unmodified code the with() chain dropped the cleanup, so this fails.
    expect(clickSpy).not.toHaveBeenCalled()
  })
})

Decisive output (unmodified code)

 ❯ src/tests/__verify__/issue-37.spec.js > ... > removes with().useEvents listeners when the owning component is torn down
   → expected "spy" to not be called at all, but actually been called 1 times

 ❯ src/tests/__verify__/issue-37.spec.js:61:26
   59|     // Correct behavior: the listener was cleaned up, so the spy must …
   60|     // On unmodified code the with() chain dropped the cleanup, so thi…
   61|     expect(clickSpy).not.toHaveBeenCalled()
       |                          ^

 Test Files  1 failed (1)
      Tests  1 failed (1)

Interpretation

The in-mount sanity assertion passed (spy fired once while mounted, then cleared), and after the full production teardown path (executeTeardowns + runCleanupFunctions) a fresh click STILL fired the spy. The click listener was never removed because with().useEvents(...) discarded the cleanup returned by useEvents. Bug confirmed exactly as described; the chainable API leaks all hook cleanups on teardown.