-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
[🐛 🔥] RN 0.70 database TypeError: undefined is not an object (evaluating '_SharedEventEmitter.default._subscriber.getSubscriptionsForType') [released for v15 and v14] #6534
Comments
@mikehardy maybe because Meta re-writed edit: Yeah, this might be the reason, this is the commit prior the above one where |
Oh no, last time they did that (across the 64/65 boundary if I recall) it was pretty messy for us internally. I'll have to look into it unless you beat me to it. Thanks for posting up the commit hash, that helps |
@mikehardy it seems it's really complex and I don't have full picture of react-native-firebase internals so I won't touch it. |
To be honest - and this is shared mostly for developer commiseration / humor - I thought I had a full picture of react-native-firebase internals until the last time they rewrote EventEmitter. I did not 😆 - after repairing the damage I think I did though. Investigating a solution now, it may be that we need to do the listener accounting ourselves / internally instead of previous style where we deferred to upstream superclass / react-native internals |
@mikehardy good luck this time 😆😆😆😆😆 |
With apologies I don't have time to test this but you have actually committed to this repo so I am going to boldly assume you are willing to roll up your sleeves and dive in a bit? Hopefully correct 💪 This is the important line for new code: facebook/react-native@e5c5dcd?diff=split#diff-0a3d564bafb3892ce96f82c6fe19b293cd3e15fe1f14d2577c03326cf9bc5cf8R127 That's a demonstration of how you can introspect on the new-style registry of listeners, though they are only exposing a listener count, while we need to get the actual listeners. So my thinking for a repair is:
Okay, what's the new logic? Do what That results in something like this as the new function: removeListenerRegistrations(listener, registrations) {
if (!Array.isArray(registrations)) {
return [];
}
const removed = [];
for (let i = 0, len = registrations.length; i < len; i++) {
const registration = registrations[i];
let subscriptions;
// EventEmitter in react-native < 0.70 had a `_subscriber` property with a method for subscriptions by type...
if (SharedEventEmitter._subscriber) {
subscriptions = SharedEventEmitter._subscriber.getSubscriptionsForType(registration);
} else {
// ...react-native 0.70 now stores subscribers as a map of Sets by type in `_registry`
const registrySubscriptionsSet = SharedEventEmitter._registry[registration];
if (registrySubscriptionsSet) {
subscriptions = Array.from(registrySubscriptionsSet;
}
}
if (subscriptions) {
for (let j = 0, l = subscriptions.length; j < l; j++) {
const subscription = subscriptions[j];
// The subscription may have been removed during this event loop.
// its listener matches the listener in method parameters
if (subscription && subscription.listener === listener) {
subscription.remove();
removed.push(registration);
this.removeRegistration(registration);
}
}
}
}
return removed;
} If you patch that in (use patch-package of course...) I think it will work, and if so, that's the PR... |
sure man, will patch it now and come back, it's gonna take a while since I rolled back to 0.68 |
On the plus side - patching it prior to moving forward is actually really good to know! It has to work on 0.68 as well - the idea is it is backwards and forwards compatible |
@mikehardy error went away. Thanks man. Here's the patch for anyone else that encountered this: diff --git a/node_modules/@react-native-firebase/database/lib/DatabaseSyncTree.js b/node_modules/@react-native-firebase/database/lib/DatabaseSyncTree.js
index 29c3057..59d4653 100644
--- a/node_modules/@react-native-firebase/database/lib/DatabaseSyncTree.js
+++ b/node_modules/@react-native-firebase/database/lib/DatabaseSyncTree.js
@@ -161,7 +161,18 @@ class DatabaseSyncTree {
for (let i = 0, len = registrations.length; i < len; i++) {
const registration = registrations[i];
- const subscriptions = SharedEventEmitter._subscriber.getSubscriptionsForType(registration);
+ let subscriptions;
+
+ // EventEmitter in react-native < 0.70 had a `_subscriber` property with a method for subscriptions by type...
+ if (SharedEventEmitter._subscriber) {
+ subscriptions = SharedEventEmitter._subscriber.getSubscriptionsForType(registration);
+ } else {
+ // ...react-native 0.70 now stores subscribers as a map of Sets by type in `_registry`
+ const registrySubscriptionsSet = SharedEventEmitter._registry[registration];
+ if (registrySubscriptionsSet) {
+ subscriptions = Array.from(registrySubscriptionsSet);
+ }
+ }
if (subscriptions) {
for (let j = 0, l = subscriptions.length; j < l; j++) {
|
@mikehardy will roll back and test on 0.68 |
@mikehardy patch works fine on |
I really appreciate all the testing - that has been a huge help - I agree 15.4.0 should work unchanged, there were no changes in that file in the releases between 14.11.1 and 15.4.0. I'll form up a PR as soon as I get a chance but it might be a day or so, I've got an unrelated deadline I'm working towards which is why I couldn't test it in the first place 😅 |
@mikehardy no worries, I might make a PR a little bit later. |
Just kicked off the release process for this one - v15.5.0 should be out in a moment with your PR included, thanks again |
For anyone that sees this, I've included it in a "courtesy release" of the v14 version series, as I know there are a lot of people still working through problems with the new If you pull the semver major v14 series or npm dist tag |
Thanks @mikehardy! |
@muzzamil76 it’s really bad to mix different versions, you’re using |
As for version mixing - here's our policy (TL;DR: do not mix them) - https://invertase.io/blog/react-native-firebase-versioning |
Issue
After upgrading
react-native
to0.70.0
andreact-native-firebase
to15.4.0
Database module throwsTypeError: undefined is not an object (evaluating '_SharedEventEmitter.default._subscriber.getSubscriptionsForType')
and the culprit is this line.It doesn't throw all the time though. It happens in both iOS (16) and Android 12.@mikehardy if you need any additional info let me know.
Just some
ref.on
andref.off
(I think these are causing the isssue):Describe your issue here
Calling
ref.off
orref.on
throws the error above.Project Files
Javascript
Click To Expand
package.json
:firebase.json
for react-native-firebase v6:# N/A
iOS
Click To Expand
ios/Podfile
:AppDelegate.m
:// N/A
Android
Click To Expand
Have you converted to AndroidX?
android/gradle.settings
jetifier=true
for Android compatibility?jetifier
for react-native compatibility?android/build.gradle
:android/app/build.gradle
:android/settings.gradle
:MainApplication.java
:AndroidManifest.xml
:Environment
Click To Expand
react-native info
output:react-native-firebase
version you're using that has this issue:15.4.0
Firebase
module(s) you're using that has the issue:Database
TypeScript
?Y
&4.8.3
React Native Firebase
andInvertase
on Twitter for updates on the library.Click To Expand and see Android 12 screenshot
The text was updated successfully, but these errors were encountered: