Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Progress Bar finishes prematurely #884

Open
tropperstyle opened this issue Feb 28, 2023 · 4 comments · May be fixed by #1039
Open

Progress Bar finishes prematurely #884

tropperstyle opened this issue Feb 28, 2023 · 4 comments · May be fixed by #1039

Comments

@tropperstyle
Copy link

tropperstyle commented Feb 28, 2023 •
edited
Loading

I have backend server that streams responses to the client as soon as possible, so the browser can start painting while a data is still pending.

<!DOCTYPE html>
<html lang="en">
  <head>
    <meta charset="utf-8">
    <title>HTML Streaming</title>
    <link rel="stylesheet" href="style.css">
    <script src="script.js"></script>
  </head>
  <body>
    <!-- Paused here, waiting for data -->

So even if await fetch() returns, we still only have the response headers, but are still waiting for the body to complete. Turbo's progress bar is really jarring in this case. It quickly jumps to 100%, then there is a long delay before the page actually navigates.

I added await here which fixes my use-case, but it introduced a failing test which I'm not sure what to do with:

  1) [chrome] › functional/frame_tests.ts:594:1 › test navigating pushing URL state from a frame navigation fires events 

    AssertionError: sets aria-busy on the <html>: expected null to equal 'true'

      606 |   assert.notOk(await nextAttributeMutationNamed(page, "frame", "aria-busy"), "removes aria-busy from the <turbo-frame>")
      607 |
    > 608 |   assert.equal(await nextAttributeMutationNamed(page, "html", "aria-busy"), "true", "sets aria-busy on the <html>")
          |          ^
      609 |   await nextEventOnTarget(page, "html", "turbo:before-visit")
      610 |   await nextEventOnTarget(page, "html", "turbo:visit")
      611 |   await nextEventOnTarget(page, "html", "turbo:before-cache")
@seanpdoyle
Copy link
Contributor

@tropperstyle thank you for opening this issue.

In the time since you've opened it, this project has undergone significant structural changes, and files have been renamed.

The "here" that you link to in your description references a file that no longer exists. The code referenced by src/http/fetch_request.ts now resides in src/http/fetch_request.js.

Could you re-share the line number that you've prefixed with await?

@tropperstyle
Copy link
Author

Sure, I also updated my previous comment: https://github.com/hotwired/turbo/blob/v7.3.0/src/http/fetch_request.ts#L134

seanpdoyle added a commit to seanpdoyle/turbo that referenced this issue Oct 13, 2023
@seanpdoyle
await for FetchRequest delegate to handle response
f87297d 
Closes [hotwired#884]

Despite the fact that `Visit` and `FrameController` are `FetchRequest`
delegate classes that implement their delegate methods asynchronously on
success ([Visit.requestSucceededWithResponse][] and
[FrameController.requestSucceededWithResponse][]) and on failure
([Visit.requestFailedWithResponse][]) and
[FrameController.requestFailedWithResponse][]), the `async
FetchRequest.receive` method doesn't use the `await` keyword to wait for
those delegates to finish handling their callbacks.

This commit adds those `await` clauses.

[Visit.requestSucceededWithResponse]: https://github.com/hotwired/turbo/blob/c207f5b25758e4a084e8ae42e49712b91cf37114/src/core/drive/visit.js#L297
[FrameController.requestSucceededWithResponse]: https://github.com/hotwired/turbo/blob/c207f5b25758e4a084e8ae42e49712b91cf37114/src/core/frames/frame_controller.js#L210
[Visit.requestFailedWithResponse]: https://github.com/hotwired/turbo/blob/c207f5b25758e4a084e8ae42e49712b91cf37114/src/core/drive/visit.js#L311
[FrameController.requestFailedWithResponse]: https://github.com/hotwired/turbo/blob/c207f5b25758e4a084e8ae42e49712b91cf37114/src/core/frames/frame_controller.js#L215

