Prepare codebase for new “moveBefore” DOM API. #282
Conversation
Things seem to be _moving_ along pretty quickly for this new API. The motivation for this (versus `insertBefore`) is that DOM state is not reset when you _move_ DOM nodes around within the same connected tree. Importantly, that means a `disconnectedCallback` followed immediately by a `connectedCallback` will no longer happen. Those two lifecycle events typically do some heavy lifting and we want to avoid them for cases like nodes simply being moved around within the same list or something. Note that this change set _prepares_ us to leverage this new API, but it doesn’t yet commit to it as support is only currently in Chrome 133. We wouldn’t normally get ahead of ourselves like this — but there are a lot of positive signals from other browsers, so it seems like this will go pretty quickly. Final note — I was able to confirm that shuffling a keyed array won’t cause disconnect / connect callbacks in the future!
|
Some places to watch:
Also, I reported a “bug” (though that’s probably too strong a word since this is just being implemented now) to Chrome about some behavioral forks I found a bit surprising: https://issues.chromium.org/issues/400758510 |
| // /** | ||
| // * Extends HTMLElement.prototype.connectedMoveCallback. | ||
| // */ | ||
| // connectedMoveCallback() {} |
There was a problem hiding this comment.
We’ll eventually want to add this to the base element for completeness / self-documentation, similar to other lifecycle methods.
| TemplateEngine.#clearObject(state); | ||
| TemplateEngine.#clearObject(arrayState); | ||
| } | ||
| switch (category) { |
There was a problem hiding this comment.
This is the crux of the change — we support five value types which can be bound into a content interpolation. Previously, the map and array where sort of mashed together into a single handler. However, the internal forks in that logic ended up being a bad factoring.
For example, now that array is handled on its own — it’s much easier to read (since it’s by far the simpler of the two cases).
|
|
||
| // TODO: #254: Future state where the “moveBefore” API is better-supported. | ||
| // // Loops over given array of “entries” to manage an array of nodes. | ||
| // static #commitContentArrayEntries(node, startNode, entries) { |
There was a problem hiding this comment.
Eventually (perhaps soon?), we’ll just uncomment this and delete the above handler. This pattern of encoding what’s next has been quite helpful (assuming these comments don’t linger indefinitely).
There was a problem hiding this comment.
can we just pull this into an open PR instead?
There was a problem hiding this comment.
My experience with trying to do that has just been stale PRs. I’m going to leave this as-is for now, but the goal will be to take up all these commented-out code blocks ahead of the 2.x release.
So, if it turns out we really cannot take this on ahead of 2.x I’ll likely just paste this snippet into #254 or something for tracking 👌
| @@ -867,9 +881,11 @@ describe('html rendering', () => { | |||
| // TODO: #254: See https://chromestatus.com/feature/5135990159835136. | |||
| it.todo('native map does not cause disconnectedCallback on list shuffle', () => { | |||
There was a problem hiding this comment.
Again — I did test this all out and indeed it does do the thing. One curiosity is that without the connectedMoveCallback registered, the behavior is different. My guess is that this is not the final behavior, but I will probably ask around to see if I can get more information there.
Things seem to be moving along pretty quickly for this new API. The motivation for this (versus
insertBefore) is that DOM state is not reset when you move DOM nodes around within the same connected tree.Importantly, that means a
disconnectedCallbackfollowed immediately by aconnectedCallbackwill no longer happen. Those two lifecycle events typically do some heavy lifting and we want to avoid them for cases like nodes simply being moved around within the same list or something.Note that this change set prepares us to leverage this new API, but it doesn’t yet commit to it as support is only currently in Chrome 133. We wouldn’t normally get ahead of ourselves like this — but there are a lot of positive signals from other browsers, so it seems like this will go pretty quickly.
Final note — I was able to confirm that shuffling a keyed array won’t cause disconnect / connect callbacks in the future!