firebase-element icon indicating copy to clipboard operation
firebase-element copied to clipboard

Added 'firebase-loaded' event to notify clients when initial data has…

Open johnlim opened this issue 8 years ago • 5 comments

… been loaded and can be accessed.

Hi,

There are instances when a client needs to know when it is able to start accessing data from firebase. Having an event to signal that really helps.

Following the docs from firebase, this PR creates an event to signal the client when data from Firebase has been loaded and can be accessed.

johnlim avatar Apr 06 '16 08:04 johnlim

Why can't clients use firebase-value and do the booking themselves?

ebidel avatar Apr 12 '16 23:04 ebidel

@ebidel They could.... but in situations where the client needs to do something only once (i.e on initial connect to firebase), without the firebase-loaded event, the client will have to do something like this

.......
   <firebase-collection location="{{location}}"
                         data="{{data}}"
                         on-firebase-value="_firebaseLoaded">
    </firebase-collection>
.......
   <script>
    Polymer({

      is: 'custom-element',

      properties: {
         _skipFlag: {
          value: false
        }
      },
     _firebaseLoaded: function() {
       if(!this._skipFlag) {
         doInitialisationStuffLikeLinkPathsEtcOnFirstLoad(); 
         this._skipFlag = true;
      }
       doOtherStuffWhenDataChanges(); 
      }

This results in clients having to "manually" set/clear flags which is error prone and also having to duplicate these checks across the app (where it's needed).

Having the firebase-loaded event would help in writing 'cleaner' client code.

johnlim avatar Apr 13 '16 02:04 johnlim

I agree it's a lot nicer to have the event and save some code writing. I'm just thinking about the cost of the listener (even a once listener) and firing an event for every users of firebase-element. The number of users that need the event might be very small compared to everyone that's using the element.

ebidel avatar Apr 13 '16 03:04 ebidel

Hmmm... we could have something like this

    _listenToQuery: function(query) {
      this._log('Adding Firebase event handlers.');
//      query.on('value', this._onQueryValue, this._onQueryCancel, this);
      query.on('child_added', this._onQueryChildAdded, this._onQueryCancel, this);
      query.on('child_removed', this._onQueryChildRemoved, this._onQueryCancel, this);
      query.on('child_changed', this._onQueryChildChanged, this._onQueryCancel, this);
      query.on('child_moved', this._onQueryChildMoved, this._onQueryCancel, this);
      query.once('value', this._onInitialDataLoaded, this);
    },
 _onInitialDataLoaded: function(snapshot) {
      this.fire('firebase-loaded', snapshot, { bubbles: false });
      this.fire('firebase-value', snapshot, { bubbles: false }); //maintain backward compatibility
      this.query.on('value', this._onQueryValue, this._onQueryCancel, this);
    },

or we could move the flag and if checking into the behaviour itself like so

    _onQueryValue: function(snapshot) {
      this._log('Firebase Event: "value"', snapshot);
      if(this._onInitialDataLoaded) {
        this.fire('firebase-loaded', snapshot, { bubbles: false }); 
      }
      this.fire('firebase-value', snapshot, { bubbles: false });
    },

but it somehow feels ugly and the original suggestion seems cleaner.

johnlim avatar Apr 13 '16 06:04 johnlim

@ebidel I've pushed an alternate proposal. Does that eliminate the concern you have?

johnlim avatar Apr 17 '16 05:04 johnlim