[hotwired#884]: hotwired#884
@seanpdoyle seanpdoyle linked a pull request Oct 13, 2023 that will close this issue
Draft
@seanpdoyle
Copy link
Contributor

Thank you! I've opened #1039. I'd appreciate review. Could we move conversation to that PR?

seanpdoyle added a commit to seanpdoyle/turbo that referenced this issue Oct 13, 2023
@seanpdoyle
await for FetchRequest delegate to handle response
2a7c917 
Closes [hotwired#884]

Despite the fact that `Visit` and `FrameController` are `FetchRequest`
delegate classes that implement their delegate methods asynchronously on
success ([Visit.requestSucceededWithResponse][] and
[FrameController.requestSucceededWithResponse][]) and on failure
([Visit.requestFailedWithResponse][]) and
[FrameController.requestFailedWithResponse][]), the `async
FetchRequest.receive` method doesn't use the `await` keyword to wait for
those delegates to finish handling their callbacks.

This commit adds those `await` clauses.

[Visit.requestSucceededWithResponse]: https://github.com/hotwired/turbo/blob/c207f5b25758e4a084e8ae42e49712b91cf37114/src/core/drive/visit.js#L297
[FrameController.requestSucceededWithResponse]: https://github.com/hotwired/turbo/blob/c207f5b25758e4a084e8ae42e49712b91cf37114/src/core/frames/frame_controller.js#L210
[Visit.requestFailedWithResponse]: https://github.com/hotwired/turbo/blob/c207f5b25758e4a084e8ae42e49712b91cf37114/src/core/drive/visit.js#L311
[FrameController.requestFailedWithResponse]: https://github.com/hotwired/turbo/blob/c207f5b25758e4a084e8ae42e49712b91cf37114/src/core/frames/frame_controller.js#L215

[hotwired#884]: hotwired#884
seanpdoyle added a commit to seanpdoyle/turbo that referenced this issue Oct 13, 2023
@seanpdoyle
await for FetchRequest delegate to handle response
d2478b5 
Closes [hotwired#884]

Despite the fact that `Visit` and `FrameController` are `FetchRequest`
delegate classes that implement their delegate methods asynchronously on
success ([Visit.requestSucceededWithResponse][] and
[FrameController.requestSucceededWithResponse][]) and on failure
([Visit.requestFailedWithResponse][]) and
[FrameController.requestFailedWithResponse][]), the `async
FetchRequest.receive` method doesn't use the `await` keyword to wait for
those delegates to finish handling their callbacks.

This commit adds those `await` clauses.

[Visit.requestSucceededWithResponse]: https://github.com/hotwired/turbo/blob/c207f5b25758e4a084e8ae42e49712b91cf37114/src/core/drive/visit.js#L297
[FrameController.requestSucceededWithResponse]: https://github.com/hotwired/turbo/blob/c207f5b25758e4a084e8ae42e49712b91cf37114/src/core/frames/frame_controller.js#L210
[Visit.requestFailedWithResponse]: https://github.com/hotwired/turbo/blob/c207f5b25758e4a084e8ae42e49712b91cf37114/src/core/drive/visit.js#L311
[FrameController.requestFailedWithResponse]: https://github.com/hotwired/turbo/blob/c207f5b25758e4a084e8ae42e49712b91cf37114/src/core/frames/frame_controller.js#L215

[hotwired#884]: hotwired#884
seanpdoyle added a commit to seanpdoyle/turbo that referenced this issue Oct 27, 2023
@seanpdoyle
await for FetchRequest delegate to handle response
0b64bb1 
Closes [hotwired#884]

Despite the fact that `Visit` and `FrameController` are `FetchRequest`
delegate classes that implement their delegate methods asynchronously on
success ([Visit.requestSucceededWithResponse][] and
[FrameController.requestSucceededWithResponse][]) and on failure
([Visit.requestFailedWithResponse][]) and
[FrameController.requestFailedWithResponse][]), the `async
FetchRequest.receive` method doesn't use the `await` keyword to wait for
those delegates to finish handling their callbacks.

This commit adds those `await` clauses.

[Visit.requestSucceededWithResponse]: https://github.com/hotwired/turbo/blob/c207f5b25758e4a084e8ae42e49712b91cf37114/src/core/drive/visit.js#L297
[FrameController.requestSucceededWithResponse]: https://github.com/hotwired/turbo/blob/c207f5b25758e4a084e8ae42e49712b91cf37114/src/core/frames/frame_controller.js#L210
[Visit.requestFailedWithResponse]: https://github.com/hotwired/turbo/blob/c207f5b25758e4a084e8ae42e49712b91cf37114/src/core/drive/visit.js#L311
[FrameController.requestFailedWithResponse]: https://github.com/hotwired/turbo/blob/c207f5b25758e4a084e8ae42e49712b91cf37114/src/core/frames/frame_controller.js#L215

[hotwired#884]: hotwired#884
seanpdoyle added a commit to seanpdoyle/turbo that referenced this issue Nov 16, 2023
@seanpdoyle
await for FetchRequest delegate to handle response
b714f1f 
Closes [hotwired#884]

Despite the fact that `Visit` and `FrameController` are `FetchRequest`
delegate classes that implement their delegate methods asynchronously on
success ([Visit.requestSucceededWithResponse][] and
[FrameController.requestSucceededWithResponse][]) and on failure
([Visit.requestFailedWithResponse][]) and
[FrameController.requestFailedWithResponse][]), the `async
FetchRequest.receive` method doesn't use the `await` keyword to wait for
those delegates to finish handling their callbacks.

This commit adds those `await` clauses.

[Visit.requestSucceededWithResponse]: https://github.com/hotwired/turbo/blob/c207f5b25758e4a084e8ae42e49712b91cf37114/src/core/drive/visit.js#L297
[FrameController.requestSucceededWithResponse]: https://github.com/hotwired/turbo/blob/c207f5b25758e4a084e8ae42e49712b91cf37114/src/core/frames/frame_controller.js#L210
[Visit.requestFailedWithResponse]: https://github.com/hotwired/turbo/blob/c207f5b25758e4a084e8ae42e49712b91cf37114/src/core/drive/visit.js#L311
[FrameController.requestFailedWithResponse]: https://github.com/hotwired/turbo/blob/c207f5b25758e4a084e8ae42e49712b91cf37114/src/core/frames/frame_controller.js#L215

[hotwired#884]: hotwired#884
seanpdoyle added a commit to seanpdoyle/turbo that referenced this issue Nov 29, 2023
@seanpdoyle
await for FetchRequest delegate to handle response
36a565d 
Closes [hotwired#884]

Despite the fact that `Visit` and `FrameController` are `FetchRequest`
delegate classes that implement their delegate methods asynchronously on
success ([Visit.requestSucceededWithResponse][] and
[FrameController.requestSucceededWithResponse][]) and on failure
([Visit.requestFailedWithResponse][]) and
[FrameController.requestFailedWithResponse][]), the `async
FetchRequest.receive` method doesn't use the `await` keyword to wait for
those delegates to finish handling their callbacks.

This commit adds those `await` clauses.

[Visit.requestSucceededWithResponse]: https://github.com/hotwired/turbo/blob/c207f5b25758e4a084e8ae42e49712b91cf37114/src/core/drive/visit.js#L297
[FrameController.requestSucceededWithResponse]: https://github.com/hotwired/turbo/blob/c207f5b25758e4a084e8ae42e49712b91cf37114/src/core/frames/frame_controller.js#L210
[Visit.requestFailedWithResponse]: https://github.com/hotwired/turbo/blob/c207f5b25758e4a084e8ae42e49712b91cf37114/src/core/drive/visit.js#L311
[FrameController.requestFailedWithResponse]: https://github.com/hotwired/turbo/blob/c207f5b25758e4a084e8ae42e49712b91cf37114/src/core/frames/frame_controller.js#L215

[hotwired#884]: hotwired#884
seanpdoyle added a commit to seanpdoyle/turbo that referenced this issue Nov 29, 2023
@seanpdoyle
await for FetchRequest delegate to handle response
88c76e1 
Closes [hotwired#884]

Despite the fact that `Visit` and `FrameController` are `FetchRequest`
delegate classes that implement their delegate methods asynchronously on
success ([Visit.requestSucceededWithResponse][] and
[FrameController.requestSucceededWithResponse][]) and on failure
([Visit.requestFailedWithResponse][]) and
[FrameController.requestFailedWithResponse][]), the `async
FetchRequest.receive` method doesn't use the `await` keyword to wait for
those delegates to finish handling their callbacks.

This commit adds those `await` clauses.

[Visit.requestSucceededWithResponse]: https://github.com/hotwired/turbo/blob/c207f5b25758e4a084e8ae42e49712b91cf37114/src/core/drive/visit.js#L297
[FrameController.requestSucceededWithResponse]: https://github.com/hotwired/turbo/blob/c207f5b25758e4a084e8ae42e49712b91cf37114/src/core/frames/frame_controller.js#L210
[Visit.requestFailedWithResponse]: https://github.com/hotwired/turbo/blob/c207f5b25758e4a084e8ae42e49712b91cf37114/src/core/drive/visit.js#L311
[FrameController.requestFailedWithResponse]: https://github.com/hotwired/turbo/blob/c207f5b25758e4a084e8ae42e49712b91cf37114/src/core/frames/frame_controller.js#L215

[hotwired#884]: hotwired#884
@seanpdoyle
Copy link
Contributor

...but it introduced a failing test which I'm not sure what to do with

@tropperstyle I've opened #1039, incorporated your suggestions, and resolved the failing test. Could you review that PR and share whether or not the changes resolve your issue locally?

seanpdoyle added a commit to seanpdoyle/turbo that referenced this issue Jan 30, 2024
@seanpdoyle
await for FetchRequest delegate to handle response
d2bfa35 
Closes [hotwired#884]

Despite the fact that `Visit` and `FrameController` are `FetchRequest`
delegate classes that implement their delegate methods asynchronously on
success ([Visit.requestSucceededWithResponse][] and
[FrameController.requestSucceededWithResponse][]) and on failure
([Visit.requestFailedWithResponse][]) and
[FrameController.requestFailedWithResponse][]), the `async
FetchRequest.receive` method doesn't use the `await` keyword to wait for
those delegates to finish handling their callbacks.

This commit adds those `await` clauses.

[Visit.requestSucceededWithResponse]: https://github.com/hotwired/turbo/blob/c207f5b25758e4a084e8ae42e49712b91cf37114/src/core/drive/visit.js#L297
[FrameController.requestSucceededWithResponse]: https://github.com/hotwired/turbo/blob/c207f5b25758e4a084e8ae42e49712b91cf37114/src/core/frames/frame_controller.js#L210
[Visit.requestFailedWithResponse]: https://github.com/hotwired/turbo/blob/c207f5b25758e4a084e8ae42e49712b91cf37114/src/core/drive/visit.js#L311
[FrameController.requestFailedWithResponse]: https://github.com/hotwired/turbo/blob/c207f5b25758e4a084e8ae42e49712b91cf37114/src/core/frames/frame_controller.js#L215

[hotwired#884]: hotwired#884
@seanpdoyle seanpdoyle mentioned this issue Feb 7, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

await for FetchRequest delegate to handle response seanpdoyle/turbo
2 participants
@tropperstyle @seanpdoyle