Support Turbo 8 same-page refreshing + morphing

[jayohms]

This builds on the great work by @pfeiffer in #177 and fixes #175, #136 and #160.

These changes allow same-page morph refreshes in Turbo 8 to refresh the current page contents without visual flicker, without displaying the activity indicator, and without displaying transitional screenshots.

These changes maintain backwards compatibility with Turbo 7 and Turbolinks 5. Additionally, it adds proper debug logging, so it's obvious when page refresh are occurring in an app.

See #177 for additional details and context. The corresponding turbo-android PR with similar changes is here: hotwired/turbo-android#292

[pfeiffer]

Wondering if this is sufficient to detect a page refresh - the corresponding check in Turbo compares the pathname of the two URLs (so replacing to a different query string is also considered a refresh) as well as explicitly checking the that the action is replace:

  isPageRefresh(visit) {
    return !visit || (this.lastRenderedLocation.pathname === visit.location.pathname && visit.action === "replace")
  }

I'm thinking the behavior should be the same here.

Preferrably, Turbo made available a Turbo.navigator.isPageRefresh(location, options) that could be used here and also by Turbo itself, similar to locationWithActionIsSamePage(..) used a few lines above. Any future changes in the logic to detect refreshes in Turbo library would also be applied here without the need for native updates.

What are your thoughts? Happy to wrap up a PR! CC @afcapel, @jayohms

[jayohms]

There are two separate concerns here, even though they're related. The first is answering the question, when do we want to potentially (re)create new ViewController instances? Previously, revisiting the same url as the current Visit/ViewController would go through the whole native proposal callback lifecycle.

I don't think this is necessary anymore going forward for replace or advance visits — even for when Turbo 8 morphing is not enabled (and for Turbo 7). If the same visit url is being proposed, let the adapter handle it completely and transparently now without letting it be an app decision. The fact that we hadn't done this previously was mostly a legacy concern before Strada components made it possible to observe state in the WebView and maintain proper state with native features built on top of the WebView. Recreating the ViewController was a blunt way to force the screen to update its state. Strada allows apps to solve this in an elegant way, now.

The second concern is does the library properly handle Turbo 8 morph refreshes? With this PR, it looks like the answer is "yes".

So, while the implementations are slightly different, I don't think we should limit ViewController visit "refreshes" to when Turbo 8 morphing is used or even just replace visits. The concern about using .href vs .pathname is valid, though — and we should decide if we want to handle it in the same way. The only concern I have here is you could theoretically setup a path configuration to handle different query urls for different ViewController implementations. Maybe that use case is an edge case that's not worth worrying about, though.

Thoughts @pfeiffer?

[pfeiffer]

@jayohms, I've been giving it some thought and I think what you say here makes a lot of sense. In answering the question "should this visit trigger a new view controller?", I suppose the answer is no, no matter if it's a replace or advance action, as long as the URL is the same.

The same happen in a browser context, where clicking a link element pointing to the same URL as the current URL, actually reloads the page, without pushing it onto the history stack. This is true for non-Turbo websites as well as Turbo-enabled websites.

Regarding .pathname vs .href, the behavior in a browser context would be to push a new item onto the history stack if the href's don't match. I guess the current behavior in this PR actually does the same; if the query params differ, a new visit will be proposed to the native side and what you mention with a path configuration handling different query urls for different ViewController implementation would be supported.

What could be considered is adding support for the pathname + replace visit as Turbo Web also does, eg letting the normal adapter perform the visit without proposing a visit on the native side in the case that:

1. The href's match OR
2. The pathname's match AND action = replace

const currentLocation = Turbo.navigator.location

currentLocation?.href === location.href ||
  (currentLocation?.pathname === location.pathname && options.action === "replace")

This would ensure that a replace with potential morph happen when query params change, but only if a replace action was explicitly provided. This matches the behavior on Turbo web.

[pfeiffer]

One could argue that the behavior of an advance action on Turbo web to the exact same href should also be considered a page refresh and use morphing, though 🤷🏻‍♂️ I mean; no history is pushed and it is a request to refresh the page.

[jayohms]

Great idea @pfeiffer, I like that balance. Could you put together a PR with this isolated change?

[jayohms]

Perfect, thanks!

[brunoprietog]

@jayohms sorry, I didn't quite understand this. Is there a special reason why you decided to use href instead of pathname? The behavior changed in hotwired/turbo#1079, you can read more about the argumentation there. Why is it different? Or is this what @pfeiffer intends to change? The same would apply to Turbo Android as well. I can do a PR on both libraries to correct that if that seems appropriate to you. Thanks!

[pfeiffer]

@brunoprietog @jayohms - I was starting to implement the pathname comparison as discussed above, but ultimately I've come to the conclusion that it will lead to unexpected behavior, and that I think the href only is a better a comparison here unless some other changes are made to the library:

In difference to the browser context, any Visitable drives navigations with it's URL. When doing refreshes bypassing the native adapters to what is essentially a different URL (same path, different params), the Visitable loose track of the URL it's displaying. The problem is that this URL is used various places, eg when reloading a Session natively. Strada components also filter messages by their location, so a difference in document.location.href vs visitableURL could lead to a broken Strada bridge.

Imagine this example:

1. Open screen A (/one)
2. On that screen, a replace visit to /one?tab=latest) is performed; as the pathnames are equal, native adapter proposal is bypassed.
3. Initiate a pull-to-refresh action on the screen - the Session will reload /one, not /one?tab=latest as expected

Ultimately, I think any visit that would change the document.location.href (except anchor navigations) should be proposed natively, so I don't think there's need for changing the behavior introduced in this PR.

[brunoprietog]

I regret we have that limitation, I honestly wish I had the same behavior as the browser adapter. Maybe there the conceptual design error is to think that doing a refresh is simply triggering a reload of the exact current URL, rather than executing a replace action. But I haven't gotten too deep into Turbo native to go into more depth.

[jayohms]

@pfeiffer You bring up some great points — right now any Visitable destination treats its location as immutable for the current ViewController. I think what we have right now will work for most situations, but we may have to tweak some fundamental assumptions in the native libraries if we want to make sure everything works consistently with the web browser adapter long-term.

@brunoprietog I think we can align things, but it's not a trivial change right now with how the iOS and Android libraries are designed.

Maybe there the conceptual design error is to think that doing a refresh is simply triggering a reload of the exact current URL, rather than executing a replace action.

It's more complicated than this. Any unique location path pattern can map to unique ViewController instances — native or web. So, any visit proposal with a url change could require a new ViewController instance, even for replace actions. There's an opportunity to be smarter about this, but it'd be quite a bit of work.

I'm going to cut a new release of turbo-ios and turbo-android with what we have now, since it's a big improvement over what we had previously. We can improve further improve things later.