Fix(menu): submenu machine.subscribe accumulation#44
Merged
Conversation
Track and tear down submenu trigger subscriptions before re-wiring, unsubscribe on menu destroy, and cancel pending submenu wiring timers when the LiveView hook is destroyed.
karim-semmoud
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes a memory and performance bug in nested menus where machine.subscribe callbacks accumulated on every LiveView update.
renderSubmenuTriggers() subscribed to parent and child Zag machines on each call without unsubscribing. The menu hook calls wireSubmenuTriggersDeep() on mount and on every updated(), so subscriber count grew with each server re-render. That caused duplicate DOM updates, stale closures, and unnecessary work.
Also fixes a related race: nested menu wiring ran in setTimeout(..., 0) with no cancellation, so wiring could run after the hook was destroyed on fast navigation.
Changes
assets/components/menu.ts
Track submenu trigger unsubscribe handles in submenuTriggerUnsubs
Tear down existing subscriptions at the start of renderSubmenuTriggers()
Clear all submenu subscriptions in destroy()
assets/hooks/menu.ts
Store submenuWireTimer from nested menu wiring
Clear the timer in destroyed() before tearing down menus
Guard the wiring callback when this.menu is gone
Set this.menu = undefined after destroy
Tests
assets/test/component/menu.test.ts: subscription replacement and destroy cleanup
assets/test/hooks/menu.test.ts: destroyed hook cancels pending wiring timer
assets/test/helpers/mock-hook.ts: shared mockHookContext, callHookMounted, callHookDestroyed for hook lifecycle tests