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

improve id matching during findBestMatch #94

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

MichaelWest22
Copy link
Collaborator

@MichaelWest22 MichaelWest22 commented Jan 30, 2025

I've added back the original idiomorph id set matching code. This code counts up the number of id's in the current new node and then as it scans though the content for the match it counts up the number of id's that will be displaced and if this ever goes over the number we will preserve in place with the match then we break and return our nothing (or our last valid saved softmatch) so that the current id nodes will instead be inserted in place instead of searching to the bottom to find the id in lower down old content. If we dont do this then sometimes it will instead remove multiple nodes with id's to make the lower down match and put them in pantry instead and discard any non id'ed content that could have been morphed possibly.
There is also an optimization added back in to return softmatch early if there are no id's in the current node so it can be softmatched quickly as it is not valid to keep searching for a id set match when there are no id's to match on!

@MichaelWest22 MichaelWest22 changed the title improve id matching during findBestMatch and fix hook case improve id matching during findBestMatch Feb 1, 2025
@MichaelWest22
Copy link
Collaborator Author

https://codepen.io/MichaelWest22/pen/myboqmO

Here is a code pen example of the textbook example from the Readme of video preservation you can do with idiomoprh. When you swap the heading div between above and below the video it should allow the id in the video to be respected and the heading div should not be picked as a good match to morph with a lower div that would destroy the id'ed videos state. With the current algorithm it just always finds the first match for every new node no matter how destructive it would be to id'ed or other nodes. By adding back the original idset matching logic that was really the core of what idiomorph was designed for it works perfectly for all situations the old idiomoprh routine could retain state like videos.

https://codepen.io/MichaelWest22/pen/RNbdjJp

Here is the same idiomoprh textbook example with this pr code showing it works great moving the header up and down even when using normal browsers without moveBefore feature just like it always did.

Also there is a focus test that was broken that is now fixed you can check out in this PR where now focus is preserved correctly even when the browser does not have moveBefore.

There are also going to be many situations where without this change in place preservation hooks that used to work to preserve nodes will just not longer work properly. When id's or nodes are reordered or moved it will discard a lot of the nodes in the middle and the preservation hooks will retain the element but it will then leave it in the wrong dom location. We should be trying to keep hooks and preservation the same as the old algorithm where possible so we dont break any existing apps and use cases. We may have better id node state preservation but for some new situations but keeping old correctness is also important.

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.

1 participant