New single-pass algorithm, new restoreFocus option, big refactor!#93
Merged
New single-pass algorithm, new restoreFocus option, big refactor!#93
restoreFocus option, big refactor!#93Conversation
Co-authored-by: MichaelWest22 <michael@latent.nz>
…s into hook file.
…, rather than deleting.
…cus and/or selection.
…s 2n fewer array allocations, where n is the number of DOM nodes with children.
Co-authored-by: MichaelWest22 <michael@latent.nz>
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
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.
Overview
This PR is a proposal for the future of Idiomorph. Michael and I have done our best to make this superior to v0.4.0 in every way. While there are further improvements that could be made, we think this is a solid stake in the ground to be merged into main, and perhaps released as v0.5.0.
Normally, I try to optimize my PRs to be a concise, cohesive, and atomic unit of work, with an easily grokkable diff. This PR is rather the opposite of that, aiming to demonstrate a clear and total vision, instead. Although, in retrospect, I should have made more effort to limit the scope.
Behavior Changes
restoreFocusoptiontwoPassoptionbeforeNodePantriedhookRefactorings
Future Work
Details
Behavior Changes
New
restoreFocusboolean optionUntil
moveBeforebecomes available, maintaining focus and selection state in all cases is impossible. In the meantime, we can paper over this by saving and restoring focus and selection states around the morph, at the cost of potential extra blur, focus, and selection events. This option is off by default, but as more cases are handled natively without extra events, we can imagine it being on by default.Removed
twoPassoptionThere is only one core algorithm now.
Removed
beforeNodePantriedhookThis was an unfortunate abstraction-leaking necessity of the specific implementation of the v0.4.0 two-pass algorithm. If you were doing interesting things with any of the other hooks, e.g. data-turbo-permanent, you'd often need to prevent the pantrying process from flattening the node trees that entered it, and thus you'd need to know how the internals of the pantrying process worked, and that there was a pantrying process at all, for that matter. This PR removes the hook, and the need for it, and now Idiomorph is back to being a magic black box.
Many edge cases caught, tested, and fixed
We've been working on this for a while, and we've been very thorough, particularly Michael. We've fixed many edge cases, capturing them in tests first. We're confident that this is a solid foundation for the future.
Hooks are called correctly in all cases
Hooks are called in the order one would expect, and with the arguments one would expect. Two-pass mode resulted in minor changes to both in v0.4.0. This PR results in hook behavior that is more correct than both v0.3.0 and v0.4.0.
Refactorings
Reorganized and modularized internal architecture
In v0.4.0, all the functions were in one IIFE, free to call each other, and the call graph looked like a plate of spaghetti, with many cyclical relationships. We've untangled and extracted five independent sub-IIFEs from the main one, and now it looks a bit more like a lasagna. A DAG lasagna :)
Simplified core algorithm
Speaking of lasagna, here is the tasty meat. By removing the two-pass mode, and further leveraging the persistentId knowledge, we've been able to significantly improve the core algorithm, while also significantly simplifying it! Here's the gist of new algo, copied from the
morphChildrenfunction:We've also been able to simplify and improve
findIdSetMatchandfindSoftMatchas well. Since persisted nodes are no longer lost when they're removed, there's no longer any need for a bail-early heuristic. This means we can always find and morph all id set matches. We still bail out early on soft matches, to reduce churn. Those two functions have also been merged into a single simplerfindBestMatchfunction, which only traverses the children once instead of twice.Finally, there's just a lot less code! Less functions, less branches, less lines, less complexity.
OuterHTML morph uses the same algorithm as innerHTML morph
This was a big win. Until now, the outerHTML morph used a complex bespoke algorithm that was entirely separate from the core morphChildren loop. This meant it wasn't nearly as well-tested, and both algorithms had peculiar and subtle strengths and weaknesses. Now, they've been merged together into one smaller general algorithm, which has the best qualities of both.
Less internal state
Michael found that the idSet code can be greatly simplified, now that we have persistentIds. We no longer need to track deadIds, for example, so that's gone. Also, we were able to to make the idMap smaller by only including ids that are persistent. There are likely further wins here, in the future.
Renamed functions and tweaked signatures for more consistency
We've standardized on "old" vs "new", and always in that order, as opposed to "from" or "to" or "current" etc. There's a bit more to do here, perhaps always passing
ctxfirst, C-style.100% test coverage!
Title says it all. We're feeling really good about the tests suite's ability to tell us when something regresses, and CI will now fail if it dips below 100%, to maintain this confidence moving forward. Big props to Michael for plugging the remaining holes, which was no small feat!
Future Work
Retain focus and selection natively in more cases, rather than restoring
This is likely the last major endeavour before we might consider Idiomorph to be more-or-less complete i.e. v1.0.0. Michael and I both have several promising ideas, but I want to get this PR merged before we dive into that. In the meantime, the
restoreFocusoption is a good stop-gap improvement.Performance improvements
According to
npm run perfon my machine, this branch is ~0%-5% slower than v0.4.0, and ~10%-20% slower than v0.3.0. To me, this sees like an acceptable trade of minor performance loss for major correctness gains. That said, I'd like to spend some time with my perf hat on to see if we can't make it any faster.Investigate how Idiomorph works (or does not) with Shadow DOM, Iframes, and other likely troublesome cases
Known unknowns. I'd like to at least be aware of the status quo before making any plans for v1.0.0.