Skip to content

Commit

Permalink
Propose a replace visit on redirection (#328)
Browse files Browse the repository at this point in the history
* Propose a `replace` visit instead of directly modifying the history and location

* Updated the browser adapter to remove the call to `followRedirect` as it now happens in the visit itself
* Updated the tests to reflect the new behaviour

* Added navigation tests for redirection

Co-authored-by: David Heinemeier Hansson <david@loudthinking.com>
  • Loading branch information
Prabhakar Bhat and dhh committed Aug 24, 2021
1 parent bc5b7cd commit 38b8f13
Show file tree
Hide file tree
Showing 5 changed files with 23 additions and 4 deletions.
7 changes: 5 additions & 2 deletions src/core/drive/visit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,7 @@ export class Visit implements FetchRequestDelegate {
this.state = VisitState.completed
this.adapter.visitCompleted(this)
this.delegate.visitCompleted(this)
this.followRedirect()
}
}

Expand Down Expand Up @@ -259,8 +260,10 @@ export class Visit implements FetchRequestDelegate {

followRedirect() {
if (this.redirectedToLocation && !this.followedRedirect) {
this.location = this.redirectedToLocation
this.history.replace(this.redirectedToLocation, this.restorationIdentifier)
this.adapter.visitProposedToLocation(this.redirectedToLocation, {
action: 'replace',
response: this.response
})
this.followedRedirect = true
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/core/native/browser_adapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ export class BrowserAdapter implements Adapter {
}

visitCompleted(visit: Visit) {
visit.followRedirect()

}

pageInvalidated() {
Expand Down
1 change: 1 addition & 0 deletions src/tests/fixtures/navigation.html
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ <h1>Navigation</h1>
<svg width="600" height="100" viewbox="-300 -50 600 100"><text><a id="cross-origin-link-inside-svg-element" href="about:blank">Cross-origin link inside SVG element</a></text></svg>
<p><a id="link-to-disabled-frame" href="/src/tests/fixtures/frames/hello.html" data-turbo-frame="hello">Disabled turbo-frame</a></p>
<p><a id="autofocus-link" href="/src/tests/fixtures/autofocus.html">autofocus.html link</a></p>
<p><a id="redirection-link" href="/__turbo/redirect?path=/src/tests/fixtures/one.html">Redirection link</a></p>
<p><a id="headers-link" href="/__turbo/headers">Headers link</a></p>
</section>

Expand Down
2 changes: 1 addition & 1 deletion src/tests/functional/form_submission_tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ export class FormSubmissionTests extends TurboDriveTestCase {

this.assert.notOk(await this.formSubmitted)
this.assert.equal(await this.pathname, "/src/tests/fixtures/one.html")
this.assert.equal(await this.visitAction, "advance")
this.assert.equal(await this.visitAction, "replace")
this.assert.equal(await this.getSearchParam("greeting"), "Hello from a form")
}

Expand Down
15 changes: 15 additions & 0 deletions src/tests/functional/navigation_tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,21 @@ export class NavigationTests extends TurboDriveTestCase {
this.assert.ok(await this.isScrolledToSelector("#main"), "scrolled to #main")
}

async "test following a redirection"() {
await this.clickSelector('#redirection-link')
await this.nextBody
this.assert.equal(await this.pathname, "/src/tests/fixtures/one.html")
this.assert.equal(await this.visitAction, "replace")
}

async "test clicking the back button after redirection"() {
await this.clickSelector('#redirection-link')
await this.nextBody
await this.goBack()
this.assert.equal(await this.pathname, "/src/tests/fixtures/navigation.html")
this.assert.equal(await this.visitAction, "restore")
}

async "test same-page anchor visits do not trigger visit events"() {
const events = [
"turbo:before-visit",
Expand Down

0 comments on commit 38b8f13

Please sign in to comment.