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

Turbo uses Fetch which doesn't support progress #652

Closed james-em closed 2 years ago

james-em commented 2 years ago

Hi,

I'm here to ask for a new feature. I have a project where I can submit heavy files and I would like to be able to follow the progress. An easy way would be to catch the form submit event, cancel it, repeat it by Ajax and follow the progress.

Pseudo code of something working (partially) with Axios

  document.querySelectorAll('form.progressable').forEach((form) => {
    form.addEventListener('submit', (e) => {
      e.preventDefault();

      // Modal has a progress bar
      const modal = new Bootstrap.Modal(formSubmitModal, {});
      const formData = new FormData(e.target);

      modal.show();

      Axios.post(e.target.action, formData, {onUploadProgress: onUploadProgress, signal: axiosController.signal})
        .then((response) => {
          if (response.status === 200) {
            Turbo.visit(response.request.responseURL, {action: "replace"});
          } else if (response.status === 301 || response.status === 302) {
            Turbo.visit(response.headers.location, {action: "replace"});
          }
        })
        .catch(() => {
          modal.hide();
        });
    });
  });

The only issue with this code is that on invalid form submission (HTTP 422), I can't load the new response in the page so I lose the validation errors. (is there a way to?)

Best case scenario I would not listen to a form submit and I would instead somehow listen for progress on the form submission itself.

  document.addEventListener('turbo:submit-start', (e) => {
    console.log(e.detail.formSubmission.fetchRequest)
  });

Based on my research, it's not yet possible to listen for progress with Fetch It's planned: https://chromestatus.com/feature/5274139738767360

Is there any workaround?

tleish commented 2 years ago

The current workaround is to use XMLHttpRequest.upload instead of fetch.

The XMLHttpRequest upload property returns an XMLHttpRequestUpload object that can be observed to monitor an upload's progress.

see: Upload progress bar using XHR (Fetch alternative)

Note: I've read that XMLHttpRequest for upload on ReactNative runs 10x slower than fetch. So if using for a mobile app you will want to test to see this slow issue applies to mobile web in general, or just ReactNative.

james-em commented 2 years ago

MLHttpRequest for upload on ReactNative runs 10x slower than fetch. So if using for a mobile app you will want to test to see this slow issue applies to mobile web in general, or just ReactNative.

Thanks for the reply. I read the article and it's basically the same implementation I talked about using Axios on the first post but with XMLHttpRequest instead. It doesn't really answer the question because on form validation error 422 it will still cancel the Turbo visit and on HTTP 200 aswell.

Right now this is what I do but with Axios

