Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cleanup if remount #4493

Merged

Conversation

tanhauhau
Copy link
Member

Fix: #4491

Context:

To apply directives in order #2446, which is to call binding, actions and event handler in the order they are declared in the attributes, we grouped them together in #2737 and #4156.

However, actions were previously called in mount phase, and binding and event handler is called in create phase.

Grouping them together, meaning that they all of them have to be called in either mount phase or in create phase.

We moved all of them into mount phase, due to #4252.

Now, whenever we update each block via update_keyed_each, existing item that got shuffled will be mounted again in the right position, triggering the mount phase, causing the the element to re-addEventListener.

The reason why normal listeners doesn't seemed to be re-add is that if you call addEventListener with the same listener function, it will not be subscribed, but for the modifier version, we created a new listener function everytime, by decorating the normal listener with modifier function.

@Conduitry
Copy link
Member

@tanhauhau Are you still feeling good about this change? Is all you think it needs is to updated a bunch of js sample tests? What do you think about not including the remount parameter if the block has no event listeners?

@tanhauhau
Copy link
Member Author

Are you still feeling good about this change?

not sure. it feels like since #2446, we've been fixing a lot of regression bugs that we did not forsee. im not sure we are on the right direction.

What do you think about not including the remount parameter if the block has no event listeners?

sounds like a good idea

@Conduitry Conduitry merged commit b4b57c3 into sveltejs:master Mar 15, 2020
@tanhauhau tanhauhau deleted the tanhauhau/listener-callback-keyed-each branch March 15, 2020 23:36
taylorzane pushed a commit to taylorzane/svelte that referenced this pull request Dec 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Event Modifier on keyed spread
2 participants