prescottprue / redux-firestore

Redux bindings for Firestore
MIT License
575 stars 102 forks source link

discussion: Document_added performance #191

Open compojoom opened 5 years ago

compojoom commented 5 years ago

I was discussing the following problem with a colleague today and I'm wondering if other have experience the same behavior.

We have a component that does a redux firestore query similar to this:

firestore().collection('whatever').where('item_id', '==', 'XX').where('comment_id', '==', 'YY')

This query returns a small number of documents. Then in another component we have a query that does only: firestore().collection('whatever').where('item_id', '==', 'XX')

Now firestore immediately returns the documents that we had with the first query and then a few miliseconds later we get a second snapshot with more data.

Redux firestore looks at the docChanges and triggers a document_added for every change.

Now the problem is that the query returns like 100+ documents. Our first query returned like 5, then the second one returns 100+ and we end up dispatching 100+ document_added events on the store. Needless to say -despite pure components our page freezes because we have a lot of connected components.

We managed to get around the problem by manually doing the query in componentDidMount and then dispatching a single listener_response. This way we only trigger 2 events. The first with 5 elements and the second with 100+. Now the page doesn't freeze.

So this got we thinking about this here: https://github.com/prescottprue/redux-firestore/blob/master/src/utils/query.js#L672

So how about instead of dispatching that many document_added we loop over the changes and group them to added, updated, deleted and dispatch single actions with those. Obviously we need slightly different reducers to handle the array of changed documents, but this should be doable.

There is a comment in that file TODO: Option for dispatching multiple changes in single action so I guess that @prescottprue already thought about that?

Wgil commented 5 years ago

I'm also suffering of a similar situation. I want to load a page with all my users in it. The issue is that LISTENER_RESPONSE returns immediately the logged in user and then I get 100+ DOCUMENT_ADDED causing my page to freeze. I think if I remove userProfile config, I'm going to get all the users in only one LISTENER_RESPONSE action. I'd need to refactor my code that depends on the current user if I remove that option.

Setting the listener manually and don't removing it solves the issue for the second time I visit the page, I still need to figure out what to do for first load.

@compojoom

We managed to get around the problem by manually doing the query in componentDidMount and then dispatching a single listener_response. This way we only trigger 2 events. The first with 5 elements and the second with 100+. Now the page doesn't freeze.

Could you please elaborate more on this?

Wgil commented 5 years ago

I couldn't figure out how to make my listener load all users at once. Storing users in different location using storeAs didn't make the trick. I ended up disabling react-redux-firebase userProfile options:

const defaultRRFConfig = {
    // userProfile: 'users', // root that user profiles are written to
  }

And then load the profile manually with:

compose(
  withStore,
  withFirestore,
  setPropTypes({
    dispatch: PropTypes.func.isRequired,
    firestore: PropTypes.object.isRequired,
    uid: PropTypes.string.isRequired
  }),
  lifecycle({
    async componentDidMount() {
      const doc = await this.props.firestore.get({
        collection: 'users',
        doc: this.props.uid
      })
      const profile = doc.data()

      this.props.dispatch({
        type: reactReduxFirebaseActionTypes.SET_PROFILE,
        profile
      })
    }
  })
)

If I go to my users screen, they are loaded in a single action. If you need firebase.profile, just grab it from there.

compojoom commented 5 years ago

sorry for the slow response:

import { dataByIdSnapshot, orderedFromSnap, getQueryConfig, firestoreRef } from 'redux-firestore/src/utils/query'

const meta = getQueryConfig(getHarvestedCropsQuery(companyId, browsingGroup, myCompanyProfile, dateRange))
    this.unsubscribe = firestoreRef(this.props.firebase, meta)
      .onSnapshot((docData) => {

        this.props.dispatch({
          type: actionTypes.LISTENER_RESPONSE,
          meta,
          payload: {
            data: dataByIdSnapshot(docData),
            ordered: orderedFromSnap(docData),
          },
          merge: {
            docs: true,
            collections: true
          },
        })
      })

you obviously need to pass dispatch as prop to the component. The meta object is the query as you need it in your component

Wgil commented 5 years ago

Above solution made my real-time profile stop working, @compojoom thanks, your code helped me to load the users list manually.

Wgil commented 4 years ago

I'm facing this issue again but this time I have only one query on the same collection:

const query = {
        collection: Team.collection,
        doc: this.props.profile.teamId,
        subcollections: [{ collection: Entity.collection }],
        storeAs: Entity.collection
      }

I tried removing storeAs, using firestoreConnect and setListener without any luck. Not only DOCUMENT_ADDED action is dispatched but also DOCUMENT_MODIFIED.