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

Add request interceptor #177

Merged
merged 4 commits into from
Jun 17, 2021
Merged

Add request interceptor #177

merged 4 commits into from
Jun 17, 2021

Conversation

kirillplatonov
Copy link
Contributor

The problem

The modern server-side embedded Shopify apps are built with JWT session tokens which are obtained on the client-side and then passed to every server request. Turbo works pretty well with it since it allows to navigate application without doing page reloading on every click or page submission. The biggest issue is passing JWT token to every Turbo request.

The solution

This PR introduces request interceptor support. It allows to insert async function between every fetch request. Inside this function you can do some preparation for request and add custom headers.

Example usage

Turbo.setRequestInterceptor(async (request) => {
  const token = await getSessionToken(window.app);
  request.addHeader("Authorization", `Bearer ${token}`);
});

@seanpdoyle
Copy link
Contributor

Could this same seam for modifying headers be achieved with a turbo:before-fetch-request event listener?

addEventListener("turbo:before-fetch-request", ({ detail: { fetchOptions }) => fetchOptions.headers["..."] = "..."))

@kirillplatonov
Copy link
Contributor Author

Changing headers with turbo:before-fetch-request event listener will work fine for the simpler case when you need to add some static header to Turbo requests. Unfortunately, with Shopify embedded apps you need to retrieve the JWT token before every request because the token lifetime is short and we don't know exactly when it's expired inside the Turbo app. JWT token retrieval is async and we need to pause Turbo's fetch request invocation until it's done. So async interceptor works best for this complex case. I used Axios interceptors as an inspiration for this approach.

@lonroth
Copy link

lonroth commented Feb 15, 2021

@kirillplatonov Nice work on this PR! Great that we won't need to fetch JWT every x seconds with your setRequestInterceptor solution and rather ask for them just when we need them. 👍

@seanpdoyle seanpdoyle added the enhancement New feature or request label Apr 1, 2021
@dhh dhh requested a review from sstephenson April 14, 2021 09:02
@kirillplatonov
Copy link
Contributor Author

Until it's merged I published my fork to npm:

yarn add turbo-edge
import * as Turbo from "turbo-edge"
yarn add turbo-rails-edge
import { Turbo } from "turbo-rails-edge"

@dhh
Copy link
Member

dhh commented Jun 17, 2021

Great stuff @kirillplatonov. Could you add some documentation for this on https://turbo.hotwire.dev/reference/drive? It's in the turbo-site repo. Thanks!

@@ -0,0 +1,15 @@
export class RequestInterceptor {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we not set this like we do progressBarDelay? Where it's set on the session, not as a singleton.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think here in Turbo it would be completely possible

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was trying to put it in session initially. But we use interceptor in FetchRequest class where we don't have an access to the session. Add fetching session from delegates not worked as well since not all delegates have access to the session. That's why I end up using a singleton. Seems like the cleanest solution available.

@kirillplatonov
Copy link
Contributor Author

Great stuff @kirillplatonov. Could you add some documentation for this on https://turbo.hotwire.dev/reference/drive? It's in the turbo-site repo. Thanks!

Added documentation here: hotwired/turbo-site#53

@dhh dhh merged commit 069690f into hotwired:main Jun 17, 2021
@awd
Copy link

awd commented Jun 18, 2021

Thanks for getting this through @kirillplatonov @dhh - useful for JWT / Shopify apps!

@dhh
Copy link
Member

dhh commented Jun 19, 2021

@kirillplatonov Upon further reflection, I think it might be worth exploring an approach similar to #28 (comment). Which basically let's the turbo:before-fetch-request pause the request. Then we could do it all in a callback instead.

@kirillplatonov
Copy link
Contributor Author

@dhh having the same pausable API for both turbo:before-render and turbo:before-fetch-request would be great. I will explore this.

@kirillplatonov kirillplatonov mentioned this pull request Jul 4, 2021
2 tasks
@kirillplatonov kirillplatonov mentioned this pull request Jul 12, 2021
@kirillplatonov kirillplatonov deleted the request-interceptor branch July 13, 2021 05:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Development

Successfully merging this pull request may close these issues.

7 participants