Allow turbo-cable-stream-source to be compatible with data-turbo-permanent

[bensheldon]

When the turbo-cable-stream-source element is disconnected from the DOM, wait a tick wait until the Turbo Render operation finishes to allow any potential DOM re-connect to occur. This prevents unnecessary channel unsubscribe/subscribe cycles when disconnected and then immediately reconnected with data-turbo-permanent and Turbo Drive.

This is possible because all of the Turbo Permanent / bardo.js operations are synchronous.

Related issues:

• <turbo-stream-source> does not work as permanent  turbo#868
• Stimilus component inside data-turbo-permanent are reinitialized turbo#471
• How to make permanent stimulusjs controller turbo#230
• An Out of Body Experience with Turbo... which makes me wonder if this entire class of problem could be sidestepped by moving permanent elements into a holding area in the dom, then moving them into the new page rather than removing and adding them (a little related in here and here)

[seanpdoyle]

Turbo relies on a variety of "timing" utilities internally, I wonder if any of them could be accessed here:

export function nextRepaint() {
  if (document.visibilityState === "hidden") {
    return nextEventLoopTick()
  } else {
    return nextAnimationFrame()
  }
}

export function nextAnimationFrame() {
  return new Promise((resolve) => requestAnimationFrame(() => resolve()))
}

export function nextEventLoopTick() {
  return new Promise((resolve) => setTimeout(() => resolve(), 0))
}

export function nextMicrotask() {
  return Promise.resolve()
}

nextEventLoopTick is the obvious candidate, though I wonder if any of the others would be more appropriate.

[bensheldon]

👍🏻 I'll take a look at what I can import.

[bensheldon]

It looks like none of the timing utilities are exported by Turbo. In the latest revision, I copied a named method called nextEventLoopTick. Not my favorite, but open to feedback (for example, I could add another file with like util.js).

[seanpdoyle]

this entire class of problem could be sidestepped by moving permanent elements into a holding area in the dom, then moving them into the new page rather than removing and adding them (hotwired/turbo#305 (comment))

This is a really interesting idea! The introduction of Element.moveBefore and the connectedMoveCallback could be extremely valuable (for both morphing and [data-turbo-permanent]).

[bensheldon]

I'll look at that test failure. Looks like it's relevant.

I appreciate the interest in beforeMove. The CanIUse on it looks pretty bleak right now unfortunately: https://caniuse.com/mdn-api_customelementregistry_define_connectedmovecallback_lifecycle_callback

[bensheldon]

@seanpdoyle I pushed up a fix for the failing test. I didn't notice that attributeChangedCallback was calling connectedCallback + disconnectedCallback; I extracted the subscribe/unsubscribe behavior separate from the newly introduced delay.

[bensheldon]

Thinking aloud, I'm wondering how we can make the lifecycle more resilient. Waiting for the nextTick works currently, but is dependent on all of the turbo permanent dom operations being synchronous, which maybe isn't a good future-forward expectation.

I think we could potentially track using turbo render events. As the order of events seems to be:

1. turbo:before-render
2. start turbo permanent operation
3. element is disconnected from dom
4. element is re-connected to dom
5. finish turbo permanent operation
6. turbo:render

I'll take a swing at that, and now that I'm a bit more familiar with the test suite, I'll see if I can lock those in too.

This turned out to be more fun and interesting than expected 😉

[bensheldon]

@seanpdoyle I just pushed up that change to hook into turbo:before-render and turbo:render instead of simply waiting a tick. The test I added was helpful.

[bensheldon]

Just noting this observer would be unnecessary if there was something like this: #498 (comment)

...as we could more simply add a listener to the page for a subscription-connected/subscription-disconnected event rather than have to set up a mutation observer.

[brunoprietog]

this entire class of problem could be sidestepped by moving permanent elements into a holding area in the dom, then moving them into the new page rather than removing and adding them (hotwired/turbo#305 (comment))

This is a really interesting idea! The introduction of Element.moveBefore and the connectedMoveCallback could be extremely valuable (for both morphing and [data-turbo-permanent]).

That's very interesting and would address the root cause of the problem. Currently, permanent elements in Turbo are not truly permanent, as they are detached and attached to the dom anyway. We can't use moveBefore, but we have Idiomorph for that, which uses it when available.