-
-
Notifications
You must be signed in to change notification settings - Fork 145
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
fix(android): disable coalescing of native events #646
Conversation
To view this pull requests documentation preview, visit the following URL: docs.page/invertase/react-native-google-mobile-ads~646 Documentation is deployed and generated using docs.page. |
Wow that looks like a subtle one to bump in to and the change seems like it makes sense after looking through related items in react-native repo A quick look for "onnativeevent" in the repo indicates this may be needed on the iOS side as well perhaps? There is a banner ad view component with event emitter that seems to hook into that machinery |
@mikehardy Hmm, it's possible that a similar patch is needed for iOS. We only ran into this issue on Android though. |
Hello 👋, this PR has been opened for more than 2 months with no activity on it. If you think this is a mistake please comment and ping a maintainer to get this merged ASAP! Thanks for contributing! You have 15 days until this gets closed automatically |
@mikehardy Do you think it would be possible to have this merged in? I unfortunately do not have time to look into iOS as we are not having any issues with it. |
sorry this went stale - I just scanned through the iOS stuff and I'm not sure it is necessary, RCTEvent has a base implementation that has canCoalesce return FALSE, so perhaps it doesn't coalesce by default so nothing to do |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #646 +/- ##
==========================================
- Coverage 43.72% 43.11% -0.60%
==========================================
Files 30 31 +1
Lines 549 573 +24
Branches 151 156 +5
==========================================
+ Hits 240 247 +7
- Misses 309 326 +17 |
No worries. Thank you for looking into it! |
all CI passed but iOS and this doesn't touch that so I merged and hit the publish button 🚢 cheers! |
### [14.3.1](v14.3.0...v14.3.1) (2024-11-13) ### Bug Fixes * **android:** disable coalescing of native events ([#646](#646)) ([fce51b1](fce51b1))
🎉 This PR is included in version 14.3.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Description
Disable native event coalescing by setting canCoalesce to false. This library uses a single event definition for multiple events. If they happen almost at the same time, some events would otherwise get ignored.
Related issues
N/A
Release Summary
android: Fix issue where some native events may not be triggered
Checklist
and followed the process outlined there for submitting PRs.
Android
iOS
e2e
tests added or updated in__tests__e2e__
jest
tests added or updated in__tests__
Test Plan
Think
react-native-google-mobile-ads
is great? Please consider supporting the project with any of the below:Invertase
on Twitter