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

Refetch function throwing: Cannot read property 'token' of undefined #250

Closed
LuisCarrilloR opened this issue Jun 12, 2020 · 20 comments · Fixed by #252
Closed

Refetch function throwing: Cannot read property 'token' of undefined #250

LuisCarrilloR opened this issue Jun 12, 2020 · 20 comments · Fixed by #252

Comments

@LuisCarrilloR
Copy link

Hello,

I created a custom hook using:

export const useCmApi = makeUseAxios({
  axios: axiosInstance(apiEndpoints.default),
});

Then I tried to use it with the option manual as it is going to be triggered just when a handler function is called:

const [{ data }, fetchActivity] = useCmApi<IPaginatedData<Activity> | undefined>(
    {
      url: Endpoint.Activities,
      method: 'PUT',
    },
    { manual: true },
  );

I try to call it in the handler as show below:

const handleChange = async (page: number, limit: number, filters?: IFilter[]) => {
    await fetchActivity({ data: { limit: 10, offset: 0 } });
  };

The problem is that when I call it on my handler it throws an error: Uncaught (in promise) TypeError: Cannot read property 'token' of undefined

@simoneb
Copy link
Owner

simoneb commented Jun 12, 2020

Which version are you using? Can you try to reproduce the issue in this codesandbox?

@LuisCarrilloR
Copy link
Author

I just fixed it updating Axios to 0.19.2 (as your codesandbox example).

Thanks!

@MichielDeMey
Copy link

MichielDeMey commented Jun 13, 2020

We're currently seeing the same behaviour on the following versions:
axios-hooks: 1.10.0
axios: 0.19.2

It seems to trigger instantly when navigating away from a component that has a useAxios hook but has not finished loading yet. (Using react-router)

I'll see if I can get repo in the code sandbox.

@MichielDeMey
Copy link

I've patched it by changing this line
cancelToken: cancelSourceRef.current.token,
to
cancelToken: cancelSourceRef.current ? cancelSourceRef.current.token : undefined,

But I'm not sure this is valid thinking. 🤔

@simoneb
Copy link
Owner

simoneb commented Jun 13, 2020

@MichielDeMey can you provide a repro? If the token cancels, it must mean that the useEffect has triggered, hence there must be a token available.

@LuisCarrilloR
Copy link
Author

Yeah It is happening again.. It is the same line as @MichielDeMey mentioned.
Latest versions:

axios-hooks: 1.10.0
axios: 0.19.2

My guess is that the hook is not ready to receive the refetch function when a child triggers a prop function on mount.
Here repro

@LuisCarrilloR LuisCarrilloR reopened this Jun 15, 2020
@simoneb
Copy link
Owner

simoneb commented Jun 15, 2020

@LuisCarrilloR thanks for the repro, I'll have a look.

@simoneb
Copy link
Owner

simoneb commented Jun 15, 2020

@LuisCarrilloR @MichielDeMey I published a prerelease version axios-hooks@1.10.1-0 which contains a simple change that should address this problem. See the linked PR for what the change is.

Can you check if it solves the issue in your case?

@simoneb
Copy link
Owner

simoneb commented Jun 15, 2020

Here's an example I put together.

Note that the original repro cannot possibly work, because if you fetch data in the child component when it mounts using a useEffect hook, because of the way the parent is implemented (early returns when loading or error), it will go into an infinite loop, because:

  • child triggers fetch
  • loading flag to true in parent unmounts child
  • fetch completes
  • parent renders child
  • child triggers fetch
  • repeat from the beginning ➡️ infinite loop

@LuisCarrilloR
Copy link
Author

Thanks for the update!
I tried with the new version and it didn't work, same error (Cannot read property 'token' of undefined).

I will try to find a way to reproduce it.

@LuisCarrilloR
Copy link
Author

Here you can find another example, it breaks when using the class component lifecycle method.

@simoneb
Copy link
Owner

simoneb commented Jun 16, 2020

Thanks @LuisCarrilloR yeah that makes sense. The quick fix I tried was to use useLayoutEffect but in practice its behavior is the same as the class lifecycle method componentWillMount, meaning that it will fire in the child first and in the parent after, leading to the same issue.

I guess the solution is not as simply as changing the hook that's used to create the cancel token, so I'll come up with a different approach. Thanks for your help troubleshooting this so far.

@LuisCarrilloR
Copy link
Author

Hello @simoneb, I have found a couple possible solutions. Let me know your thought or if it breaks something else.

  1. I Created a hook that runs before the render (possibly an anti-pattern like componentWillMount) and then as a normal useEffect
const useWillMountEffect = (fn, inputs) => {
  const willMount = React.useRef(true);
  const functionRef = React.useCallback(fn, [...inputs]);
  if (willMount.current) {
    functionRef();
  }
  willMount.current = false;
  React.useEffect(() => {
    if (!willMount.current) functionRef();
  }, [...inputs, functionRef]);
};
  1. Why not to initialize the cancel when it is declared?
    const cancelSourceRef = React.useRef(StaticAxios.CancelToken.source());

@MichielDeMey
Copy link

@simoneb Meanwhile, I can confirm that the prerelease fixes my specific issue. 👍
I was calling the refetch function from a child component. Note that all components are functional components.

@simoneb
Copy link
Owner

simoneb commented Jun 19, 2020

Thanks @MichielDeMey. That won't be the final solution, I'll have to come up with a more robust approach.

Thanks @LuisCarrilloR, I started thinking about about a more generic approach, initializing the token together with the ref will be probably part of the solution.

@simoneb
Copy link
Owner

simoneb commented Jun 20, 2020

@MichielDeMey @LuisCarrilloR I published a prerelease version 1.10.1-4 which should address this issue altogether.

The rationale I followed is that any request triggered by the hook should cancel any other request, whether it's the useEffect hook used by axios-hooks or a manual request.

Can you please try it out and let me know if you find any additional issues?

@LuisCarrilloR
Copy link
Author

Nice thanks!

I am going to start moving some requests and I will let you know if something breaks.

simoneb added a commit that referenced this issue Jun 21, 2020
fixes #250

This will make it possible to pass the refetch function down to children component and let them
invoke it, without erroring on a missing token.

New behavior: cancelation errors are thrown for requests generated by the `refetch` function.
This is more consistent with how response and other errors are returned when fetching manually.
simoneb added a commit that referenced this issue Jun 21, 2020
fixes #250

This will make it possible to pass the refetch function down to children component and let them
invoke it, without erroring on a missing token.

New behavior: cancelation errors are thrown for requests generated by the `refetch` function.
This is more consistent with how response and other errors are returned when fetching manually.
@simoneb
Copy link
Owner

simoneb commented Jun 21, 2020

This is now addresses in 1.11.0. Please reopen this issue if you still encounter problems.

@dev-bjoern
Copy link

For me the cancelToken option now does not work anymore - calling source.cancel does not cancel the request anymore. I need to cancel a request manually even if no new request is started (what axios-hooks now does automatically).
Should this work?

@simoneb
Copy link
Owner

simoneb commented Jul 6, 2020

@chickenwing Which version are you using and can you provide a repro please?

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 a pull request may close this issue.

4 participants