-
Notifications
You must be signed in to change notification settings - Fork 377
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
Update Push Subscription/Permission Getters to Async #1649
Conversation
- Create new async methods - Mark old getters as deprecated - Update permissionNative to access value from bridge vs. local
- Mark old methods as deprecated
0dd5572
to
e898034
Compare
@nan-li I Successfully reran test scenarios on Android 14 Emulator and iOS 17.2 Emulator:
|
|
||
//Previous state | ||
NSMutableDictionary *previousObject = [NSMutableDictionary new]; | ||
previousObject[@"token"] = (state.previous.token && ![state.previous.token isEqualToString:@""]) ? state.previous.token : [NSNull null]; |
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.
These lines get pretty complex, we can consider refactoring these into getter helpers, that can also be used for the User State observer., once this PR and that one are combined / merged.
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.
We should also update the PR description to callout the API "breaking-sorta" change that the push subscription observer will be passed nullable properties now.
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.
Thanks for flagging these! I've edited the PR description with the minor breaking change and will be sure to include it in release notes. Refactoring mentioned above will happen in a future PR.
Description
One Line Summary
Update Push Subscription/Permission Getters to Async
Details
Motivation
We received reports of listeners not behaving as expected; certain getters were returning null due to the value not being available until returned from the native bridge. Updating getter methods to async will address this issue.
Scope
New async methods were added while also marking old methods as deprecated. Also updated
requestPermission
andpermssionNative
to account for changes made to these getters.Testing
Manual testing
Tested running and building app on Android 14 emulator and iOS 17.2 Emulator, to ensure new methods work as expected.
Affected code checklist
Checklist
Overview
Testing
Final pass
This change is