-
-
Notifications
You must be signed in to change notification settings - Fork 453
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
Feature request: allow to return promise in fetchOptions function #234
Comments
So, it makes complete sense to delay the queries a little depending on your environment, but fetchOptions is mostly a convenience helper that's leftover from previous versions. It might make more sense for you to write an exchange to manipulate the queries' context. We haven't provided a helper for this yet, but in the meantime let me know if you'd like any code snippet for achieving this? |
yes please. AlthoughI have to ask-do you really believe that forcing users to write their own exchange even for such a basic usecase is desirable? Users migrating from apollo-client will have hard time grokking the whole wonka API. |
@capaj No, we wouldn't force anyone to write exchanges at any time. On the other hand it stands to reason that My theory is that once we provide a helper for errors and changing the context, then it'll become unnecessary for most though to write their own exchanges. This would then be similar to Apollo's |
@capaj Depending on how your Auth/routes are setup, there's a good chance you could:
I'm not a huge fan of having async fetchOptions but I can see value in being able to update the client's fetch options. This shouldn't be too hard to implement in the client and also the provider. |
Looks like this could be a valid use case. I'll have a play around with it this weekend. |
@capaj I've created a PR (#264) but while trying it out, it got me wondering whether a setter/async fetchOptions are required for your use case. We already support functional implementations of fetchOptions which are called at every fetch call. You could provide a function which gets the access token from localStorage (see below example). // Client declaration
const client = createClient({
//..
fetchOptions: () => { headers: { 'Some-Auth-Header': window.localStorage.getItem('some-auth-token') } }
}); Looking at the docs for your Auth provider, you shouldn't need to manually request a session key / refresh (source).
Let me know if the above example works for you. |
@andyrichardson The docs are correct that there is no manual refresh, but its the auto-refresh functionality supplied by Amplify which forces async access patterns to the current JWT. This is the normal access pattern for Amplify: const token = (await Auth.currentSession())
.getAccessToken()
.getJwtToken(); the async call to currentSession() will refresh tokens if necessary on access. I'm not aware of a synchronous access pattern to Amplify's auth store without reading their namespace from localStorage, which is not a good pattern. In fact it works hard to prevent access to stale credentials. The most straightforward solution I see is to modify fetch.ts. Call Promise.resolve(extraOptions) after extraOptions is defined, and have the rest of the method inside that promise's then function. |
We were discussing a lot how best to make this work and we were actually thinking of building in an escape hatch. But another solution that we're currently favouring might be to add an exchange for this similar to this: https://www.apollographql.com/docs/link/links/context/ We might still expose a second escape hatch but the reason for this is that it might be pretty dangerous to attach more stateless functions to urql that aren't exchanges, especially if they're asynchronous. It does increase the surface area of things that might go wrong. That's actually why this has taken a while but we'll likely ship a solution for this in the upcoming v1.1, hopefully this week. I'm not quite sure whether this exchange pattern would work for you but it's essentially more powerful than extending the reach of fetchOptions 👍 |
Since this is a very small exchange we'll likely show just a guide and example of how to set this up as part of #247. If anyone else then publishes a "generic exchange" for this that'd be awesome! |
+1 on this.
Even though |
@papigers there are also alternative solutions, like adding tokens to a component that passes them on to a client that is kept in state. We’re currently exploring how we can structure the project to ship more “utility exchanges”. Part of why we haven’t done that before is to make urql‘s bundlesize appear as small as it’ll realistically be. We’re now considering to ship more exchanges and communicate that treeshaking will enable the same size as before. One concern has been that we do believe in browser ESM imports which would see larger files being downloaded as part of that. Either way, we’ve got the following exchanges planned:
That being said, all this code exists alteady and you’re welcome to — for now — copy it yourself, open issues with questions and/or for help (or ask on Spectrum), and use it without learning any of Wonka.
Generally I totally agree and we need to alleviate this problem. On the other hand it’s worth mentioning that Exchanges will remain our main point of extensibility and implementation of features, so we encourage people to maintain third-party exchange packages while we work this out 🙌 Hope this helps! |
@kitten thanks for the quick reply. I understand your desire to keep exchanges as the main point of extensibility, but I still think you need to differentiate between minor change/extension such as this which requirer minor tweaks in the core API, and bigger extensions that would be solved using exchanges, but we can agree to disagree :) That being said, great package - well done :) |
I'm using this custom exchange and working fine: import {
Provider,
createClient,
Exchange,
Operation,
dedupExchange,
cacheExchange,
fetchExchange,
debugExchange
} from "urql";
import { pipe, mergeMap, fromPromise, fromValue, map } from "wonka";
const isPromise = (value: any) => value && typeof value.then === "function";
export const fetchOptionsExchange = (fn: any): Exchange => ({
forward
}) => ops$ => {
return pipe(
ops$,
mergeMap((operation: Operation) => {
const result = fn(operation.context.fetchOptions);
return pipe(
isPromise(result) ? fromPromise(result) : fromValue(result),
map((fetchOptions: RequestInit | (() => RequestInit)) => ({
...operation,
context: { ...operation.context, fetchOptions }
}))
);
}),
forward
);
};
const client = createClient({
url: "https://countries.trevorblades.com/",
exchanges: [
dedupExchange,
debugExchange,
cacheExchange,
// Must be called before fetchExchange and after all others sync exchanges, respecting the rule Synchronous first, Asynchronous last
fetchOptionsExchange(async (fetchOptions: any) => {
return Promise.resolve({
...fetchOptions,
headers: {
Authorization: "Bearer mySuperToken"
}
});
}),
fetchExchange
]
}); @kitten maybe somthing like this could be part of the core library or like the retryExchange. |
Hi, I've created a tiny library that add headers asynchronously. Hope it helps someone. |
Thanks @acro5piano for writing the helper. |
Is it supported now? Switching to apollo-client because of this. |
Starting (maybe) URQL v4, async header can be implemented using import { createClient, mapExchange, Operation, makeOperation } from 'urql'
const client = createClient({
url: "https://countries.trevorblades.com/",
exchanges: [
cacheExchange,
// Must be called before fetchExchange and after all others sync exchanges, respecting the rule Synchronous first, Asynchronous last
mapExchange({
async onOperation(operation) {
return makeOperation(operation.kind, operation, {
...operation.context,
fetchOptions: {
...operation.context.fetchOptions,
headers: {
Authorization: await Promise.resolve("Bearer mySuperToken")
},
},
})
},
}),
fetchExchange
]
}); |
there are often cases when in order to obtain a JWT token, we have to call an async function. For example with AWS amplify: https://aws-amplify.github.io/docs/js/authentication#retrieve-current-session
it would be nice if urql would allow us to return fetch options from the
fetch
asynchronously.currently fetch options type is defined here: https://github.com/FormidableLabs/urql/blob/9a49818c4a54583d1f457772323dab58c78211df/src/client.ts#L32
and as you can see it only accepts a static object or a function which returns synchronously.
The text was updated successfully, but these errors were encountered: