Fix firebaseObject destroyed caused delayed on('value') update failure.

[ulion]

Description

Fix a bug, which was triggered when a node set to null while the firebaseObject is on it.
the on('value') got triggered with snap value null, which call the applyUpdate method, and it delay the call with angular's $evalAsync method, but when that code finally be called, the firebaseObject was already set to null due to the destroy call.

the similar fix was already added to the error callback just below this code block, should also add to this update callback.

[jwngr]

Thanks for the pull request. For tracking purposes, can you please open a new issue and include a repro? Ideally we'd also have a test case here which reproduces the issue.

Overall, the change looks good to me, although @katowulf is the expert in this part of the code.

[ulion]

sorry, it was in a complex angular project. I don't know how to minimize the code to reproduce it. but it will happen, in theory. so hope @katowulf can comment. in my env: I was using $firebaseObject under an extended $firebaseArray, so when the $firebaseArray got $$removed call (triggered due to one of the array item object was set to null), I let it call $firebaseObject's $destroy method, and then, the applyUpdate got called with snap.val() === null, but firebaseObject is already null for that case. 2016-12-20 8:43 GMT+08:00 Jacob Wenger <notifications@github.com>:

…

Thanks for the pull request. For tracking purposes, can you please open a new issue and include a repro? Ideally we'd also have a test case here which reproduces the issue. Overall, the change looks good to me, although @katowulf <https://github.com/katowulf> is the expert in this part of the code. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#888 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAKsHRSwWw4e-oZQLAa1Od60VyARVjU1ks5rJySmgaJpZM4LRVKe> .

-- Ulion

[jwngr]

Cool, thanks @ulion. I'll let @katowulf take it from here.

[katowulf]

This update seems fine, but using synchronized objects in a synchronized array is unsupported and shouldn't be necessary. The array already handles sync of all the records, so there's no reason to be created additional overhead and listeners.

In all cases in the last several years that people have done this, it was completely unnecessary and redundant, or it was a hack that could have been implemented elegantly and more efficiently by using $extend.

[katowulf]

lgtm