livewire icon indicating copy to clipboard operation
livewire copied to clipboard

Add return cleanup function for $wire.$on

Open mrfelipemartins opened this issue 1 year ago • 3 comments

1️⃣ Is this something that is wanted/needed? Did you create a discussion about it first? Yes, this change is needed to improve the management of event listeners. There wasn’t an existing discussion specifically about this issue, but the change addresses a common problem many developers face when dealing with event listeners.

2️⃣ Did you create a branch for your fix/feature? (Main branch PR's will be closed) Yes

3️⃣ Does it contain multiple, unrelated changes? Please separate the PRs out. No, this PR contains a single and focused change

4️⃣ Does it include tests? (Required) Yes, the PR includes new Dusk tests to verify the behavior of the $wire.on method and the new cleanup functionality.

5️⃣ Please include a thorough description (including small code snippets if possible) of the improvement and reasons why it's useful. Currently, the $wire.$on method in Livewire does not provide a mechanism to remove event listeners once they are no longer needed and rely only on browser garbage collector. In my case, I am using Mingle.js with React and for this particular case my listener resulted in multiple event triggers that could not be resolved with the current implementation of $wire.on.

mrfelipemartins avatar Sep 02 '24 21:09 mrfelipemartins

Thanks for the PR 🙌 Could you add the test to the SupportEvents directory instead of legacy_tests (this should not be used preferably) and use the new test syntax as shown in the Events browser test?

PhiloNL avatar Sep 12 '24 11:09 PhiloNL

@PhiloNL, sure. I just updated the tests!

mrfelipemartins avatar Sep 13 '24 13:09 mrfelipemartins

@PhiloNL I am not sure if the failing tests above are related to this PR since locally the test I added passed successfully

mrfelipemartins avatar Sep 18 '24 12:09 mrfelipemartins

@mrfelipemartins thanks for the PR! I've just merged main in, and it seems your change is impacting some of the other $on tests like test_can_listen_for_component_event_with_this_on_in_javascript. Are you able to have a look at that?

joshhanley avatar Dec 11 '24 07:12 joshhanley

it still leaks after element is removed from DOM and will make Uncaught (in promise) Could not find Livewire component in DOM tree if you use $wire inside $wire.$on and require to be fixed in $wire.js file too.

simple reproduction:

<div x-data="{ count: 0 }">
    <div x-init="() => {
        const cleanup = $wire.$on('message', (event) => {
            count++
            $wire.set('message', event.message)
        })

        document.getElementById('cleanup').addEventListener('click', () => {
            cleanup()
            $wire.second()
        })
    }" wire:replace.self>
        <span x-text="count"></span>
        <span>{{ $message }}</span>
        <button class="text-red-500" wire:click="first">Emit Event</button>
        <button id="cleanup">Cleanup</button>
    </div>
</div>

maybe can be fixed using cleanup when element is removed too if this PR merged?

danie-ramdhani avatar Dec 18 '24 11:12 danie-ramdhani

@mrfelipemartins I've just merged the main branch in and there are still some failing tests. Are you able to have a look at them? As I believe they are caused by changes in this PR. Specifically this test test_can_listen_for_component_event_with_this_on_in_javascript. Thanks!

joshhanley avatar Mar 20 '25 06:03 joshhanley

Closing as failing tests haven't been fixed. Feel free to resubmit once they're fixed. Thanks!

joshhanley avatar May 29 '25 07:05 joshhanley