Turbo Frame with promoted navigation caching too early

[willcosgrove]

Hi, I think I've stumbled on an issue with the new feature of allowing a Turbo Frame to drive navigation. It appears that the page is being cached earlier than it does when a Turbo Frame is not driving navigation.

I have a video recorded that shows 3 UI bugs that I think are all explained by this.

CleanShot.2021-11-24.at.13.57.00-converted.2.mp4

So everything visible in the window is in a Turbo Frame (there is UI above scrolled out of view that is not in the Turbo Frame). There is a form that encompasses all the filters in the dropdowns. The form is doing a GET request to load new results with the applied filters. Here are the 3 UI bugs to look for:

1. The date dropdown has a stimulus controller that, upon connect() instantiates a Litepicker, and upon disconnect() destroys the litepicker, removing its DOM. After restoring the initial page visit, there are two sets of Litepicker DOM in the dropdown. This indicates that a snapshot was taken before disconnect() was called.
2. The button in the Carrier dropdown gets disabled by Turbo and there is some disable-specific text inside the button. After restoring the initial page visit, the button remains disabled. This indicates to me that the snapshot was taken before Turbo re-enabled the button.
3. There is a Stimulus controller that wraps the filter form that is in charge of resetting the state of the inputs when the dropdown closes, so if the user checks a box and closes the dropdown without clicking "Apply" the checkbox will revert to its initial state. This stimulus controller adds a listener to document for turbo:before-cache that resets the form. But after restoring the initial page visit, the checkbox is still checked. It doesn't get unchecked until the dropdown is opened, and then closed. So the snapshot seems to be happening before turbo:before-cache gets called.

Here's the abbreviated markup of the Turbo Frame

turbo-frame#manifests data-turbo-action="advance" autoscroll=true data-autoscroll-block="start"
  .pb-3.sm:flex.sm:items-center.sm:justify-between
    .flex-1
      = render 'filters'
  / ...

And because it is the smallest controller to include for an example, here is that reset controller:

import { Controller } from "@hotwired/stimulus"

export default class extends Controller {
  static targets = ["reset"]

  connect() {
    document.addEventListener("turbo:before-cache", this.reset)
  }

  disconnect() {
    document.removeEventListener("turbo:before-cache", this.reset)
    this.reset()
  }

  reset = () => {
    if (this.element.reset) {
      this.element.reset()
    }
    for (let target of this.resetTargets) { target.reset() }
  }
}

Let me know if there's any other info I can provide!

[seanpdoyle]

@willcosgrove thank you for opening this issue with this much context and information!

I'm having a little trouble reproducing your scenario outlined within the Turbo test suite, so I've resorted to manual testing while experimenting with a fix.

I've attempted to resolve the timing issues on a branch on my fork: https://github.com/hotwired/turbo/compare/seanpdoyle:frame-visit-snapshot-timing. Could you test if that build resolves the underlying issues?

[willcosgrove]

Thanks for working on this!

I'm going to show my ignorance here, but what is the most straightforward way to test this build on my local project? It looks like that branch doesn't have any files in dist so I'm assuming I can't just point to your repo and branch in my package.json file. I don't think my webpack is configured to run things in node_modules through any special loaders, like typescript.

[seanpdoyle]

If you clone the turbo repository locally (either from hotwired/ or seanpdoyle/ or any other GitHub repository), you can build its output assets to the dist/ directory:

• yarn install to install Turbo's dependencies (including TypeScript)
• yarn build to compile and output
• yarn link to signal to Yarn that you'd like to use a local version for the package

If you're dependent on Turbo through hotwired/turbo-rails, you'll need to repeat the process for that repository:

• yarn link @hotwired/turbo to signal to Yarn that you want to use your local version of Turbo
• yarn install to install Turbo Rails' dependencies (including the local Turbo)
• yarn build to compile and output

Then, within your application's directory:

• yarn link @hotwired/turbo-rails (if you're depending on Turbo through the Rails plugin)
• yarn link @hotwired/turbo (if you're depending on Turbo directly)
• yarn install to install the dependencies, including the local Turbo(-Rails) version

That should integrate your local branches. You can use the same method to noodle around with debugging or troubleshooting Turbo changes. If you're making changes to Turbo or Turbo Rails, you'll need to re-build each package (and potentially re-install the dependencies) to have the changes materialize in your application.

It's worth noting that once yarn link is invoked, the link sticks around until you yarn unlink $PACKAGE_NAME.

[willcosgrove]

Thanks for the detailed explanation! I will test your changes out this morning and report back.

[willcosgrove]

@seanpdoyle I can confirm that fixed it! Thank you for all your help 🙇‍♂️

[willcosgrove]

Hey @seanpdoyle, sorry to revive this issue, but it appears that there's been a regression at some point. We just updated to 7.2.0 a few days ago, and I noticed that this bug is back.

I am unfortunately not familiar enough with Turbo internals to figure out what changed between then and now that would have caused the problem. I did run a git bisect to see where the problem was introduced, and from that I found this commit: 837e977 which is the very next commit on main. But the behavior at that point was different. It appeared to not be restoring any snapshots when I hit the back button. So it was broken in a different way. But at 7.2.0 it is restoring the snapshots when I use the back button, but it's got the incorrect form state, and the double date picker as it did before #488 .

Let me know if there's anything more I can do to assist in tracking this down. Sorry I don't know more about the Turbo internals to be of more help.

[seanpdoyle]

Thank you for opening this back up for discussion @willcosgrove.

This might be addressed by #746. Are you able to try to reproduce your issue against that branch?

[willcosgrove]

I just checked it out, and tried it against my project and unfortunately it doesn't seem to fix the issue. When I hit the back button the form state is still checked, and the datepicker is duplicated 😕

[brunoalod]

This is still not working.

[brunoalod]

So, since Github actually uses Turbo, I looked at the HTML and applying target="_top" fixes it. Not sure why.

[PedroAugustoRamalhoDuarte]

Hello @willcosgrove, do you have any solution for this one? I am facing the problem with select2 picker.