hotwired / turbo

The speed of a single-page web application without having to write any JavaScript
https://turbo.hotwired.dev
MIT License
6.65k stars 421 forks source link

Form submit not clearing cache #193

Closed jonsgreen closed 2 years ago

jonsgreen commented 3 years ago

I have a form that posts to an action that changes the state of an object and then redirects to a different page. If I then hit the back button I am given an old snapshot of the page and I need to refresh to get the correct view based on the updated state.

I would have expected Turbo to clear the cache in such cases but that appears to not be happening. I am wondering if this is expected behavior? If so then is there something simple I should be doing to get the desired behavior? If not then is this a bug or is there something I might be doing wrong?

I am happy to create an MVCE but thought I should check first before wasting time on something that is already clearly known.

jonsgreen commented 3 years ago

@seanpdoyle I apologize for pinging you directly on this issue but I have seen you show up in a lot on issues I have been watching and your support has been very valuable. I can imagine you are terribly busy and there must be an overwhelming amount of issues to address with Hotwire. For now I would be more than satisfied with just a few words on whether I have named something known or not and whether anything is likely to be changed in Hotwire to address this problem.

At the moment I am just turning off caching with the no-cache meta tag for the whole application to avoid these undesirable caching effects but that does not seem ideal long term if there are other options.

I would be happy to dig in and work on a solution if this seems possible and something that others would value.

seanpdoyle commented 3 years ago

Thank you for raising this, @jonsgreen.

I am happy to create an MVCE but thought I should check first before wasting time on something that is already clearly known.

To help narrow down the surface area a bit, a minimal example that reproduces the behavior would be valuable?

If you're able to add one, a failing functional test case would be a valuable alternative.

There are lines of code in the Navigator to clear the snapshot cache on a form submission success and a form submission failure.

In the meantime, a clearCache() function is exported from the @hotwired/turbo package, you can call it if the current implementation isn't covering your case.

jonsgreen commented 3 years ago

@seanpdoyle Thanks for responding. I have created an MVCE here. Hopefully it illustrates the problem and will help you answer whether there is a problem with our code, whether there is some simple change we can make to correct the behavior or whether there is something at fault in turbo.

I put some notes right in the README. I look forward to your response.

jonsgreen commented 3 years ago

@seanpdoyle I decided to dig into the code a bit to see if I could understand a bit more about what is happening. It is quite the maze for someone from the outside who doesn't know very much about the internals of Turbo. However, from what I can tell, as you mentioned above the cache does get cleared with the form submission success. However, it seem like a few lines down a visit is proposed and that seems to reestablish the snapshot cache. Not sure if that helps but thought I would point it out.

javan commented 3 years ago

I just confirmed this by fiddling around the console. Submitting a form clears the cache and then caches the current page (with the form that we don’t want cached) when navigating away.

jonsgreen commented 3 years ago

Thanks @javan for confirming this issue. I am curious if in the scope of things whether this would be considered a bug and if so how significant. Is there anyway that I can help in moving this issue forward?

weaverryan commented 3 years ago

Hi there!

I just hit this also - and I'm also trying to think of whether this is expected or a bug... and if a bug how to fix.

The behavior also happens form submitting a form without success:

A) Navigate to a page with a form (e.g. /register) B) Submit the form blank, it re-renders with validation errors C) Click to navigate away from the page D) Click back to the page with the form (e.g. /register)

On (D), the filled form with validation errors will show up briefly (the snapshot) before being replaced with a fresh form. This is... sort of expected: when you navigate on (C), like any other navigation, this takes a snapshot of the page (with the filled form and validation errors).

This seems tricky to fix. The only fix I can think of is to:

As a policy, disable snapshots on pages with forms. This would need to be done by the user with <meta name="turbo-cache-control" content="no-preview">

We could try to be smarter... and cache the page only in its "initial" (unsubmitted) form... and then not cache future snapshots. This would fix the "preview" problem (showing the filled out form as a preview when you click back to the page) but it would have the negative effect of showing the blank form on restoration visits (e.g. when I hit back, now the form would be blank, whereas currently, the form contains the filled-fields & errors from my submit).

scottjg commented 3 years ago

i'm running into this as well. in our case we're on an "edit" page and disable the button when submitting the form (data-disable-with). The rails app returns a redirect. When I hit the back button, i'm back at the edit form, but the submit button is still disabled. this was something that always "just worked" with turbolinks so i'm not sure the right way to fix this?

weaverryan commented 3 years ago

To clarify, I think there are 2 possible bugs - and fixing one is pretty clear:

A) On a successful POST, Turbo already clears the cache... but then re-caches the page that was just submitted - https://github.com/hotwired/turbo/issues/193#issuecomment-797042499 - and that seems like a clear mistake.

B) On unsuccessful POST, Turbo does not clear the cache... which causes problems... but the solution is not clear (unless Turbolinks already has a perfect solution) - https://github.com/hotwired/turbo/issues/193#issuecomment-833727739

Item (A) is probably a "lowish hanging fruit" - I didn't want my introduction of item (B) to cloud things.

Cheers!

altla commented 2 years ago

I'm also running into this issue. Not only when navigating back with the browsers' back button but also when navigating back to the form by clicking a link. The form retains the filled in values and the button status seems to be disabled.

@jonsgreen how did you get around this? I've currently disabled turbo on my form with data: {turbo: false} but am hoping for a better solution.

pinzonjulian commented 2 years ago

Regarding the compatibility between Rails ujs (button being disabled) and Turbo:

