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

feat: If AppGroup defined in plist apply group dir. #703

Closed
wants to merge 3 commits into from

Conversation

calism2
Copy link

@calism2 calism2 commented Jul 16, 2024

This commit adds functionality to check if an AppGroup is defined in the info.plist. If it is, the group directory is applied, allowing for shared storage between the main app and its extensions.

Closes #702

This commit adds functionality to check if an AppGroup is defined in the info.plist. If it is, the group directory is applied, allowing for shared storage between the main app and its extensions.
Copy link
Owner

@mrousavy mrousavy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for the PR! Just a few minor nitpicks.

package/cpp/MmkvHostObject.cpp Outdated Show resolved Hide resolved
package/cpp/MmkvHostObject.cpp Outdated Show resolved Hide resolved
package/cpp/MmkvHostObject.cpp Outdated Show resolved Hide resolved
package/cpp/MmkvHostObject.cpp Outdated Show resolved Hide resolved
package/cpp/MmkvHostObject.cpp Outdated Show resolved Hide resolved
@calism2
Copy link
Author

calism2 commented Jul 17, 2024

Hi,

Thank you for your fast response. I have implemented the requested changes. Additionally, if the AppGroup capability is not added correctly, the app will throw a null pointer exception, which might be difficult for JavaScript developers to trace. Therefore, I have added a custom runtime error to handle this scenario.

@calism2 calism2 requested a review from mrousavy July 17, 2024 17:35
@mrousavy mrousavy mentioned this pull request Jul 22, 2024
@mrousavy
Copy link
Owner

Hey! Thanks for the updates, but I felt like moving getAppGroup into MmkvPlatformContext made more sense, as it is iOS-specific code and I don't want to cram that into the shared .cpp file.

Let me know what you think: #705

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

Successfully merging this pull request may close these issues.

Support for App Groups in MMKV 3.0.0-beta.6
2 participants