Missing check for promise function together with the inputs

[namnm]

We need to improve the if statement in the cache check to: if (promiseCache.promiseFn === promise && deepEqual(inputs, promiseCache.inputs)) Example of an invalid case:

const fn1 = () => { ... }
const fn2 = () => { ... }
// Because both functions have the same parameter
//  so the deepEqual can not guarantee the correct cache

To do so, when construct the cache item, you need to save the promiseFn as well:

  const promiseCache: PromiseCache = {
    promiseFn: promise,
    promise:
      ...
  };

[ngocdaothanh]

save the promiseFn as well...

I think we can't use promiseFn as part of the cache key, because:

• promiseFn can be created dynamically, different instances of the same function won't be equal. For example, (() => 1) === (() => 1) returns false.
• promiseFn.toString() is good enough, but we can't rely on it 100%, because different functions may have the same toString() result.

One solution is designing usePromise to have a category parameter (which will be included to cache key), like this:

const usePromise = (
  promise: (...inputs: any) => any, 
  inputs: Array<any>, 
  category: string = promise.toString(), 
  lifespan: number = 0
)

category is set to promise.toString() by default because it's good enough.

[ngocdaothanh]

For now, I'm using this wrapper in my projects:

import originalUsePromise from 'react-promise-suspense'

type Options = {
  category?: string
  lifespan?: number
}

export default function usePromise<T>(
  promise: (...inputs: any[]) => Promise<T>,
  inputs: any[],
  options: Options = {}
): T {
  const category = options.category || promise.toString()
  const lifespan = options.lifespan || 0

  const cacheKeys = [category, ...inputs]
  const callPromise = () => promise.apply(null, inputs)
  return originalUsePromise(callPromise, cacheKeys, lifespan)
}

[namnm]

@ngocdaothanh Thanks for giving your snippet, it is really insightful.

You are right we should not compare the function reference (I was proposing that because at that time most of my functions were declared outside of render function and thus they were static)

However this library is not in maintain and has other issues as well which the author doesnt seem to have interest on. So, do you think that you would like to fork the repo and publish a new package? I'm glad to support and use it in my projects.

[ngocdaothanh]

@namnm From the issues you created at this repo, it seems you are very passionate about this project, and you have more than a year of experience with this project, more than me. So please fork the repo and publish a new package.

[msereniti]

You can use fork of this package that solves this issues and many others react-use-await