Please correct me if I'm wrong but my understanding is that data-disable-with was written with a different paradigm than Turbo and Stimulus and it is incompatible with Turbo's new form submissions.

In practice, this means that data-disable-with isn't aware of the lifecycle of Turbo and it doesn't clean up after itself; it doesn't return the button to its original state after it was used to submit a form. In Turbolinks (predecessor of Turbo) we didn't have complete form support so submitting a form would just blow the cache and issue a replace visit so you never ran into a cached disabled button. We didn't have the need back then for the button to return back to its original state prior to caching, and if we did, we would probably use the before-cache event to clean things up for the snapshot.

This is where Stimulus comes in. In a Stimulus controller the disconnect callback runs when the page's HTML disappears and before Turbo caches the snapshot. If you were to implement data-disable-with as a Stimulus controller, one option would be to use the disconnect callback to return the button to its original state before page caching.

Bottom line is: I don't think we have an answer to Rails UJS data-disable-with just yet. But Stimulus is more than capable to handle these scenario in a more robust way.

Also, there is a library called mrujs (Modern Rails UJS) which I believe serves as a modern alternative to the classic rails ujs and is compatible with Turbo.

altla commented 2 years ago

In my case I've actually removed Rails ujs. I also don't mind adding something like data: { disable_with: false } to the submit buttons. (Is it normal that I need to do this even though ujs is not installed?)

Still, the fact that all data is shown in the form seems like a problem. I haven't found any way to just disable the cache.

seanpdoyle commented 2 years ago

There's an ongoing discussion about a replacement for [data-disable] and [data-disable-with]: https://github.com/hotwired/turbo/pull/386

There's also an ongoing discussion about backwards compatible support for UJS: https://github.com/hotwired/turbo-rails/pull/257

As far as the snapshot cache is concerned, I wonder if it'd be worth introducing a skipSnapshot option or something like that to the VisitOptions, then passing it to the ensuing proposeVisit call following a form submission.

KonnorRogers commented 2 years ago

@pinzonjulian yes Mrujs currently accounts for data-disable-with on turbo:submit-start and turbo:submit-end

https://github.com/ParamagicDev/mrujs/blob/9fd403e66c9641955004f6f1047c0e75638f0f8d/src/elementEnabler.ts#L43

https://github.com/ParamagicDev/mrujs/blob/9fd403e66c9641955004f6f1047c0e75638f0f8d/src/elementDisabler.ts#L41

I plan to move these to the MrujsTurbo plugin, but currently they're in the core of Mrujs. I would like to do more work in this regard in reference to @seanpdoyle 's best practices by using buttons.

There is also some compatibility issues to be solved with mrujs in relation to TurboFrames, but perhaps that's out of the scope of Mrujs which will add data-turbo="false" to any form or link that has data-remote="true"

https://github.com/ParamagicDev/mrujs/blob/main/src/remoteWatcher.ts

pinzonjulian commented 2 years ago

In my case I've actually removed Rails ujs. I also don't mind adding something like data: { disable_with: false } to the submit buttons. (Is it normal that I need to do this even though ujs is not installed?)

Still, the fact that all data is shown in the form seems like a problem. I haven't found any way to just disable the cache.

You shouldn't need to do data: { disable_with: false }. That feature is not enabled by default in Rails UJS

mtomov commented 2 years ago

I was getting stale cache when using a turbo frame. In this case, added a reusable Stimulus controller to clear it up, as suggested using clearCache by @seanpdoyle . Thanks for the pointer!

// turbo_clear_cache_controller.js
// Add `data-controller="turbo-clear-cache"` to any form within a turbo frame
// that you'd like to clear the cache for

import { Controller } from 'stimulus'
import { clearCache } from '@hotwired/turbo'

export default class extends Controller {
  initialize() {
    this.afterRequest = this.afterRequest.bind(this)
  }

  connect() {
    this.element.addEventListener(
      'turbo:before-fetch-response',
      this.afterRequest
    )
  }

  disconnect() {
    this.element.removeEventListener(
      'turbo:before-fetch-response',
      this.afterRequest
    )
  }

  afterRequest({ detail }) {
    if (detail.fetchResponse.succeeded) {
      clearCache()
    }
  }
}
tleish commented 2 years ago

Does adding the following solve the issue?

const FetchMethod = { get: 0 };
document.addEventListener('turbo:submit-end', async ({ detail }) => {
  const nonGetFetch = detail.formSubmission.fetchRequest.method !== FetchMethod.get;
  const responseHTML = await detail.fetchResponse.responseHTML;
  if (detail.success && nonGetFetch && responseHTML) {
    Turbo.clearCache();
  }
});
maxwell commented 2 years ago
const FetchMethod = { get: 0 };
document.addEventListener('turbo:submit-end', async ({ detail }) => {
  const nonGetFetch = detail.formSubmission.fetchRequest.method !== FetchMethod.get;
  const responseHTML = await detail.fetchResponse.responseHTML;
  if (detail.success && nonGetFetch && responseHTML) {
    Turbo.clearCache();
  }
});

I had to add a timeout in order to make sure that the cache clearing happened after the errant "recache the page" behavior kicked in. Maybe 'turbo:submit-end' isn't late enough in the event stack?


const FetchMethod = { get: 0 };
document.addEventListener('turbo:submit-end', async ({ detail }) => {
  const nonGetFetch = detail.formSubmission.fetchRequest.method !== FetchMethod.get;
  const responseHTML = await detail.fetchResponse.responseHTML;
  if (detail.success && nonGetFetch && responseHTML) {
    setTimeout(() => {
      Turbo.clearCache();
    }, "1000");
  }
});