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.

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

fix: allow programmatic visits to go cross-origin #410

Merged
merged 5 commits into from
Sep 28, 2021

Conversation

KonnorRogers
Copy link
Contributor

@KonnorRogers KonnorRogers commented Sep 28, 2021

Currently, when calling Turbo.visit("https://example.com", { action: "advance" }) , the site will not navigate to the https://example.com and in addition a number of errors pop up here:

Screen Shot 2021-09-27 at 6 04 09 PM

Most of these errors have to do with trying to fetch cross-origin as well as adding history for a cross-origin url. This PR takes previous art from Turbolinks here:

https://github.com/turbolinks/turbolinks/blob/80cd172ed5b3a35eda85c58e428de9f1f1d06fc8/src/controller.ts#L70-L80

and uses previous art from Turbo's own link delegator here:

turbo/src/core/session.ts

Lines 334 to 336 in 5982a67

locationIsVisitable(location: URL) {
return isPrefixedBy(location, this.snapshot.rootLocation) && isHTML(location)
}

Unfortunately, I ran into multiple issues trying to get tests for this due to my chromedriver only supporting up to v92 and chrome itself being v94. I tried manually installing and upgrading both and was unsuccessful. Regardless, I think drawing on prior art and having done some manual testing as well, all appears to be well.

Video Before:

Screen.Recording.2021-09-27.at.11.06.50.PM.mov

Video After:

Screen.Recording.2021-09-27.at.11.04.12.PM.mov

@KonnorRogers KonnorRogers changed the title fix: allow visit to go cross-origin fix: allow programmatic visits to go cross-origin Sep 28, 2021
@Matt-Yorkley
Copy link
Contributor

You're a star! 🌟

Just for clarity; this should close issue #401 right? Looks like there's a few folks that would be excited to see it fixed (myself included)

@KonnorRogers
Copy link
Contributor Author

KonnorRogers commented Sep 28, 2021

@Matt-Yorkley I believe that issue has to do with when you submit a form and then redirect to an external URL. This will not fix that. This is purely for programmatic visits. IE: Turbo.visit

@dhh dhh merged commit acb003d into hotwired:main Sep 28, 2021
@Matt-Yorkley
Copy link
Contributor

@Matt-Yorkley I believe that issue has to do with when you submit a form and then redirect to an external URL. This will not fix that. This is purely for programmatic visits. IE: Turbo.visit

Ah, okay! I assumed if a Stream responded with a redirect to an external URL it would hit the same code path when it came to navigating..? I'll take your word for it though! 👌😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants