search

facebookarchive / redux-react-hook

React Hook for accessing state and dispatch from a Redux store
MIT License
2.16k stars 103 forks source link

Fix memoize: save argument only if function does not throw #94

Closed pauldijou closed 4 years ago

pauldijou commented 4 years ago

Before, if by any chance, the fn function was to throw, the argument would have been saved into prevArg, resulting in returning value from the previous call if it was to be called again with the same argument after that.

Side note, I'm not sure if worth fixing it, but if arg is undefined on the first call, then if will match the initial prevArg and directly return value (which is also undefined). I don't think anything is preventing users from having an initial undefined state, might not happen a lot though. One easy way to fix it would be to create whatever object and assigning to let prevArg: AT = initialPrevArg; so that the strict equality can never yield true on the first call.

ianobermiller commented 4 years ago

Thanks for the clear explanation!

Side note, I'm not sure if worth fixing it, but if arg is undefined on the first call, then if will match the initial prevArg and directly return value (which is also undefined). I don't think anything is preventing users from having an initial undefined state, might not happen a lot though. One easy way to fix it would be to create whatever object and assigning to let prevArg: AT = initialPrevArg; so that the strict equality can never yield true on the first call.

Great point, I think assigning an empty object would be reasonable, with a good comment.

pauldijou commented 4 years ago

Cool. I will do another pull request for the initial prevArg.