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

Cast listener with __block #11461

Merged
merged 1 commit into from Jun 20, 2023
Merged

Cast listener with __block #11461

merged 1 commit into from Jun 20, 2023

Conversation

qdpham13
Copy link
Contributor

Cast Listener with __block so changes can be made inside dispatch_async block.
Fixes bug reported by user

@google-oss-bot
Copy link

1 Warning
⚠️ Did you forget to add a changelog entry? (Add #no-changelog to the PR description to silence this warning.)

Generated by 🚫 Danger

@google-oss-bot
Copy link

Coverage Report 1

Affected Products

  • FirebaseRemoteConfig-iOS-FirebaseRemoteConfig.framework

    Overall coverage changed from 71.00% (3f5972f) to 71.01% (60b4cf2) by +0.01%.

    FilenameBase (3f5972f)Merge (60b4cf2)Diff
    RCNConfigRealtime.m35.85%35.93%+0.08%

Test Logs

  1. https://storage.googleapis.com/firebase-sdk-metric-reports/HTl7mODq6T.html

@mikehardy
Copy link
Contributor

Fantastic, thanks @qdpham13 - I was able to view the raw file from your tree, paste that into my build in place of my current file contents and re-test my scenario.

Previously, since the listeners were not removed from the SDK they would accumulate, and at the javascript layer of react-native-firebase we would see the clear symptom of more events than we expected (specifically: we would see one extra event for each add/remove cycle we did as we probed the APIS and our implementation)

With your change, I no longer see an incremental accumulation of extra callback events, indicating that the listeners are now being removed inside the SDK --> so this appears to work 👍

Copy link
Contributor

@mikehardy mikehardy left a comment

Choose a reason for hiding this comment

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

weak objective-c skills and zero authority in this repo :-) but I did test it, appears to work

@paulb777 paulb777 added this to the 10.12.0 - M134 milestone Jun 20, 2023
@qdpham13 qdpham13 merged commit 0e4aa6d into master Jun 20, 2023
43 checks passed
@qdpham13 qdpham13 deleted the realtime-rc-patch-1 branch June 20, 2023 23:38
@firebase firebase locked and limited conversation to collaborators Jul 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

remote config realtime listeners not removed
4 participants