Include url in turbo:before-fetch-request event
#289
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ctrochalakis
commented
Jun 20, 2021
| @@ -71,6 +71,14 @@ export class VisitTests extends TurboDriveTestCase { | |||
| this.resetRequestInterceptor() | |||
| } | |||
|
|
|||
| async "test turbo:before-fetch-request event.detail"() { | |||
There was a problem hiding this comment.
I am getting a timeout error when I run the whole suite, but everything seems to work if I run this specific test in isolation (by adding intern.configure({ grep: /turbo:before/ }) in runner.js).
It's strange since it's basically identical to the request interceptor test.
× firefox on linux 5.10.0-0.bpo.5-amd64 - VisitTests - turbo:before-fetch-request event.detail (2.5s)
TimeoutError: Timeout reached on firefox on linux 5.10.0-0.bpo.5-amd64 - VisitTests - turbo:before-fetch-request event.detail#
at Timeout._onTimeout @ src/lib/Test.ts:226:24
at ontimeout @ timers.js:436:11
at tryOnTimeout @ timers.js:300:5
at listOnTimeout @ timers.js:263:5
at Timer.processTimers @ timers.js:223:10
There was a problem hiding this comment.
We are seeing the same on #299. Only for Firefox. Odd. Please do investigate!
|
@ctrochalakis could you please resolve merge conflicts? The |
TurboDriveTestCase maintains a eventLogChannel that acts as a cursor for the page's window.eventLogs array. Before every test the eventLogChannel is drained by reading all events and its internal index (cursor) is updated accordingly. The race was happening when we changed the page location and not blocking on it: the eventLogChannel was draining the previous page bumping its internal index. After the page was reload that index was out of sync since the new page had an new, empty eventLogs array. Order of events * async location change * draining eventLogChannel on the previous page * actual location change * eventLogChannel is out-of-sync This is was fixed as part of hotwired#289
ctrochalakis
force-pushed
the
include-url-in-turbo-before-fetch
branch
from
9e67f91
to
33170b3
Compare
|
@ctrochalakis nice catch with visits tests race condition! 👍 |
ctrochalakis
force-pushed
the
include-url-in-turbo-before-fetch
branch
from
33170b3
to
09b846a
Compare
dhh
pushed a commit
that referenced
this pull request
Jul 14, 2021
TurboDriveTestCase maintains a eventLogChannel that acts as a cursor for the page's window.eventLogs array. Before every test the eventLogChannel is drained by reading all events and its internal index (cursor) is updated accordingly. The race was happening when we changed the page location and not blocking on it: the eventLogChannel was draining the previous page bumping its internal index. After the page was reload that index was out of sync since the new page had an new, empty eventLogs array. Order of events * async location change * draining eventLogChannel on the previous page * actual location change * eventLogChannel is out-of-sync This is was fixed as part of #289
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hello all and thank you for Turbo.
Although the
turbo:before-fetch-requestevent includes the request'sfetchOptions(method, headers, etc), it doesn't include the URL itself. With this patch applied you can access it withevent.detail.url.If it gets accepted I can send a PR to add a reference to the
eventsdocumentation in https://github.com/hotwired/turbo-site/.