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

Ensure fullpage_redirect_to works with Turbo #27

Closed

Conversation

yourivdlans
Copy link

@yourivdlans yourivdlans commented Feb 6, 2022

Hi!

After upgrading my app to use Turbo instead of Turbolinks the redirect to a charge (in case a user wants to upgrade their subscription / plan) stopped working.

I was doing this using some inline JS which would be rendered instead of an actual redirect which would call some AppBridge code to perform the redirect. See: Shopify/shopify_app#1287

Because the method fullpage_redirect_to relies on Turbolinks I was also not able to use this.

Therefore I came up with the following solution: Render a div which initialises a stimulus redirect controller which then uses the window.app (that also contains the host param) to dispatch a redirect request.

The code is based on:

@yourivdlans yourivdlans force-pushed the fix-fullpage-redirect-to branch 2 times, most recently from ea2d884 to 2715701 Compare February 6, 2022 17:03
@kirillplatonov
Copy link
Owner

Hey.

I've been trying to reproduce the issue but fullpage_redirect_to seems to work fine with Turbo. I tested the regular link, frame, and form submission. These are examples that I used: 9c4e7b0

Which use case do you have?

@yourivdlans
Copy link
Author

Thanks for trying to reproduce the problem! This led me to find the actual problem. The partial shopify_app/shared/redirect.html.erb was rendered by fullpage_redirect_to but the JS included in that view was never executed.

Aparently this was caused due to data-turbo-track being set to reload, after removing this attribute everything started working.

- <%= javascript_include_tag "application", "data-turbo-track": "reload", defer: true %>
+ <%= javascript_include_tag "application" %>

One thing I did notice is that fullpage_redirect_to seems to still be using Shopify AppBridge 1.x.

See: https://github.com/Shopify/shopify_app/blob/3a75d14cfa2cc2e2cdb37e6479b61d9cc7145206/app/assets/javascripts/shopify_app/app_bridge_redirect.js#L1

@yourivdlans yourivdlans deleted the fix-fullpage-redirect-to branch February 17, 2022 13:23
@yourivdlans
Copy link
Author

Oh one other thing that I found, if you would try to use fullpage_redirect_to from a method that was called using POST Turbo returns the following error: Error: Form responses must redirect to another location.

This can be fixed by manually setting response.status = :see_other right before calling fullpage_redirect_to.

@kirillplatonov
Copy link
Owner

Yeah, data-turbo-track is actually breaking a lot of things in embedded apps.

As for fullpage_redirect_to I suppose it makes sense to open a PR in shopify_app with an upgrade to AppBridge 2.0 and maybe set :see_other status for turbo frame POST responses automatically.

@yourivdlans
Copy link
Author

As for fullpage_redirect_to I suppose it makes sense to open a PR in shopify_app with an upgrade to AppBridge 2.0 and maybe set :see_other status for turbo frame POST responses automatically.

Yeah good idea, I'll try and see if I can open a PR in shopify_app.

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

Successfully merging this pull request may close these issues.

2 participants