-
Notifications
You must be signed in to change notification settings - Fork 34
Add mmkv v4 support for MMKV dev tools #84
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
base: main
Are you sure you want to change the base?
Conversation
Refactor useMMKVDevTools to use createMMKV from v4
Bump version and update dependencies in package.json
| { | ||
| "name": "@dev-plugins/react-native-mmkv", | ||
| "version": "0.4.0", | ||
| "version": "0.4.1", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| "version": "0.4.1", | |
| "version": "0.4.0", |
kindly keep it as it was and we'll see how to bump it
| }: { | ||
| type Params = { | ||
| errorHandler?: (error: Error) => void; | ||
| storage?: MMKV; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| storage?: MMKV; | |
| storage: MMKV; |
let's make storage required. Then we don't need to worry about how to construct mmkv and the plugin potentially stays compatible with both v3 and v4 (and so on)
| } catch (e) { | ||
| handleError(e); | ||
| } | ||
| const n = storage.getNumber?.(key); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| const n = storage.getNumber?.(key); | |
| const n = storage.getNumber(key); |
no need for optional here and below?
| const result = await listener(params); | ||
|
|
||
| client?.sendMessage(`ack:${event}`, { result }); | ||
| client.sendMessage(`ack:${event}`, { result: result ?? true }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why result ?? true
| on('set', async ({ key, value }) => { | ||
| if (key !== undefined && value !== undefined) { | ||
| storage.set(key, value); | ||
| return true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what's the reason to return a boolean?
| subscriptions.push( | ||
| on('remove', async ({ key }) => { | ||
| if (key !== undefined) { | ||
| storage.remove(key); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
v3 uses the delete method. By calling delete or remove we could stay compatible with both versions.
If that's a problem for some reason, lets target only v4. in which case we need to bump the mmkv entry in peerDependencies
This pull request updates the
@dev-plugins/react-native-mmkvpackage to support the latest versions of its dependencies and aligns the code with breaking changes in thereact-native-mmkvAPI. The main focus is on upgrading toreact-native-mmkvv4 and updating the usage of its API in the codebase.Dependency upgrades:
react-native-mmkvfrom^3.1.0to^4.0.0inpackage.json, and updatedexpoto^54.0.0to ensure compatibility with the latest ecosystem.0.4.0to0.4.1to reflect these changes.Code changes for
react-native-mmkvv4 compatibility:MMKVconstructor with the newcreateMMKVfactory function inuseMMKVDevTools.ts. [1] [2]deletetoremoveto match the new API inreact-native-mmkvv4.