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

useCallback/useEffect support custom comparator #14476

Closed
kpaxqin opened this issue Dec 20, 2018 · 30 comments
Closed

useCallback/useEffect support custom comparator #14476

kpaxqin opened this issue Dec 20, 2018 · 30 comments

Comments

@kpaxqin
Copy link
Contributor

@kpaxqin kpaxqin commented Dec 20, 2018

Currently we can pass an array as second argument when using useCallback or useEffect like below:

useCallback(()=> {
  doSth(a, b)
}, [a, b]) // how to do deep equal if a is an object ?

The problem is it only compare array items with ===, it there any way to compare complex object ?

Support custom comparator as third argument looks not bad:

useCallback(()=> {
  doSth(a, b)
  }, 
  [complexObject], 
  (item, previousItem)=> { //custom compare logic, return true || false here }
)
@arianon
Copy link

@arianon arianon commented Dec 20, 2018

You could do this instead.

useCallback(() => {
  doSth(a, b)
}, [a, a.prop, a.anotherProp, a.prop.nestedProp /* and so on */, b])

Or you could try using JSON.stringify(a) in the inputs array.

Loading

@aweary
Copy link
Contributor

@aweary aweary commented Dec 20, 2018

You can use the result of the custom equality check as a value inside the array to get this behavior.

useEffect(
  () => {
    // ...
  },
  [compare(a, b)]
);

Loading

@heyimalex
Copy link

@heyimalex heyimalex commented Dec 20, 2018

@aweary How would that work? They want to use the custom comparator to check compare(a, prevA), not compare(a, b). The previous value isn't accessible in scope.

Loading

@aweary
Copy link
Contributor

@aweary aweary commented Dec 20, 2018

@heyimalex you can use a ref to store a reference to the previous value. This is mentioned in the Hooks FAQ: How to get the previous props or state?
.

Loading

@heyimalex
Copy link

@heyimalex heyimalex commented Dec 20, 2018

@aweary How would passing [compare(a, prevA)] work? Wouldn't that make it update on transitions from true -> false and from false -> true when you really just want it to update when false? Sorry if I'm missing something obvious!

I was trying to write what the op wanted and here's what I got.

function usePrevious(value) {
    const ref = useRef();
    useEffect(() => {
        ref.current = value;
    });
    return ref.current;
}

function useCallbackCustomEquality(cb, args, equalityFunc) {
    const prevArgs = usePrevious(args);
    const argsAreEqual =
        prevArgs !== undefined &&
        args.length === prevArgs.length &&
        args.every((v, i) => equalityFunc(v, prevArgs[i]));
    const callbackRef = useRef();
    useEffect(() => {
        if (!argsAreEqual) {
            callbackRef.current = cb;
        }
    })
    return argsAreEqual ? callbackRef.current : cb;
}

Loading

@aweary
Copy link
Contributor

@aweary aweary commented Dec 20, 2018

@kpaxqin yeah it's sort of a half-baked solution 😕your approach is fine I think for useCallback. For useEffect you'd do something similar except you'd add a check for equality inside a useEffect before calling the provided effect callback.

Loading

@heyimalex
Copy link

@heyimalex heyimalex commented Dec 20, 2018

You could actually encapsulate the arg memoization in one hook and then use that for all of the other functions.

function useMemoizeArgs(args, equalityCheck) {
  const ref = useRef();
  const prevArgs = ref.current;
  const argsAreEqual =
        prevArgs !== undefined &&
        args.length === prevArgs.length &&
        args.every((v, i) => equalityFunc(v, prevArgs[i]));
  useEffect(() => {
      if (!argsAreEqual) {
        ref.current = args;
      }
  });
  return argsAreEqual ? prevArgs : args;
}

useEffect(() => {
  // ...
}, useMemoizeArgs([a,b], equalityCheck));

Loading

@eyston
Copy link

@eyston eyston commented Dec 23, 2018

A similar API to React.memo would be great for useEffect and friends when used with an immutable data structure library -- such as clojurescript. Not to over simplify it but it at least appears to be a very accessible change:

export function useEffect(
create: () => mixed,
inputs: Array<mixed> | void | null,
): void {
useEffectImpl(
UpdateEffect | PassiveEffect,
UnmountPassive | MountPassive,
create,
inputs,
);
}
function useEffectImpl(fiberEffectTag, hookEffectTag, create, inputs): void {
currentlyRenderingFiber = resolveCurrentlyRenderingFiber();
workInProgressHook = createWorkInProgressHook();
let nextInputs = inputs !== undefined && inputs !== null ? inputs : [create];
let destroy = null;
if (currentHook !== null) {
const prevEffect = currentHook.memoizedState;
destroy = prevEffect.destroy;
if (areHookInputsEqual(nextInputs, prevEffect.inputs)) {
pushEffect(NoHookEffect, create, destroy, nextInputs);
return;
}
}

Loading

@kpaxqin
Copy link
Contributor Author

@kpaxqin kpaxqin commented Dec 25, 2018

@heyimalex Thanks for your solution, it works great for me, but still got one risk as a common solution.

For example: re-render every second, use current time as input array, compare time and run effect if minute has changed.

The key is: A equal to B and B equal to C might not means A equal to C for custom comparator

Use another value as compare result could get rid of this

function useMemoizeArgs(args, equalityCheck) {
  const ref = useRef();
  const flag = ref.current ? ref.current.flag : 0;
  const prevArgs = ref.current ? ref.current.args : undefined;
  const argsAreEqual =
        prevArgs !== undefined &&
        args.length === prevArgs.length &&
        args.every((v, i) => equalityFunc(v, prevArgs[i]));
  useEffect(() => {
      ref.current = {
         args,
         flag: !argsAreEqual ? (flag + 1) : flag
      }
  });
  return ref.current.flag
}

useEffect(() => {
  // ...
}, useMemoizeArgs([a,b], equalityCheck));

Still think the best way would be support custom comparator officially for those hooks which need to compare inputs.

Loading

@slestang
Copy link

@slestang slestang commented Feb 27, 2019

The solution given by @kpaxqin didn't work during my tests because:

  1. ref.current is undefined on first call so return ref.current.flag crash
  2. the way argsAreEqual is compute ends up returning true for the first call, therefore the flag is always incremented at least once

I modify it to:

const useMemoizeArgs = (args, equalityFunc) => {
  const ref = useRef();
  const flag = ref.current ? ref.current.flag : 0;
  const prevArgs = ref.current ? ref.current.args : undefined;
  const argsHaveChanged =
    prevArgs !== undefined &&
    (args.length !== prevArgs.length || args.some((v, i) => equalityFunc(v, prevArgs[i])));

  useEffect(() => {
    ref.current = {
      args,
      flag: argsHaveChanged ? flag + 1 : flag,
    };
  });

  return flag;
};

which works for my use case.

But obviously if hooks supported a custom comparator like React.memo as parameter, things will be a lot better/simpler.

Loading

@kalekseev
Copy link

@kalekseev kalekseev commented Mar 3, 2019

My main use cases where deep equal required is data fetching

const useFetch = (config) => {
  const [data, setData] = useState(null); 
  useEffect(() => {
    axios(config).then(response => setState(response.data)
  }, [config]);  // <-- will fetch on each render
  return [data];
}

// in render
const DocumentList = ({ archived }) => {
  const data = useFetch({url: '/documents/', method: "GET", params: { archived }, timeout: 1000 }) 
  ...
}

I checked top google results for "react use fetch hook", they are either have a bug (refetch on each rerender) or use one of the methods:

So it would be good to have a comparator as a third argument or a section in docs that explains the right way for such use cases.

Loading

@gaearon
Copy link
Member

@gaearon gaearon commented Mar 9, 2019

Right now we have no plans to do this. Custom comparisons can get very costly when spread across components — especially when people attempt to put deep equality checks there. Usually there's a different way to structure the code so that you don't need it.

There are a few common solutions:

Option 1: Hoist it up

If some value is supposed to be always static, there's no need for it to be in the render function at all.
For example:

const useFetch = createFetch({ /* static config */});

function MyComponent() {
  const foo = useFetch();
}

That completely solves the problem. If instantiating it is annoying, you can always have your own useFetch module that just re-exports the instantiated result (or even provides a few variations):

// ./myUseFetch.js
import createFetch from 'some-library';

export const useFetch = createFetch({ /* config 1 */ })
export const useFetchWithAuth = createFetch({ /* config 2 */ })

Option 2: Embrace dynamic values

Another option is the opposite. Embrace that values are always dynamic, and ask the API consumer to memoize them.

function MyComponent() {
  const config = useMemo(() => ({
    foo: 42,
  }, []); // Memoized
  const data = useFetch(url, config);
}

This might seem like it's convoluted. However for low-level Hooks you probably will create higher-level wrappers anyway. So those can be responsible for correct memoization.

// ./myUseFetch.js
import {useContext} from 'react';
import useFetch from 'some-library';
import AuthContext from './auth-context';

export function useFetchWithAuth(url) {
  const authToken = useContext(AuthContext);
  const config = useMemo(() => ({
    foo: 42,
    auth: authToken
  }), [authToken]); // Only changes the config if authToken changes
  return useFetch(url, config);
}

// ./MyComponent.js
function MyComponent() {
  const data = useFethWithAuth('someurl.com'); // No config
}

Then effectively the users don't need to configure anything at the call site — or you can expose limited supported options as specific arguments. That's probably a better API than an arbitrarily deep "options" object anyway.

Option 3: (be careful) JSON.stringify

Many people expressed a desire for deep equality checks. However, deep equality checks are very bad because they have unpredictable performance. They can be fast one day, and then you change the data structure slightly, and they become super slow because the traversal complexity has changed. So we want to explicitly discourage using deep checks — and useEffect API currently does that.

However, there are cases where it stands in the way of ergonomics too much. Such as with code like

const variables = {userId: id};
const data = useQuery(variables);

In this case there is no actual deep comparisons necessary. In this example, we know our data structure is relatively shallow, doesn't have cycles, and is easily serializable (e.g. because we plan to send that data structure over the network as a request). It doesn't have functions or weird objects like Dates.

In those rare cases it's acceptable to pass [JSON.stringify(variables)] as a dependency.

“Wait!” I hear you saying, “Isn’t JSON.stringify() slow?”

On small inputs, like the example above, it is very fast.

It does get slow on larger inputs. But at that point deep equality is also slow enough that you’ll want to rethink your strategy and API. Such as going back to a granular invalidation with useMemo. That gives you the performance — if indeed, you care about it.


There are probably minor other use cases that don't quite fit into these. For those use cases it is acceptable to use refs as an escape hatch. But you should strongly consider to avoid comparing the values manually, and rethinking your API if you feel you need that.

Loading

@gaearon gaearon closed this Mar 9, 2019
peternycander added a commit to peternycander/reactjs.org that referenced this issue Mar 17, 2019
Dan listed useMemo as a solution for avoiding re-triggering an effect
facebook/react#14476 (comment)
If this is an accepted use case, the docs should reflect it.
@orestis
Copy link

@orestis orestis commented Mar 19, 2019

Just commenting that this is an issue with ClojureScript that provides its own equality semantics, where equality comparison of data structures are based on value and not of reference. So using the Object.is algorithm without an escape hatch is an issue. Similarly for setState.

Loading

@jozanza
Copy link

@jozanza jozanza commented May 21, 2019

@gaearon Option 3 is simple and works well for my case but clashes with react-hooks/exhaustive-deps since the non-serialized object is used in the effect. Is it just not possible with this linting option enabled?

Loading

@mvitkin
Copy link

@mvitkin mvitkin commented Feb 13, 2020

@Carduelis Thank you for asking this, I've been scratching my head for a while trying to figure out what the best approach to this problem is. The useMemo solution that @gaearon presented is almost perfect for us, except for that tidbit in the docs you quoted about semantic guarantees makes me nervous.

I've seen answers saying to use refs if you need semantic guarantees, but I can't wrap my head around how to apply those to this use case.

My example:

function useFetch({ url, requestBody }){
     const [result, setResult] = useState({ loading: false, data: null, error: null })
     const queryFunc = useCallback(() => {
            // ... do fetch
     }, [url, requestBody]);
     return [result, queryFunc];
}

function MyComponent(props) {
    const body = useMemo(() => ({ id: props.id }), [props.id])
    const [result, query] = useFetch("example", body)

    useEffect(() => {
         query();
    } , [query])
}

As you can see, we use useMemo to preserve the reference as well as kick off a new query whenever props.id changes. If useMemo randomly decides the recalculate its value, this would cause an unexpected fetch call. How could we use refs here?

Loading

@denis-sokolov
Copy link
Contributor

@denis-sokolov denis-sokolov commented Feb 14, 2020

Consider the use-memo-one package with provides useMemo and useCallback with the added semantic guarantee.

Loading

Padraic-O-Mhuiris added a commit to OasisDEX/mcd-cdp-portal that referenced this issue Feb 28, 2020
* observables-examples

* Updates

* Connect front-end to multicall observables via useObservable

* Stringify object type dependencies in useObservable

See: facebook/react#14476 (comment)

* Register multicall schemas on maker initialization

- Register multicall schemas on maker initialization
- Add experimental .watch convenience helper and map it to useObservable hook

* Use array of ilks for ilkPrices observable

* Use experimental .watch convenience helper

* Add type definitions for watch alias

Add typescript definitions for watch alias that extends WatchInterfaceMcd (which should be defined in dai-plugin-mcd)

* Add jsconfig.json for better editor type definition support

See: https://code.visualstudio.com/docs/languages/jsconfig#_why-do-i-need-a-jsconfigjson-file

* Return single value from useObservable hook

* Update watch subscriptions to handle single return value

* Add type definition reference to useObservable.js

* rudimentary implementation of watch.vault in Generate componenent

* Watch vault in CDPDisplay

* use observables for vault data in presentation component

* remove cdp dep from presentation

* update testchain & adjust test values for latest snapshot

* SDK-740 pass vault object to Generate component & fix tests

* Use vault computed observable helper functions

* implement vault observable with Withdraw component

* Refactor vault observable logic in CDPDisplay

* SDK-740 pass vault object to Payback component & fix tests

* SDK-740 pass vault object to Deposit component & fix tests

* SDK-740 fix CDPDisplay test

* SDK-740 fix Presentation test

* Catch errors in useObservable and set value to null

* Use only vault observable in CDPDisplay

* update price feed string in Deposit

* make vault observables formatter more flexible

* use vault.id instead of passing cdpId to sidebar components

* tweak formatter

* add symbol to price feed in sidebars

* use built in calc functions in Withdraw sidebar

* cleanup

* fix RatioDisplay colors

* add RatioDisplay component back to Generate

* fix Generate input validation

* use multicall.watch method

* fix negative value error

* Fix useObservable linter warnings

* remove references to liquidationRatioSimple

* fix setMax in Payback

* integrated multicall changes for savings ticker

* refix setMax in payback

* Simplify DsrInfo

* Fixed useEffect warnigs

* Use latest version of dai and dai-plugin-mcd packages

* skip save test while debugging

* Add missing useSavings hook dependencies

* Simplify useSavings hook

* Add feature flag for dynamic decimals for dsr ticker

* Move imports above FF

* use useVault in the main Save page and pass data to child components

* fetch balances from multicall in useWalletBalances hook

* add mocked observables to fix tests

* use observables for save sidebar details

* fixed DSR earnings ticker miscalculation

* Fixed accounting for earnings interest between starts

* Remove unnecessary useEffect in DSRInfo

* Move allowance test in save spec to top

* Use latest dai-plugin-mcd package

* Use watch observable hooks in useTokenAllowance

* update useTokenAllowance hook & fix tests

* Using useVault to populate CDPList and Overview pages

* Bump dai and dai-plugin-mcd package versions

* Set useObservable value to undefined if deps change

* Set useObservable value to undefined if deps change

* update tests to wait for proxy and allowance data

* moved useVault logic to provider instead of hook

* Fix some breaking tests for Vault provider change

* fix CDPCreate test & begin adding observables to onboarding

* update plugin to dai-plugin-mcd-v1.1.3

* Add DSR to useWalletBalances & conform to rules of hooks

* Use observables for account box wallet balances

* Add total vaults created to system sidebar

* Remove unused tokensWithBalances helper

* Update test to use showWalletTokens config

* Create and start multicall watcher in MakerProvider

* watching systemCollateralization observable

* fixes some tests and begin work on onboarding observables

* mock systemCollateralization for Save test

* move onboarding observables to parent file and remove feeds

* remove unused code related to vault data

* Add id to mock vault to prevent errors

- Add id to mock vault to prevent errors
- Revert watch.proxyAddress to using single address arg to prevent weird RxJS warnings

* check user first vault with observables

* fix some breaking changes

* use new computed observable with calc functions for collateral types

* add remaining collateral type data to onboarding

* updated useProxy to use observables

* Use find not filter on collateralTypesData

* Fix validation errors in onboarding

* Fix console.log

* removed accounts reducer

* removed cdps reducer

* remove all reducers

* Removed old reducers and store

* fix tests

* implement useProxy hook in onboarding & update test with proxy setup

* Unmount after test to prevent error

* Update useProxy to expect null from watch.proxyAddress

* Use latest dai and dai-plugin-mcd versions

* Refactor Send.spec test

* add useProxy hook to DSR onboarding & update tests

* Added proxy check in Vaults provider

* make mocking observables more flexible with useMakerMock hook

* clean up tests

* initial work

* Added SaveOverview

* bump dai-mcd-plugin

* disable buttons if viewedAddress !== wallet address

* Added tests

* Reroute to connect wallet save page if connecting from read only and
viewing other addresses save page

* include url search so rerouting maintains network

* test for non-proxy account in Save

* cleanup
-round stability fee & DSR to nearest int for display.
-round balance in deposit sidebar correctly.
-fix infinity error for liquidationPrice in sidebar.

* UI improvements
-always show col. ratio
-don't round up
-don't register local schemas

* fix formatting in onboarding

* Ensure onboarding state is shown only when we have finality on the viewedAddressVaultsData

* Proxy onboarding, awaiting confirmations is more robust

* using new blockheight hook in dsr onboarding

* set hasAllowance to true if tokenSymbol is 'ETH'

* show BAT & DAI balances in hw wallet address table

* show custom error message if user does not have contract data enabled for ledger

* fix route check for Withdraw button

* display custom messages based on ledger error type

Co-authored-by: Michael Elliot <michael@digital.com.au>
Co-authored-by: Phil Bain <bain.pmcg@gmail.com>
michaelelliot added a commit to OasisDEX/mcd-cdp-portal that referenced this issue Mar 4, 2020
* Connect front-end to multicall observables via useObservable

* Stringify object type dependencies in useObservable

See: facebook/react#14476 (comment)

* Register multicall schemas on maker initialization

- Register multicall schemas on maker initialization
- Add experimental .watch convenience helper and map it to useObservable hook

* Use array of ilks for ilkPrices observable

* Use experimental .watch convenience helper

* Add type definitions for watch alias

Add typescript definitions for watch alias that extends WatchInterfaceMcd (which should be defined in dai-plugin-mcd)

* Add jsconfig.json for better editor type definition support

See: https://code.visualstudio.com/docs/languages/jsconfig#_why-do-i-need-a-jsconfigjson-file

* Return single value from useObservable hook

* Update watch subscriptions to handle single return value

* Add type definition reference to useObservable.js

* rudimentary implementation of watch.vault in Generate componenent

* Watch vault in CDPDisplay

* use observables for vault data in presentation component

* remove cdp dep from presentation

* update testchain & adjust test values for latest snapshot

* SDK-740 pass vault object to Generate component & fix tests

* Use vault computed observable helper functions

* implement vault observable with Withdraw component

* Refactor vault observable logic in CDPDisplay

* SDK-740 pass vault object to Payback component & fix tests

* SDK-740 pass vault object to Deposit component & fix tests

* SDK-740 fix CDPDisplay test

* SDK-740 fix Presentation test

* Catch errors in useObservable and set value to null

* Use only vault observable in CDPDisplay

* update price feed string in Deposit

* make vault observables formatter more flexible

* use vault.id instead of passing cdpId to sidebar components

* tweak formatter

* add symbol to price feed in sidebars

* use built in calc functions in Withdraw sidebar

* cleanup

* fix RatioDisplay colors

* add RatioDisplay component back to Generate

* fix Generate input validation

* use multicall.watch method

* fix negative value error

* Fix useObservable linter warnings

* remove references to liquidationRatioSimple

* fix setMax in Payback

* integrated multicall changes for savings ticker

* refix setMax in payback

* Simplify DsrInfo

* Fixed useEffect warnigs

* Use latest version of dai and dai-plugin-mcd packages

* skip save test while debugging

* Add missing useSavings hook dependencies

* Simplify useSavings hook

* Add feature flag for dynamic decimals for dsr ticker

* Move imports above FF

* use useVault in the main Save page and pass data to child components

* fetch balances from multicall in useWalletBalances hook

* add mocked observables to fix tests

* use observables for save sidebar details

* fixed DSR earnings ticker miscalculation

* Fixed accounting for earnings interest between starts

* Remove unnecessary useEffect in DSRInfo

* Move allowance test in save spec to top

* Use latest dai-plugin-mcd package

* Use watch observable hooks in useTokenAllowance

* update useTokenAllowance hook & fix tests

* Using useVault to populate CDPList and Overview pages

* Bump dai and dai-plugin-mcd package versions

* Set useObservable value to undefined if deps change

* Set useObservable value to undefined if deps change

* update tests to wait for proxy and allowance data

* moved useVault logic to provider instead of hook

* Fix some breaking tests for Vault provider change

* fix CDPCreate test & begin adding observables to onboarding

* update plugin to dai-plugin-mcd-v1.1.3

* Add DSR to useWalletBalances & conform to rules of hooks

* Use observables for account box wallet balances

* Add total vaults created to system sidebar

* Remove unused tokensWithBalances helper

* Update test to use showWalletTokens config

* Create and start multicall watcher in MakerProvider

* watching systemCollateralization observable

* fixes some tests and begin work on onboarding observables

* mock systemCollateralization for Save test

* move onboarding observables to parent file and remove feeds

* remove unused code related to vault data

* Add id to mock vault to prevent errors

- Add id to mock vault to prevent errors
- Revert watch.proxyAddress to using single address arg to prevent weird RxJS warnings

* check user first vault with observables

* fix some breaking changes

* use new computed observable with calc functions for collateral types

* add remaining collateral type data to onboarding

* updated useProxy to use observables

* Use find not filter on collateralTypesData

* Fix validation errors in onboarding

* Fix console.log

* removed accounts reducer

* removed cdps reducer

* remove all reducers

* Removed old reducers and store

* fix tests

* implement useProxy hook in onboarding & update test with proxy setup

* Unmount after test to prevent error

* Update useProxy to expect null from watch.proxyAddress

* Use latest dai and dai-plugin-mcd versions

* Refactor Send.spec test

* add useProxy hook to DSR onboarding & update tests

* Added proxy check in Vaults provider

* make mocking observables more flexible with useMakerMock hook

* clean up tests

* initial work

* Added SaveOverview

* bump dai-mcd-plugin

* disable buttons if viewedAddress !== wallet address

* Added tests

* Reroute to connect wallet save page if connecting from read only and
viewing other addresses save page

* include url search so rerouting maintains network

* test for non-proxy account in Save

* cleanup
-round stability fee & DSR to nearest int for display.
-round balance in deposit sidebar correctly.
-fix infinity error for liquidationPrice in sidebar.

* UI improvements
-always show col. ratio
-don't round up
-don't register local schemas

* fix formatting in onboarding

* Ensure onboarding state is shown only when we have finality on the viewedAddressVaultsData

* Proxy onboarding, awaiting confirmations is more robust

* using new blockheight hook in dsr onboarding

* set hasAllowance to true if tokenSymbol is 'ETH'

* show BAT & DAI balances in hw wallet address table

* show custom error message if user does not have contract data enabled for ledger

* fix route check for Withdraw button

* display custom messages based on ledger error type

Co-authored-by: sirromdev <sirromdev@gmail.com>
Co-authored-by: Michael Elliot <michael@digital.com.au>
Co-authored-by: Phil Bain <bain.pmcg@gmail.com>
@kotarella1110
Copy link

@kotarella1110 kotarella1110 commented Mar 6, 2020

I have built use-custom-compare hooks to solve this. I hope this will help.

Loading

@bluefire2121
Copy link

@bluefire2121 bluefire2121 commented Mar 7, 2020

I make apps with several data fetches within useEffect() and event functions. It took me a nearly a year to figure it out, but @gaearon 's option 1 is correct. Hoisting your calls up in separate modules is the right answer in 9 out of 10 cases.

Now I don't agree with option 2. Using useMemo() to deliver a static instance variable against an empty dependency array is, in my opinion, the wrong hook. Try first to determine why your dependency array should remain empty. If there are indeed no dependencies, then try useRef() instead.

For option 3, don't pass in a JSON.stringify() object/array into the list of dependency. Instead, let the hook hit and add conditionals to determine whether an inner function should proceed.

Assuming you have a shallow array of objects:

useCallback(() => {
   if (JSON.stringify(myJSONArr) !== JSON.stringify(somethingElse) {
         execFunc()
      }
}, [myJSONArr])

Loading

@charlie0077
Copy link

@charlie0077 charlie0077 commented Apr 21, 2020

I don't understand why react does not offer an option to do deep equal compare. Everyone needs to do the search and build their own, it is a waste of time. Summing everyone's time together is a ton.

Loading

@simkessy
Copy link

@simkessy simkessy commented Apr 26, 2020

I don't understand why react does not offer an option to do deep equal compare. Everyone needs to do the search and build their own, it is a waste of time. Summing everyone's time together is a ton.

Yea this is quite annoying. I settled on using object-hash package seems to work alright

Loading

@charlie0077
Copy link

@charlie0077 charlie0077 commented Apr 27, 2020

I don't understand why react does not offer an option to do deep equal compare. Everyone needs to do the search and build their own, it is a waste of time. Summing everyone's time together is a ton.

Yea this is quite annoying. I settled on using object-hash package seems to work alright

Does object-hash works for object with key in different order?

Loading

@simkessy
Copy link

@simkessy simkessy commented Apr 27, 2020

it will likely generate a new hash

Loading

@OliverJAsh
Copy link

@OliverJAsh OliverJAsh commented May 2, 2020

I'm a bit surprised by the fact that React is inconsistent here, because React.memo does have this. Why is this functionality provided there when the hooks do not have it?

Loading

@OliverJAsh
Copy link

@OliverJAsh OliverJAsh commented May 14, 2020

I wanted to share a problem I run into very frequently, which I believe would be solved if React supported custom equality functions for useMemo, useCallback and useEffect. /cc @gaearon

Problem

Take the example of deriving arrays.

Full example: https://stackblitz.com/edit/react-use-memo-result-yj2wxr?file=index.tsx

console.log(fruits);
// => ['apple', 'banana', 'grapes']

const yellowFruits = useMemo(
  () => fruits.filter((fruit) => fruit === "banana"),
  [fruits]
);
// => ['banana']

Image that fruits is memoized correctly. It is an array that only changes reference when the contents inside of it change.

When fruits changes, we recompute yellowFruits, which will create a new reference.

This means that we could have a new reference for yellowFruits even if the array contents have not changed. Example:

console.log(fruits);
// => ['apple', 'banana', 'grapes', 'pineapple'] // ✅ new content, new reference

const yellowFruits = useMemo(
  () => fruits.filter((fruit) => fruit === "banana"),
  [fruits]
);
// => ['banana'] // ❌ same content, new reference
  • If yellowFruits is passed downstream as a prop to a memo'd component, we will have wasted re-renders.
  • If it's passed to useEffect as a dependency, the effect will run more frequently than it should (as seen in the example).
  • If it's passed to useMemo or a reselect selector as a dependency, the memoized function will run more frequently than it should, causing further memoization issues downstream.

You get the idea.

Solution

Custom equality functions for useMemo, useCallback and useEffect would solve this problem:

 useEffect(() => {
   console.log("yellowFruits changed");
-}, [yellowFruits]);
+}, [yellowFruits], shallowEqual);

To demonstrate this solution, here's an example that's using use-custom-compare (thanks @kotarella1110!). But ideally we wouldn't need to resort to library for this.


We can already pass custom equality functions to reselect's createSelector and React's React.memo HOC. Why would we want to treat these hooks any differently?

Loading

@vipcxj
Copy link

@vipcxj vipcxj commented Jul 3, 2020

Right now we have no plans to do this. Custom comparisons can get very costly when spread across components — especially when people attempt to put deep equality checks there. Usually there's a different way to structure the code so that you don't need it.

There are a few common solutions:

Option 1: Hoist it up

If some value is supposed to be always static, there's no need for it to be in the render function at all.
For example:

const useFetch = createFetch({ /* static config */});

function MyComponent() {
  const foo = useFetch();
}

That completely solves the problem. If instantiating it is annoying, you can always have your own useFetch module that just re-exports the instantiated result (or even provides a few variations):

// ./myUseFetch.js
import createFetch from 'some-library';

export const useFetch = createFetch({ /* config 1 */ })
export const useFetchWithAuth = createFetch({ /* config 2 */ })

Option 2: Embrace dynamic values

Another option is the opposite. Embrace that values are always dynamic, and ask the API consumer to memoize them.

function MyComponent() {
  const config = useMemo(() => ({
    foo: 42,
  }, []); // Memoized
  const data = useFetch(url, config);
}

This might seem like it's convoluted. However for low-level Hooks you probably will create higher-level wrappers anyway. So those can be responsible for correct memoization.

// ./myUseFetch.js
import {useContext} from 'react';
import useFetch from 'some-library';
import AuthContext from './auth-context';

export function useFetchWithAuth(url) {
  const authToken = useContext(AuthContext);
  const config = useMemo(() => ({
    foo: 42,
    auth: authToken
  }), [authToken]); // Only changes the config if authToken changes
  return useFetch(url, config);
}

// ./MyComponent.js
function MyComponent() {
  const data = useFethWithAuth('someurl.com'); // No config
}

Then effectively the users don't need to configure anything at the call site — or you can expose limited supported options as specific arguments. That's probably a better API than an arbitrarily deep "options" object anyway.

Option 3: (be careful) JSON.stringify

Many people expressed a desire for deep equality checks. However, deep equality checks are very bad because they have unpredictable performance. They can be fast one day, and then you change the data structure slightly, and they become super slow because the traversal complexity has changed. So we want to explicitly discourage using deep checks — and useEffect API currently does that.

However, there are cases where it stands in the way of ergonomics too much. Such as with code like

const variables = {userId: id};
const data = useQuery(variables);

In this case there is no actual deep comparisons necessary. In this example, we know our data structure is relatively shallow, doesn't have cycles, and is easily serializable (e.g. because we plan to send that data structure over the network as a request). It doesn't have functions or weird objects like Dates.

In those rare cases it's acceptable to pass [JSON.stringify(variables)] as a dependency.

“Wait!” I hear you saying, “Isn’t JSON.stringify() slow?”

On small inputs, like the example above, it is very fast.

It does get slow on larger inputs. But at that point deep equality is also slow enough that you’ll want to rethink your strategy and API. Such as going back to a granular invalidation with useMemo. That gives you the performance — if indeed, you care about it.

There are probably minor other use cases that don't quite fit into these. For those use cases it is acceptable to use refs as an escape hatch. But you should strongly consider to avoid comparing the values manually, and rethinking your API if you feel you need that.

I have a use case.
for example, there are variables:

const a = [bigObject0, bigObject1, ...manyBigObjects] // length can be change
const b = { bigObject0, bigObject1, bigObject2, ...manyBigObjects } // length can be change
const c = 'I amd just a string'
const d = { a: 1, b: '2' } // length fixed.

React.useEffect(() => {
}, [...a, Object.keys(b).map(key => b[key]), c, d.a, d.b]);

For this use case, Option 3 not work well, because I don't want to serialize the deep objects. And if a, b can't be memorized well, option 2 not work too. Option 1 not work in many situation. So I choose the solution in the code. And it seems work well. But React always show a warning of it. React found the length of deps may change and it told me that's bad. So why bad?

Loading

@tboulis
Copy link

@tboulis tboulis commented Jul 29, 2020

I have built use-custom-compare hooks to solve this. I hope this will help.

Give this man a medal!

Loading

@justjake
Copy link

@justjake justjake commented Nov 5, 2020

I think it is quite easy to compose a solution to this problem with one or two small hooks, I think. I just started using hooks, though, so perhaps my approach is somehow incorrect

// Returns a number representing the "version" of current.
function useChangeId<T>(next: T, isEqual: (prev: T | undefined, next: T) => boolean) {
  const prev = useRef<T>()
  const id = useRef(0)
  if (!isEqual(prev.current, next)) {
    id.current++
  }
  useEffect(() => { prev.current = next }, [next])
  return id
}

interface FetchConfig { /* ... etc ... */ }

function useFetch(config:  FetchConfig) {
  // Returns a new ID when config does not deep-equal previous config
  const changeId = useChangeId(config, _.isEqual)
  // Returns a new callback when changeId changes.
  return useCallback(() => implementUseFetchHere(config), [changeId])
}

function MyComponent(props: { url: string }) {
  const fetchConfig = { url, ... }
  const fetch = useFetch(fetchConfig)
  return <button onClick={fetch}>Fetch</button>
}

Loading

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet