FirebaseExtended / polymerfire

Polymer Web Components for Firebase
https://www.webcomponents.org/element/firebase/polymerfire
MIT License
459 stars 142 forks source link

Firestore Live Binding Splices Multiple Times. #326

Open Westbrook opened 6 years ago

Westbrook commented 6 years ago

Description

When a live collection is mapped to with more than one property:

messages: {
        type: Array,
        collection: '{roomType}/{roomName}/events',
        live: true
}

vs the more standard

messages: {
        type: Array,
        collection: 'rooms/{roomName}/events',
        live: true
}

The bindings are triggered twice.

Expected outcome

Getting a fourth item from remote on live update should turn:

Item 1
Item 2
Item 3

into

Item 1
Item 2
Item 3
Item 4

Actual outcome

Item 1
Item 2
Item 3

turns into

Item 1
Item 2
Item 3
Item 4
Item 4

and then into

Item 1
Item 2
Item 3
Item 4
Item 5
Item 5
Item 4

Steps to reproduce

  1. Create a component with the Firestore mixin.
  2. Making a live binding of a property to a collection, mapping to that collection with two properties
    messages: {
        type: Array,
        collection: '{roomType}/{roomName}/events',
        live: true
    }
  3. Add data.
  4. See it spliced in twice.

Browsers Affected

merlinnot commented 6 years ago

Hm, could you please provide some small repro of this? What’s your Polymer version? https://github.com/Polymer/polymer/releases/tag/v2.5.0 introduced some changes to the splice method, it might have an impact on this. I’ll try to take a look at this ASAP.

I believe the PR you’ve made does not fix the core problem here, unfortunately, but the concept might fix some other issues, so I’d love to help you with it.

Westbrook commented 6 years ago

Totally, I was having trouble getting this to work earlier and wanted to make sure I opened this issue, but this is the expected experience:

https://intelligent-bracket.glitch.me/

and this is the current one:

https://pepper-purchase.glitch.me/

Notice that in intelligent-bracket there are no bindings in the collection path test/test2/items allowing data to be added correctly, however when the path is updated to {prop}/{prop2}/items in pepper-purchase as I'd like to in my application the splicing is occurring twice for every add.

I tracked it down to the method _firestoreUpdateBinding getting called twice for this binding, even though _firestoreBind is only called once, which is how I was drawn to the code at line 210. With the subsequent call at line 215 prevented when the method in question has _createMethodObserver applied seems to correct this issue.

Westbrook commented 6 years ago

Also, I am using Polymer 2.5.0. However, as I can see the mixin call _firestoreAssignCollection twice for each add event, I'd be hard pressed to see how the splice changes would be related to the problems I'm experiencing with this issue.