form.addEventListener('submit', (e) => {
  /*
   * This cancel Turbo default behavior
   * /
  e.preventDefault();

........

 Axios.post(e.target.action, formData, {onUploadProgress: onUploadProgress, signal: axiosController.signal})
        .then((response) => {
          if (response.status === 200) {
            Turbo.visit(response.request.responseURL, {action: "replace"});
          } else if (response.status === 301 || response.status === 302) {
            // It seems the browser is hiding the 302 to Javascript or at least that we can't catch it even
            // with follow redirect disabled. Leaving here in case it ever changes...
            Turbo.visit(response.headers.location, {action: "replace"});
          }
        })
        .catch(({response}) => {
          if (response.status === 422) {
            const newForm = parseHTMLDocument(response.data).querySelector('body');
            document.body.innerHTML = newForm.innerHTML;
            document.dispatchEvent(new Event('page:load'));
          }
          modal.hide();
        });
    });

It would be nice to

Any solution? From my point of view, if Turbo wasn't using fetch, I could just read the progress from this and not alter any behavior.

document.addEventListener('turbo:submit-start', (e) => {
    console.log(e.detail.formSubmission.fetchRequest)
  });

Thanks in advance

tleish commented 2 years ago

Based on my research, it's not yet possible to listen for progress with Fetch

Ah, sorry my response was only focused on upload progress, I didn't address the validation error loss

The only issue with this code is that on invalid form submission (HTTP 422), I can't load the new response in the page so I lose the validation errors. (is there a way to?)

I believe it's possible to execute a Turbo#visit without sending another request by setting the response option. Something like:

         } else if (response.status === 301 || response.status === 302) {
            // It seems the browser is hiding the 302 to Javascript or at least that we can't catch it even
            // with follow redirect disabled. Leaving here in case it ever changes...
-           Turbo.visit(response.headers.location, {action: "replace"});
+           Turbo.visit(response.headers.location, {action: "replace", response: response.data});
});

if Turbo wasn't using fetch...

Fetch is at the core of Turbo Drive (caching). You could disable turbo drive and still use turbo-frames/turbo-streams if your main reasons for using turbo focus around turbo-frames or turbo-streams. From the turbo handbook

If you want Drive to be opt-in rather than opt-out, then you can set Turbo.session.drive = false; then, data-turbo="true" is used to enable Drive on a per-element basis. If you’re importing Turbo in a JavaScript pack, you can do this globally:

Turbo.session.drive = false

FYI, you might be interested in https://github.com/hotwired/turbo/pull/445 and https://github.com/hotwired/turbo-rails/pull/367. While the concept is focused on "breaking out of turbo-frames using the response from the server", there might be a similar approach to "breaking out of turbo response from the response of the server".

james-em commented 2 years ago

@tleish Hi again,

Thanks for the quick response. I tried doing instead of this

        .catch(({response}) => {
          modal.hide();

          if (response.status === 422) {
            const newForm = parseHTMLDocument(response.data).querySelector('body');
            document.body.innerHTML = newForm.innerHTML;
            document.dispatchEvent(new Event('page:load'));
          }
        });

This

.catch(({response}) => {
          modal.hide();
          if (response.status === 422) {
            Turbo.visit(location.href, {action: "replace", response: response.data});
          }
        });

There is no crash but the response is not used because there is no form validation errors shown unlike in the first scenario

tleish commented 2 years ago

I'm not sure I understand. Are you saying the fetch response includes the error, but when using it inside a Turbo.visit, it doesn't render the errors? What am I missing?

james-em commented 2 years ago

I'm not sure I understand. Are you saying the fetch response includes the error, but when using it inside a Turbo.visit, it doesn't render the errors? What am I missing?

Yes i'm saying when doing Turbo.visit(location.href, {action: "replace", response: response.data});

response.data includes de validation errors in the "HTML" string but they are not showing on my browser.

When I do this instead it is shown properly

            const newForm = parseHTMLDocument(response.data).querySelector('body');
            document.body.innerHTML = newForm.innerHTML;

It feels like response: response.data doesn't do much. Is it documented somewhere?

tleish commented 2 years ago

Looks like turbo expects the response to option in a specific format. You could try:

const visitResponse = {
  statusCode: response.status,
  redirected: false,
  responseHTML: response.data
}
Turbo.visit(location.href, {action: "replace", response: visitResponse});
james-em commented 2 years ago

@tleish Works perfectly!! Thank you. I'm happy with this workaround for now but I still think Fetch is lacking in terms of functionality :)

tleish commented 2 years ago

Glad you figured out it out. Sorry, can't help with browser fetch lacking in progress reporting.

james-em commented 2 years ago

@tleish I hadn't seen sooner, while it does work in generates javascript errors:

Capture d’écran, le 2022-08-03 à 11 51 44 Capture d’écran, le 2022-08-03 à 11 51 52

tleish commented 2 years ago

What version of turbo are you using?

james-em commented 2 years ago

What version of turbo are you using?

"@hotwired/stimulus": "^3.0.1",
"@hotwired/turbo-rails": "^7.1.0"

It seems all my Javascript is being reloaded because Google Map complains aswell to be already loaded

tleish commented 2 years ago

I know this error can happen when the turbo library is loaded twice. I'd need to see the full HTML original page and the responding page to help further.

james-em commented 2 years ago

I know this error can happen when the turbo library is loaded twice. I'd need to see the full HTML original page and the responding page to help further.

I tested on a blank page with only my javascript included and an empty body. Then I ran in my chrome console the following

Turbo.visit(location.href, { 
    action: "replace", 
    response: { statusCode: 422, redirected: false, responseHTML: '<html><head><script src="/assets/website-0d9b64a3eed4586efd830b75e49611d6047feba5f73692a54362d02907dbbc6d.js" data-turbo-track="reload" defer="defer"></script></head><body><h1>Test</h1></body></html>' }
})

where the script is the same javascript and I get the same error.

The only thing I have in my js file is

import { Turbo } from "@hotwired/turbo-rails"
window.Turbo = Turbo;

Edit

I woud like to add: if I change the HTTP Code from 422 to 200, it doesn't use the response html. It makes a new GET and use that response. Why?

tleish commented 2 years ago

where the script is the same javascript and I get the same error.

There's a conflict with the way the javascript loads that causing the turbo library to be loaded twice.

This is it working without any errors: https://codepen.io/tleish/pen/dymmMKg

if I change the HTTP Code from 422 to 200

Not sure.

tleish commented 2 years ago

If you have more question as you try and figure this out, I suggest closing this issue and using the community forums in asking for general help:

https://discuss.hotwired.dev/

james-em commented 2 years ago

https://codepen.io/tleish/pen/dymmMKg

Thanks for the help, I will head to the discuss website.

Just want to add that your exemple is flawed because the <head> is not including any javascript and you're Turbo.visit an HTML snipped that doesn't include any JS either.

My Rails application share the same <head>across all the pages and the Turbo is included in the JS file that is included in the <head>. I don't know what is the difference between doing what I'm doing and a click on an anchor <a> link, but the response is the same and it only generates an error when I'm manually trying to visit an HTML string response. When I click anchor link it works as expected

Edit: Linking issue https://discuss.hotwired.dev/t/turbo-visit-with-html-response-generates-error-an-is-inconsistent/4348 Edit 2: Workaround that works for now is

const { body } = (new DOMParser()).parseFromString(html, 'text/html');
document.body.replaceWith(body);