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

Update memorize function #59

Open
mpcarolin opened this issue Oct 26, 2020 · 2 comments
Open

Update memorize function #59

mpcarolin opened this issue Oct 26, 2020 · 2 comments

Comments

@mpcarolin
Copy link
Contributor

mpcarolin commented Oct 26, 2020

The JSUtils memorize function currently has some behavior I think is non-intuitive or borderline buggy. I have some ideas about fixing this soon, but I'm writing this here so as not to forget.

  • by default, if you do not define the getCacheKey function, it memoizes the function call using only the first argument as its cache key. I would expect it to use all the arguments by default.
  • the cache value is set using a computed property name:
{
  [key]: value
}
  • this implicitly calls key.toString() to get the actual cache key. However, if the argument is an object, to key will always be [ object Object ], which would reuse the same cached value for different input.
  • I would rather it use a shallow-equal comparison for objects, like react's useMemo hook.
@lancetipton
Copy link
Contributor

lancetipton commented Oct 26, 2020

Yah, I remember you bringing this up a while ago. It's good point.
And since it was brought up, A shallow equal helper has been added to the library, so we could just reuse that.
Feel free to create an issue is Jira, so we can get it done

@mpcarolin
Copy link
Contributor Author

Yah, I remember you bringing this up a while ago. It's good point.
And since it was brought up, A shallow equal helper has been added to the library, so we could just reuse that.
Feel free to create an issue is Jira, so we can get it done

Okay will do

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

2 participants