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

Access Operation inside isTokenValidOrUndefined and handleFetch/handleError #48

Closed
justenau opened this issue Jan 27, 2022 · 13 comments · Fixed by #49
Closed

Access Operation inside isTokenValidOrUndefined and handleFetch/handleError #48

justenau opened this issue Jan 27, 2022 · 13 comments · Fixed by #49

Comments

@justenau
Copy link

justenau commented Jan 27, 2022

When using this library we ran into issue of not being able to access Redux state or dispatch actions inside the TokenRefreshLink because Operation was not accessible inside of it. We had to tweek it a bit so that these functions would pass Operation object.

I believe this use case is quite common because if we store accessToken inside Redux and after refreshing it want to change the state, there's no possible way to that without having access to context.

This requires minimal changes to the existing code though helps a lot, so it would be nice to have this functionality as part of this library.

export type HandleResponse = (operation: Operation, accessTokenField: string) => void;
export type HandleError = (operation: Operation, err: Error) => void;
export type IsTokenValidOrUndefined = (operation: Operation, ...args: any[]) => boolean;
if (!this.fetching) {
            this.fetching = true;
            this.fetchAccessToken()
                .then(this.handleResponse(operation, this.accessTokenField))
                .then(body => {
                    const token = this.extractToken(body);

                    if (!token) {
                        throw new Error('[Token Refresh Link]: Unable to retrieve new access token');
                    }
                    return token;
                })
                .then(payload => this.handleFetch(operation, payload))
                .catch(error => this.handleError(operation, error))
                .finally(() => {
                    this.fetching = false;
                    this.queue.consumeQueue();
                });
        }
@newsiberian
Copy link
Owner

hi, @justenau , could you please make a PR?

@newsiberian
Copy link
Owner

Hello again, @justenau , I've changed an order of arguments a bit to make it less breaking changer and published new version. could you please verify that it still works for you? and thanks for contibution

@justenau
Copy link
Author

Works as a charm, thank you for such a quick collaboration!
P.S. README.md should be updated accordingly to your changes as well :)

@newsiberian
Copy link
Owner

I think I've already updated README. thanks)

@kharithomas
Copy link

kharithomas commented Feb 4, 2022

Great timing, this seems to solve a similar problem I had #50

After reading the commit messages and updated README.md, I can't seem to find an explanation of what operation does or how it's intended to be used as a callback.

@justenau Is it possible to add a small example to use with Context API?

@justenau
Copy link
Author

justenau commented Feb 4, 2022

@kharithomas I looked at #50 and it seems you run into an identical situation as I had so here's an example how to solve it by using operation:

new TokenRefreshLink({
    accessTokenField: 'accessToken',
    isTokenValidOrUndefined: operation => {
        const { getState } = operation.getContext();
        const accessToken = accessTokenSelector(getState());
        if (!accessToken) {
            return true;
        }
        try {
            const { exp } = jwtDecode(accessToken);
            return Date.now() < exp * 1000;
        } catch {
            return false;
        }
    },
    fetchAccessToken: () =>
       // ... did not need to use operation here
    handleFetch: (accessToken, operation) => {
        const { dispatch } = operation.getContext();
        dispatch(setAccessToken(accessToken));
    },
     ...........
});

Basically this operation object contains context so you can reach the state from it and use dispatch to update it and so on.

I think we could update README.md to include such example as well.

@kharithomas
Copy link

@justenau Okay that makes a lot more sense - thanks!

@kharithomas
Copy link

kharithomas commented Feb 14, 2022

@justenau I'd like to ask one more clarifying question. I decided to convert my setup to use React Redux & Redux Toolkit.

Usually w/ Redux we create a store then access is directly using store.getState() inside a subscriber method to get the current state from the store.

Here, it seems getState doesn't exist on operation.getContext()

import store, { authActions, selectAccessToken } from "./store";

// ... omitted for brevity

const tokenRefreshLink = new TokenRefreshLink({
  accessTokenField: "token",
    isTokenValidOrUndefined: (operation) => {
      const { getState } = operation.getContext();
      const token = selectAccessToken(getState());    
  
      if (!token) {
        return true;
      }
     
      // ... remaining link setup

ss

Is there an additional step needed? It's not explicitly mentioned, but how do we specify our Redux store in the apollo-link-token-refresh setup?

@justenau
Copy link
Author

@kharithomas Very common practice in Redux is to wrap the App with Provider which makes it possible to access the store in all other nested components because it puts the Redux store and the current store state into context. You can look into this for reference https://react-redux.js.org/using-react-redux/accessing-store

As far as I can see from your example you already have your store accessible if you can import it that easily from './store'. In that case I guess you might even get away without this additional complexity of extracting state from operation.context as you can simply call store.getState() as you've said and extract necessary values from there. 🤔

@kharithomas
Copy link

Thanks for the quick reply. And yes, that's how I've set mine up.

ReactDOM.render(
  <ApolloProvider client={client}>
    <Provider store={store}>
      <Router>
        <App />
      </Router>
    </Provider>
  </ApolloProvider>,
  document.getElementById("root")
);

For this reason, I am also wondering if the additional complexity provides any additional benefits that just directly accessing the store.

@justenau
Copy link
Author

Hmm, then it's weird that you cannot access getState as it should be part of the context 🤔 Though it's hard to pinpoint the issue now since I don't know the inner-workings of your project. If calling store.getState() does the job for you then there's definitely no need to add this complexity, I added these changes only because in the project I'm working on there was no way to access store - it lived outside the project and we didn't have access to it anywhere besides the Context, so using operation in these functions solved the issue.

@kharithomas
Copy link

kharithomas commented Feb 14, 2022

Update* my configuration of Redux was incorrect - accessing the store directly instead of through operation does work as intended. Tonight, I will still put together a code sandbox to demo what I have.

Original message:

Right, but I can confirm that this setup isn't working.

Seems the dispatch isn't getting called correctly from here.

I might be able to put together an example on code sandbox to demo

@nazmeln
Copy link

nazmeln commented Feb 8, 2024

Hi @justenau @kharithomas ,
I also have an issue with that operation.getContext() doesn't return redux store and I cannot access the getState method. Maybe, the logic above is applicable only for custom Redux Context?

My Redux Provider config is same as here.

cc: @newsiberian

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