Skip to content
This repository was archived by the owner on Mar 17, 2025. It is now read-only.

Conversation

jwngr
Copy link

@jwngr jwngr commented Nov 6, 2014

@katowulf - This is a backwards-compatible solution to upgrade AngularFire to 2.0.x without having it throw any deprecation warnings. Unfortunately, we cannot polyfill DataSnapshot.key() since we do not publicly expose DataSnapshot, only Firebase. As a result, I had to add these helper methods to use key() if it exists, or otherwise use name(). We need to make sure that in the future, we are aware that we cannot do things like snapshot.key() or snapshot.name() and instead need to use this._getSnapshotKey(snapshot).

The good news is that this is literally all we need to do to achieve backwards compatibility. We don't use the deprecated limit() method internally and all other methods are already backwards compatible.

cc/ @davideast

@coveralls
Copy link

Coverage Status

Coverage increased (+0.03%) when pulling ee6a21c on jw-key-polyfill into a4e4671 on master.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment this and explain why it exists? (that we we won't wonder what it was here for in a year) Might also want to //todo it so we can remove it later.

@katowulf
Copy link
Contributor

katowulf commented Nov 6, 2014

Looks good @jwngr; two comments for you to consider.

@jwngr
Copy link
Author

jwngr commented Nov 6, 2014

@katowulf - Back to you. Thanks for the comments!

katowulf added a commit that referenced this pull request Nov 6, 2014
Update methods to Firebase 2.0.x
@katowulf katowulf merged commit cd4ef13 into master Nov 6, 2014
@katowulf
Copy link
Contributor

katowulf commented Nov 6, 2014

Thanks for the amazing work on these updates, @jwngr! Great stuff.

@katowulf katowulf deleted the jw-key-polyfill branch February 15, 2015 19:21
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants