GoogleWebComponents / firebase-element

Web components for the Firebase Web API
https://elements.polymer-project.org/elements/firebase-element
95 stars 79 forks source link

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

Open johnlim opened 8 years ago

johnlim commented 8 years ago

… 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.

ebidel commented 8 years ago

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

johnlim commented 8 years ago

@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.

ebidel commented 8 years ago

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.

johnlim commented 8 years ago

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 commented 8 years ago

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