Reprocess elements after specific changes are made to them #156
Conversation
This reverts commit e00eb8a.
Co-authored-by: Matt Almeida <matt7@hey.com>
| function simulateMorph() { | ||
| const textNode = document.getElementById("one").firstChild | ||
| textNode.nodeValue = "changed" | ||
| } |
There was a problem hiding this comment.
| incrementUpdates: (element) => | ||
| @elements.get(element).updates++ | ||
| element.setAttribute("data-updates", @elements.get(element).updates) |
There was a problem hiding this comment.
Only done for testing purposes.
We mutate the element as a response to mutations, so this PR is prone to infinite loops if we're not careful. I think asserting the number of updates is a good protection against this.
| element = if target.nodeType is Node.TEXT_NODE | ||
| target.parentNode | ||
| else | ||
| target |
There was a problem hiding this comment.
Turbo morphs could change either/or — the data attributes on the element itself, or the value of the nested text node
|
|
||
| observe: (element) => | ||
| observer = new MutationObserver(@processMutations) | ||
| observer.observe(element, characterData: true, subtree: true, attributes: true, attributeFilter: OBSERVABLE_ATTRIBUTES) |
There was a problem hiding this comment.
Per Mozilla, setting textContent (which is what this library uses to update the element) removes all of the node's children and replaces them with a single text node with the given string value. So it shouldn't cause infinite loops due to this configuration.
This was also my experience while testing. But of course we'll want to thoroughly test on our apps before a public release.
|
@jorgemanrubia helpfully pointed out we might be able to simplify to a single observer + selector matching and an in-memory cache of the elements. I'll try that next. |
|
@josefarias any update on this? |
Hey @northeastprince. No updates, sorry. It's looking like it might be a while before I personally can find time during work to tackle this. Might tackle it recreationally in my time off but that's a big if as I've been rather busy. In the meantime, you might try the event listener from your linked PR or mark the |
Coming from #155 by @northeastprince
Fixes #151
This PR creates mutation observers for every
<time>element tracked by LocalTime so they'll be reprocessed when certain changes occur.We do this through a new class called
ElementObservationswhich keeps track of which elements already have observers, and also manages said observers.Note that not removing an observer after an element is removed from the dom can cause memory leaks. So we've modified our
PageObserverto notify the controller of any relevant<time>elements that are removed. Their observers are then promptly disconnected.Also note that we modify the
<time>elements as a response to their mutations. This is prone to infinite loops, so we must be careful. Thus, we're also addingPlaywrightto run integration tests so we can ship more confidently — and also assert that this will work with Turbo 8 morphs, which is the main motivation behind this change.As a side-effect, the integration-tests page can be accessed manually as a sort of playground:
Finally, if you wanted to test this out with actual Turbo 8 (instead of simulating a morph), you can do that by reverting 5ba6367. There's more info in the commit.