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

useCache generate a new Map everytime it's ran #80

Closed
pocesar opened this issue May 19, 2019 · 2 comments
Closed

useCache generate a new Map everytime it's ran #80

pocesar opened this issue May 19, 2019 · 2 comments

Comments

@pocesar
Copy link

pocesar commented May 19, 2019

this part of the code https://github.com/wsmd/react-use-form-state/blob/master/src/useCache.js#L4

should be using useMemo instead with some mechanism for busting itself. this library already makes a lot of useless re-renders, it should start focusing on optimizing the code

@wsmd
Copy link
Owner

wsmd commented May 19, 2019

Hi @pocesar - there was a temporary regression with caching in 0.10.1 that was fixed in the latest version yesterday, so please make sure you are on 0.10.2.

It's worth noting that the caching issue you are seeing isn't related to the map itself. In fact, useRef doesn't create a new map on every re-render:

https://reactjs.org/docs/hooks-reference.html#useref

useRef returns a mutable ref object whose .current property is initialized to the passed argument (initialValue). The returned object will persist for the full lifetime of the component.

Here's in a example to demonstrate the performance of this library running on the latest version: https://yq1l6185ox.codesandbox.io/

Please make sure you are on the latest version and if you are still having any problems, post a comment here to let me know (preferably with a code example) and I'll be happy to re-open this issue.

@pocesar
Copy link
Author

pocesar commented May 20, 2019

yeah, I'm using the latest version, but that's not what I meant. the "new Map()" is eagerly executed, and not lazily like in:

const state = useState(() => {
  return { some: 'initialState' }
})

const memoized = useMemo(() => {
  return new Map()
}, [])

the way code is created in this library seems to not deal with ref stability (for functions, objects, etc). this is a constructive criticism, that should focus on optimizations. spread props is already bad by itself and is the main drive of this hook.
when using hooks from other people, we expect that the refs are stable, like here https://github.com/kitze/react-hanger/blob/master/src/useInput.ts#L19

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