adriancarriger / angularfire2-offline

🔌 A simple wrapper for AngularFire2 to read and write to Firebase while offline, even after a complete refresh.
https://angularfire2-offline.firebaseapp.com/
MIT License
207 stars 48 forks source link

Object field order on connected update causes unwanted subscribe. #57

Closed tja4472 closed 7 years ago

tja4472 commented 7 years ago
export class TodoService {
    private fb_CurrentTodos$: AfoListObservable<any[]>;

    constructor(
        db: AngularFireOfflineDatabase,
    ) {
        console.log('TodoService:constructor');
        this.fb_CurrentTodos$ = db.list(FIREBASE_CURRENT_TODOS);

        this.fb_CurrentTodos$.subscribe(x => {
            console.log('this.fb_CurrentTodos$.subscribe', x);
        });
    }

When connected this will cause the subscribe to be called once.

let desc: string = 'a' + Math.random();
this.fb_CurrentTodos$.update("-KXpuvXAo3jYqlp5s8OH",
    {
        description: desc,
        index: 1,
        isComplete: false,
        name: "second",
    });

When connected this will cause the subscribe to be called twice.

let desc: string = 'a' + Math.random();
this.fb_CurrentTodos$.update("-KXpuvXAo3jYqlp5s8OH",
    {
        description: desc,
        isComplete: false,                
        index: 1,
        name: "second",
    }); 

Note that the order of the fields is different.

  uniqueNext(newValue) {
    console.log('uniqueNext');
    // Sort
    if (this.previousValue) { this.previousValue.sort((a, b) => a.$key - b.$key); }
    if (newValue) { newValue.sort((a, b) => a.$key - b.$key); }

    console.log('uniqueNext:previousValue', this.previousValue);
    console.log('uniqueNext:newValue', newValue);

    if (this.updated > 1 || (stringify(this.previousValue) !== stringify(newValue))) {
      this.previousValue = Object.assign([], newValue);
      console.log('uniqueNext:this.next(newValue) ***********');
      this.next(newValue);
      this.updated++;
    }
  }

Chrome log

internal-list-observable.ts:126 updateSubscribers
internal-list-observable.ts:72 uniqueNext
internal-list-observable.ts:77 uniqueNext:previousValue [{…}]0: {description: "a0.9693274861715127", index: 1, isComplete: false, name: "second", $exists: ƒ, …}length: 1__proto__: Array(0)
internal-list-observable.ts:78 uniqueNext:newValue [{…}]0: {description: "a0.12690608011070337", isComplete: false, index: 1, name: "second", $exists: ƒ, …}length: 1__proto__: Array(0)
internal-list-observable.ts:82 uniqueNext:this.next(newValue) ***********

internal-list-observable.ts:72 uniqueNext
internal-list-observable.ts:77 uniqueNext:previousValue [{…}]0: {description: "a0.12690608011070337", isComplete: false, index: 1, name: "second", $exists: ƒ, …}length: 1__proto__: Array(0)
internal-list-observable.ts:78 uniqueNext:newValue [{…}]0: {description: "a0.12690608011070337", index: 1, isComplete: false, name: "second", $exists: ƒ, …}length: 1__proto__: Array(0)
internal-list-observable.ts:82 uniqueNext:this.next(newValue) ***********

todo.service.ts:34 this.fb_CurrentTodos$.subscribe [{…}]0: {description: "a0.12690608011070337", isComplete: false, index: 1, name: "second", $exists: ƒ, …}length: 1__proto__: Array(0)
todo.service.ts:134 fromFirebaseTodo
todo.service.ts:34 this.fb_CurrentTodos$.subscribe [{…}]0: {description: "a0.12690608011070337", index: 1, isComplete: false, name: "second", $exists: ƒ, …}length: 1__proto__: Array(0)
todo.service.ts:134 fromFirebaseTodo

For the second 'this.next' the array is identical apart from the field ordering

previousValue

0: {description: "a0.12690608011070337", isComplete: false, index: 1, name: "second", $exists: ƒ, …}

newValue

0: {description: "a0.12690608011070337", index: 1, isComplete: false, name: "second", $exists: ƒ, …}

Tim

adriancarriger commented 7 years ago

Hi @tja4472, thanks for the post!

This is probably related to using JSON.stringify.. I'm open to using other methods, but I'm hoping to not seriously impact performance.

Anyone interested, feel free to leave your thoughts on if it's better to keep JSON.stringify (for performance) or switch to another method (for accuracy). Thanks!

tja4472 commented 7 years ago

Hi @adriancarriger

Thanks for the interesting links.

I would keep stringify and I'll make sure I have the object properties in the correct order when updating. :smile:

Regards Tim

adriancarriger commented 7 years ago

Sounds good! 👍