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

Add React-compatible update function to useMMKVStorage hook #98

Closed
SaltedBlowfish opened this issue May 16, 2021 · 5 comments
Closed

Add React-compatible update function to useMMKVStorage hook #98

SaltedBlowfish opened this issue May 16, 2021 · 5 comments

Comments

@SaltedBlowfish
Copy link
Contributor

SaltedBlowfish commented May 16, 2021

See #13 (comment) for the original mention that led to this issue being opened

Currently, the useMMKVStorage hook doesn't reflect the expectation of the "setter" portion of the React built-in state hooks. It's not documented that it should, but would provide convenience for developers implementing this package...

Ideally, we would be able to have a state change with a previous value provided, similarly to the React.useState hook for making sure value writes are happening without overwriting previous states:

React's API

const [value, setValue] = React.useState(0);
...
setValue((prev) => {
  // we have access to prev in here for making safe changes to the state
  return prev + 1
})

RN MMKV's API

const MMKV = new MMKVStorage.Loader().initialize();
const [value, setValue] = useMMKVStorage('counter', MMKV);
...
setValue(
  // cannot be a function that accepts previous state
)

To support this, the setValue function above must support a callback as a parameter for dynamically computing the next state.

Usage example:

const MMKV = new MMKVStorage.Loader().initialize();
const [value, setValue] = useMMKVStorage<number>('counter', MMKV);
...
setValue((prev) => {
  return prev === undefined ? 0 : prev + 1
})
@ammarahm-ed
Copy link
Owner

Hey @SaltedBlowfish do you plan to work on sending a PR with this feature?

@ammarahm-ed
Copy link
Owner

@SaltedBlowfish I have added support for setter. test it out on master so I can release the next version

@SaltedBlowfish
Copy link
Contributor Author

@ammarahm-ed Sorry for the delay. I did plan on working on this but I'm traveling right now and ended up not having the time. I didn't realize you'd get to it so quickly. I won't be able to test it out for another week or so though.

@SaltedBlowfish
Copy link
Contributor Author

@ammarahm-ed I ended up pulling your changes and resolving conflicts with my fork. See my PR here: #103

I have not tested it yet (like I said I'm traveling right now).

ammarahm-ed added a commit that referenced this issue May 24, 2021
#98 Add React-compatible update function to useMMKVStorage hook
@ammarahm-ed
Copy link
Owner

Shipped in v0.5.9

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