invertase / react-native-firebase

🔥 A well-tested feature-rich modular Firebase implementation for React Native. Supports both iOS & Android platforms for all Firebase services.
https://rnfirebase.io
Other
11.69k stars 2.21k forks source link

🔥 Realtime Database: ref.on() does not return unsubscription function #3812

Closed iqqmuT closed 4 years ago

iqqmuT commented 4 years ago

Issue

A text snippet from Usage of Realtime Database doc:

The event handler also returns a function, allowing you to unsubscribe from events.

Example from there:

import React, { useEffect } from 'react';
import database from '@react-native-firebase/database';

function User({ userId }) {
  useEffect(() => {
    const subscriber = database()
      .ref(`/users/${userId}`).on('value', snapshot => {
        console.log('User data: ', snapshot.val());
      });

    // Stop listening for updates when no longer required
    return () => subscriber();
  }, [userId]);
}

However, the function returned from on() does not unsubscribe from the event. Looking at the source, it looks like the given callback function is returned instead (packages/database/lib/DatabaseQuery.js:342). This seems to be a bug.

The only way to unsubscribe is to call ref.off() method:

import React, { useEffect } from 'react';
import database from '@react-native-firebase/database';

function User({ userId }) {
  useEffect(() => {
    const subscriber = database()
      .ref(`/users/${userId}`).on('value', snapshot => {
        console.log('User data: ', snapshot.val());
      });

    // Stop listening for updates when no longer required
    return () => database()
      .ref(`/users/${userId}`).off('value');
  }, [userId]);
}

Project Files

Javascript

Click To Expand

#### `package.json`: ```json # N/A ``` #### `firebase.json` for react-native-firebase v6: ```json # N/A ```

iOS

Click To Expand

#### `ios/Podfile`: - [ ] I'm not using Pods - [x] I'm using Pods and my Podfile looks like: ```ruby # N/A ``` #### `AppDelegate.m`: ```objc // N/A ```


Android

Click To Expand

#### Have you converted to AndroidX? - [ ] my application is an AndroidX application? - [ ] I am using `android/gradle.settings` `jetifier=true` for Android compatibility? - [ ] I am using the NPM package `jetifier` for react-native compatibility? #### `android/build.gradle`: ```groovy // N/A ``` #### `android/app/build.gradle`: ```groovy // N/A ``` #### `android/settings.gradle`: ```groovy // N/A ``` #### `MainApplication.java`: ```java // N/A ``` #### `AndroidManifest.xml`: ```xml ```


Environment

Click To Expand

**`react-native info` output:** ``` System: OS: Linux 5.4 Ubuntu 20.04 LTS (Focal Fossa) CPU: (8) x64 Intel(R) Core(TM) i7-7700HQ CPU @ 2.80GHz Memory: 1.27 GB / 31.32 GB Shell: 5.8 - /usr/bin/zsh Binaries: Node: 14.0.0 - ~/.nvm/versions/node/v14.0.0/bin/node Yarn: Not Found npm: 6.14.4 - ~/.nvm/versions/node/v14.0.0/bin/npm Watchman: 4.9.0 - /usr/bin/watchman SDKs: Android SDK: Not Found IDEs: Android Studio: 3.6 AI-192.7142.36.36.6392135 Languages: Java: Not Found Python: Not Found npmPackages: @react-native-community/cli: Not Found react: 16.11.0 => 16.11.0 react-native: 0.62.2 => 0.62.2 npmGlobalPackages: *react-native*: Not Found ``` - **Platform that you're experiencing the issue on**: - [ ] iOS - [ ] Android - [ ] **iOS** but have not tested behavior on Android - [x] **Android** but have not tested behavior on iOS - [ ] Both - **`react-native-firebase` version you're using that has this issue:** - `7.2.2` (@react-native-firebase/database) - **`Firebase` module(s) you're using that has the issue:** - `database` - **Are you using `TypeScript`?** - `N`


iqqmuT commented 4 years ago

According to this https://github.com/invertase/react-native-firebase/pull/3813#issuecomment-647390100 , should the documentation be updated in https://rnfirebase.io/database/usage#realtime-changes ?

russellwheatley commented 4 years ago

@iqqmuT good catch! If you'd like to make a PR, we'd be happy to accept 😄