React-redux-firebase: bug(query): remove does not update state when query is applied

[nehvaleem]

Do you want to request a feature or report a bug?

bug

What is the current behavior?

I am facing an issue when events won't fire when I delete element from my database when firebaseconnect using queryParams. Data in redux is refreshed when creating new data, but upon deletion it stays the same.

If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal demo of the problem via codesandbox or similar.

export default compose(
  connect(
    ({ firebase, router }) => ({
      auth: firebase.auth,
    })
  ),
  firebaseConnect((props, firebase) => {
    return [
      { path: `/organizations`, populates, queryParams: [ `orderByChild=members/${props.auth.uid}`, 'equalTo=true' ] }
    ]
  }
  ),
  connect(
    ({ firebase, router }) => ({
      organizations: firebase.data.organizations,
      isRequesting: firebase.requesting.organizations,
      location: router.location
    })
  )
)(Organizations)

and when I am adding new organization it works fine, eg.

createOrganization(data) {
    this.props.firebase.pushWithMeta('organizations', {
      name: data.name,
      members: {
        [this.props.auth.uid]: true
      },
      owners: {
        [this.props.auth.uid]: true
      },
      projects: []
    }).then((snapshot) => {
      console.log('data pushed', snapshot);
      // this.refs.createOrganizationForm.refs.formsy.reset();
      this.refs.createOrganizationForm.formsyForm.reset();
    });
  }

When creating new organization data in redux is refreshed.

But when I try to delete one, the data in firebase redux isn't refreshed (until I refresh the page).

deleteOrganization(event, organizationId) {
    event.preventDefault();

    this.props.firebase.remove(`organizations/${organizationId}`).then((snapshot) => {
      console.log('organization removed', snapshot);
    });
  }

What is the expected behavior?

Data in firebase redux key should be updated when deleting.

Which versions of dependencies, and which browser and OS are affected by this issue? Did this work in previous versions or setups?

"react-redux": "^5.0.6",
"react-redux-firebase": "^2.0.0-beta.6",
"react-router-dom": "^4.1.2",
"react-router-redux": "^5.0.0-alpha.6",
"react-scripts": "1.0.10",
"react-test-renderer": "^15.6.1",
"react-three-renderer": "^3.2.1",
"redux": "^3.7.2",
"redux-devtools-extension": "^2.13.2",
"redux-saga": "^0.15.6",

I think that issue is browser/os independent.

[prescottprue]

Going to try to reproduce myself, but if the query is attached at that path, then removing data should come through the listener (and therefore update redux).

Side Note:

Not sure it is related, but I don't believe you can do orderByChild=members/someId. From what I understand, orderByChild only accepts a single key(not a nested key).

[nehvaleem]

Thanks, let me know if I can provide some additional info. Creating data at given path works, but removing doesn't.

In terms of nested keys I think there is an option :) First of all it works for me, and secondly https://firebase.googleblog.com/2015/09/introducing-multi-location-updates-and_86.html (section about _Ordering queries by deep paths_)

[gwitteveen]

Facing a similar issue

const FILESPATH = 'media';

export default compose(
firebaseConnect([
    FILESPATH
]),
connect(
  ({  nav, mediaselector:{ itemsSelected, trash }, firebase: { data: { media }} }) => ({
      media,
      nav,
      itemsSelected,
      trash,
  })
))(Ads)

When a file is uploaded/key is added, 'media' updates. When a file/key is removed from 'media', the key itself is removed from ...firebaseio.com, but media does not update.

