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

useAsyncFn - deps parameter should be required #1671

Open
joakimgunst opened this issue Nov 30, 2020 · 2 comments
Open

useAsyncFn - deps parameter should be required #1671

joakimgunst opened this issue Nov 30, 2020 · 2 comments

Comments

@joakimgunst
Copy link

The second parameter in the useAsyncFn is a dependency array and it is currently optional with a default value of [], which is passed on to useCallback internally. An empty array means that the callback function is memoized and never updated.

This is unexpected because it doesn't match how native React hooks work, e.g. in useMemo an undefined dependency array causes the value to be recomputed on every render, and in useEffect an undefined array causes the effect to fire on every render. Both of these are different from an empty array.

I think it would be safer to make the second parameter required, which would match how useCallback works. This is a breaking change so maybe not so easy to do, but hopefully it could still be fixed at some point.

@fnune
Copy link
Contributor

fnune commented Mar 24, 2021

Current workaround:

import React from "react";
import "react-use";
import { AsyncFnReturn, AsyncState } from "react-use/lib/useAsyncFn";
import { FnReturningPromise, PromiseType } from "react-use/lib/util";

declare module "react-use" {
    export type StateFromFnReturningPromise<
        T extends FnReturningPromise
    > = AsyncState<PromiseType<ReturnType<T>>>;

    /**
     * Please add a dependencies array. The weird `never` return type is there
     * to ban usage of this hook without a dependencies array.
     */
    export function useAsyncFn(
        fn: unknown,
        deps?: undefined,
        initialState?: unknown
    ): never;
    export function useAsyncFn<T extends FnReturningPromise>(
        fn: T,
        deps: React.DependencyList,
        initialState?: StateFromFnReturningPromise<T>
    ): AsyncFnReturn<T>;

    /**
     * Please add a dependencies array. The weird `never` return type is there
     * to ban usage of this hook without a dependencies array.
     */
    export function useAsync(fn: unknown, deps?: undefined): never;
    export function useAsync<T extends FnReturningPromise>(
        fn: T,
        deps: React.DependencyList
    ): AsyncState<PromiseType<ReturnType<T>>>;
}

Augmenting a module declaration with a function adds an overload, so it's necessary to ban the signatures you don't want.

@JoeDuncko
Copy link

Hi! @react-hookz/web, the new library by one of react-use's former maintainers (background here and here) has a new implementation of useAsyncFn called useAsync that solves this issue.

For those interested, there's an official migration guide for migrating from react-use to @react-hookz/web.

Hope this helps!

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

No branches or pull requests

3 participants