Snapshot caching issue after upgrading from turbolinks to turbo

[thanosbellos]

Hello.
Since the Turbolinks - Deferred snapshot caching pull request was merged, i have been using disconnect method to teardown my controller(i.e deinitialize my uppy file uploader and remove the custom file input from the dom).

I have upgraded to use Turbo (Drive) recently and i have noticed that i got a different behaviour using the exact same code

In order to be sure that it hadn't something to do with my config or with the js library i was using(uppy), i created two demo projects, one that uses turbo drive and one that uses turbolinks and a simple stimulus controller that adds/removes elements when connecting/disconnecting:

• Demo-turbolinks
• Demo-turbo-drive

The stimulus controller is adding an element when connecting and removing the element when disconnecting:

import { Controller } from "stimulus";

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

  connect() {
    this.outputTarget.innerHTML += "<p>Hello, Stimulus!</p>";
  }

  disconnect() {

    const child = this.outputTarget.firstChild;
    this.outputTarget.removeChild(child);
    // or     this.outputTarget.innerHTML += "";
  }
}

The behaviour with turbo drive:

---

The behaviour with turbolinks

Tested on Mac Os Big Sur on latest Safari and Mozilla firefox developer edition (latest version)
Rails 6.1.0 with latest version of turbo-rails and turbolinks.

[thanosbellos]

i believe that #102 is also related with my experience after upgrading to Turbo drive.

[sstephenson]

Would you mind testing with v7.0.0-beta.4 to see if the issue has been fixed?

[thanosbellos]

Hallo, i have checked it and the issue still exists but it has really improved in comparison to the v7.0.0-beta2

Here it is a gif with how it behaves in the latest version of turbo:

branch with sample project testing turbo-v7.0.0beta4

Edit - discard this comment.

[thanosbellos]

I am really sorry, i think i shouldn't have used firstChild. I changed my code to this and it works perfectly.

// Visit The Stimulus Handbook for more details
// https://stimulusjs.org/handbook/introduction
//
// This example controller works with specially annotated HTML like:
//
// <div data-controller="hello">
//   <h1 data-target="hello.output"></h1>
// </div>

import { Controller } from "stimulus";

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

  connect() {
    console.log("Connecting");
    const p = document.createElement("p");
    p.innerHTML = "hellos stimulus";
    this.outputTarget.appendChild(p);

    if (this.isPreview) {
      console.log("Connecting from a preview");
    }
  }

  disconnect() {
    console.log("disconnecting");
    const child = this.outputTarget.firstElementChild;
    console.log(child);
    this.outputTarget.removeChild(child);
    console.log(this.outputTarget);
  }

  get isPreview() {
    return document.documentElement.hasAttribute("data-turbo-preview");
  }
}

Moreover and most importantly i checked it with some controllers that would reset some libraries (uppy, tiny-slider, flatpickr), or my custom controllers and the problem seems to have disappeared.. I will do some more checks though in order to be sure..

Again i am really sorry for my previous message. Thanks a lot for your efforts and your work.

[jonsgreen]

I might be having a similar problem in which a stimulus controller is being connected twice on a return to a page even though I am telling my connect logic to only run if it is not a preview. This is actually an even bigger deal because I am trying to establish an action cable connection and perhaps because of this race condition issue (not sure) I often don't get a confirmation and lose cable connection to the server.

What is even more strange is that if I add a no-preview meta tag to the page then the issue does not happen so I am pretty sure that a preview of some sort is happening. I don't know the turbo code at all but the little I look at it I am wondering if perhaps isPreview is just always coming out false: https://github.com/hotwired/turbo-rails/blob/main/app/assets/javascripts/turbo.js#L2544.

Any updates would be great and if there is anyway I can help with debugging or solving this problem I am happy to contribute.

[seanpdoyle]

@thanosbellos thank you for opening this.

Is the behavior restored by #162?

[jonsgreen]

@seanpdoyle I would also really like to try out your branch to see if it will fix my problem but I am not quite sure of how to use it locally since I am requiring turbo-rails but your branch is just turbo and I am not quite sure how to replace my installation to use it. Please advise, thanks!

[thanosbellos]

@seanpdoyle Hi and thanks for your response. I finally got some time to test this pull request out.

data-turbo-preview behaviour is restored. Moreover some issues with back/forward to a page with a controller i had (stimulus-components/stimulus-lightbox) were also fixed.

Just for reference:
How it behaved with the latest beta(v7.0.0-beta.4)
https://user-images.githubusercontent.com/425283/107262259-fd9c4a80-6a48-11eb-8fe0-edde8d2a6c37.mp4

Here the fixed behaviour after using #162
https://user-images.githubusercontent.com/425283/107262440-3b00d800-6a49-11eb-85bc-b1185baeb86a.mp4

Thanks a lot for your efforts and your work.

@jonsgreen
I downloaded the pull request, build it, got the dist folder and threw it in my project's node_modules/@hotwired/turbo/ folder replacing the old dist folder. It ain't sophisticated but it worked :P

[jonsgreen]

This definitely gets a 👍 from me. I am now able to block subscribing by the usual check for preview which prevents the race condition I was observing.

I am curious how soon this might get merged in to master @seanpdoyle? Seems like this is an important bugfix? Thanks so much for attending to this.

Thanks @thanosbellos for reporting this issue and the tips for how to test locally.

[seanpdoyle]

I believe this was fixed as part of #169, which shared implementation change overlap with #162.

[thanosbellos]

Hm.. Interesting.. As i mentioned above, when i tried the #162 there was an unexpected gain in one of my controllers besides the restoration of turbo preview.

@seanpdoyle Hi and thanks for your response. I finally got some time to test this pull request out.

data-turbo-preview behaviour is restored. Moreover some issues with back/forward to a page with a controller i had (stimulus-components/stimulus-lightbox) were also fixed.

Just for reference:
How it behaved with the latest beta(v7.0.0-beta.4)
https://user-images.githubusercontent.com/425283/107262259-fd9c4a80-6a48-11eb-8fe0-edde8d2a6c37.mp4

Here the fixed behaviour after using #162
https://user-images.githubusercontent.com/425283/107262440-3b00d800-6a49-11eb-85bc-b1185baeb86a.mp4

I just tried the same controller with the main branch (after #169 was merged). The date-preview issue is gone.

In my surprise though, the issue with that controller, that was gone using the #162, is now still there. I will dive into it in the weekend.

[jonsgreen]

I am curious if there are any updates on this fix? Has this actually been merged and if so when would we expect it to be in a new release?

[seanpdoyle]

@jonsgreen #169 has been merged into main. If that resolves the issue, then the fix is available, but has not yet been released as part of the upcoming v7.0.0.beta5 release.

[thanosbellos]

Just for clarity, i close this issue due to the fact that since the problems i initially described have been solved with the release v7.0.0.beta4.

The following related issue has been resolved on the main branch.

I might be having a similar problem in which a stimulus controller is being connected twice on a return to a page even though I am telling my connect logic to only run if it is not a preview. This is actually an even bigger deal because I am trying to establish an action cable connection and perhaps because of this race condition issue (not sure) I often don't get a confirmation and lose cable connection to the server.

What is even more strange is that if I add a no-preview meta tag to the page then the issue does not happen so I am pretty sure that a preview of some sort is happening. I don't know the turbo code at all but the little I look at it I am wondering if perhaps isPreview is just always coming out false: https://github.com/hotwired/turbo-rails/blob/main/app/assets/javascripts/turbo.js#L2544.

Any updates would be great and if there is anyway I can help with debugging or solving this problem I am happy to contribute.

Thanks a lot for your work and help!!