Here is where it gets interesting: when I first delete a key (media doesn't update) and then add a key, media does update but both the deleted key and new key are in media.

When all keys are removed from media, media does update and does not contain any keys as expected.

    "react-redux": "^4.4.5",
    "react-redux-firebase": "^2.0.0-beta.7",

[prescottprue]

This seems to be due to recent additions of merging logic.

Current Plan

Add an action (something like REMOVE) that will be fired when calling remove method at a certain path. With this, data in redux will remain updated as things are removed. This does not handle the case of data being removed outside of the method call.

Outside of remove

Since this is the opposite of what others have been going for with the MERGE logic (#255) it will require a little more planning (open to suggestions and proposed syntax examples).

Here are some ideas I had so far:

Query Config
Passing a config option in the query that forces the listener to "run over" (i.e. not merge) the path in redux when data is received.

Maybe something as simple as this:

firebaseConnect([{ path: 'todos', merge: false }])

Not sure how clear that would be to indicate within the docs without explaining merging, so maybe a better name?

Reducer Comparison Logic

There is also the idea of trying to handle this in the reducer logic through comparisons of listener response and current redux state. The main issue with this is that is can get "expensive" depending on this size of objects.

[prescottprue]

Can anyone confirm they are still experiencing this? @nehvaleem? If so, this can be reopened, but going to close for now.

I am no longer able to replicate through calling remove or through removing directly on the database. Going to add a test that checks this case sometime soon.

[nehvaleem]

@prescottprue to be honest I haven't checked this very precisely but I've noticed that in my case problem seems to be gone since beta.8 or later. I will let you know if the problem resurface.

[prescottprue]

@nehvaleem Thats right about where I was expecting it would have been caught too. Great to know, thanks for the update.

[sshadmand]

Just implemented a remove action and experiencing the same problem. Calling ref.child(...).remove() is not updating the data saved to firebase.data.

    "react-redux": "^5.0.6",
    "react-redux-firebase": "^2.0.0-beta.8",

[prescottprue]

@sshadmand You need to update 😄 . The 2 series is on 2.0.0-beta.14 now. You can switch your package file to "react-redux-firebase": "next" if you always want the most up to date version of 2.0.0 (until it is stabilized and released on latest).

[sshadmand]

Worked. Thanks @prescottprue!

[KubaKocieba]

I'm still having the problem in ver 2.0.4 :(. Removing item(s) is ok. But the event listener is broken then and doesn't update redux-state on adding new items to the database.

[prescottprue]

@KubaKocieba We may want to open an new issue for what you are mentioning with steps to reproduce. Not sure it is the same thing being mentioned here since deleting works.

Using remove shouldn't be affecting listeners attached by firebaseConnect - even if the item is deleted on which you have a listener, the data should become null not remove the listener.

[KubaKocieba]

@prescottprue , just please tell me how should I describe the problem and way to reproduce.

[prescottprue]

@KubaKocieba Create a new issue describing what you are seeing, what you expect to see, and steps to make it happen as described in the issue template.

It can take the form of a codesandbox, codepen, code snippets, etc.

[KubaKocieba]

ok, I prepared some code in codesandbox:
https://codesandbox.io/s/mym9p5jo99
Firebase config is empty there now.
Seems that everything is fine when setting config, though.
It might be my application's code fault, so I'm sorry for disturbing,
I'll try to find my mistake.

[towfiqi]

@prescottprue Having the same issue. After loading the app, when I add an item, it works fine. But removing the newly added items doesnt update the state. even though the item is being removed from Firebase successfully.
If I reload the app and remove the item, and the state is updated perfectly. So removing dynamically added new items does not update the state.

I tried your suggestion of merge="false" did not work:

export default compose(
    firebaseConnect((props, firebase) =>  props.auth.uid && [{ path: `/playlists/${props.auth.uid}/`, merge: false }] ),
    connect(
      (state, props) => ({
        userSettings: state.user,
        playlists: getVal(state.firebase, `data/playlists/${props.auth.uid}`), 
        userplaylists: state.userplaylists
      }),
      {addSong, hideAddSongModal}
    )
  )(Playlists);

Remove Action:

export const removeSong = (uid, playlistID, songID) =>
  (dispatch, getState, getFirebase) => {
    const firebase = getFirebase();
    firebase.remove(`playlists/${uid}/${playlistID}/songs/${songID}`)
      .then((data) => {
          console.log('REMOVE: Done!');
      })
};

[towfiqi]

Ok. Just set the dispatchRemoveAction to true in config and its working fine.

[prescottprue]

merge: false is not yet supported. The merge parameter is an example of what the syntax may be in the future to support this.

Using dispatchRemoveAction or getting data from ordered instead of data should get things to update as you expect.

[towfiqi]

@prescottprue Scott, sorry to bother again.

Just moved from firebase to firestore and used firestoreConnect instead of firebaseConnect and same issue occurs. dispatchRemoveAction set to true, but doesnt solve it. Removing item doesnt update the state. Any help? Thanks.

Edit: Issues in detail:

1. When a document is created and then deleted without the page refresh, the state is not updated, although the document is deleted from the collection.

2. And when a existing document is deleted, its not removed from the state, instead , it removes all the attributes of that document and replace them with null.

Before delete:

{
  kl2jah21jma: {count:3, data:'xwqsaa'}, 
  x3nja5soa: {count:10, data:'blablabla'}
}

After Delete:

{
  kl2jah21jma: null, 
  x3nja5soa: {count:10, data:'blablabla'}
}

P.S: Using the latest version of react-redux and react-redux-firebase

[prescottprue]

@towfiqi null means that the data is loaded and does not exist. When the value is undefined that means it hasn't been loaded at all. For folks that have a listener attached on a path that they are deleting, they would see isLoaded(someProps) === false, where you would want to see it equal to true since you do know it is loaded, that path is just empty. With a listener attached, you would then see a flash of it going from value -> undefined -> null.

Open to ideas, just want to make sure we aren't breaking functionality that others are depending on.

[towfiqi]

Thanks. I understand. I am counting the item counts like this in several places through the app: Object.keys(data).length. I guess i will filter out the data with null value or use getVal.

Maybe thats whats causing issue number 1 too. (deleting a just created item is not removed). Which strangely not occurring any more. If it does, I will let you know with more info. Thanks

[prescottprue]

@towfiqi If you are counting items you could do Object.keys(data).filter(item => !!item).length to remove null items or you could use lodash's compact: compact(Object.keys(data)).length.

Also, something to note: If you are counting items and/or mapping over them it may be easier to use state.firebase.ordered (an ordered array of objects) instead of state.firebase.data (object by keys). That way you wouldn't be doing Object.keys at all, and the data.length would be correct. Then if you ever need data for a specific item, you just need to pass around the key and you can look the item up from data.

[towfiqi]

Yeah thats what I was thinking of doing to filter the data. And since I am using firestore, I am not cannot use state.firebase.ordered. Thanks.

[prescottprue]

@towfiqi There is also state.firestore.ordered (since redux-firestore is setup the same way as react-redux-firebase). If you are filtering client side after the query make sure to use a selector.

[towfiqi]

Thanks it works! :)