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

🐛 Fixes reinitializing moved elements #3995

Merged
merged 1 commit into from
Jan 24, 2024

Conversation

ekwoka
Copy link
Contributor

@ekwoka ekwoka commented Jan 24, 2024

Fixes #3994

Introduced in #3951

Adds a test for it :)

@SimoTod
Copy link
Collaborator

SimoTod commented Jan 24, 2024

I think this will reintroduce the issue for the guy who originally made the change. I suspect the problem is that he sets the init flag after processing the directives while he should do it at the very beginning (inside the if).

@ekwoka
Copy link
Contributor Author

ekwoka commented Jan 24, 2024

I do agree.

but I think fixing this should be priority over ensuring that case also works.

I will investigate an additional PR that can handle both situations more easily.

But it seems clear to me that now it is likely breaking all elements that move. or at least POTENTIALLY breaking them depending on the side effects of their directives.

@calebporzio
Copy link
Collaborator

Thanks @ekwoka, I think I actually removed that line by mistake when I was refactoring that PR? I changed addedNodes and removedNodes to Sets to ensure uniqueness, but we also need this check in there for "move" scenarios.

Thanks again. Will tag a patch release asap

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.

3 participants