firebase / geofire-js

GeoFire for JavaScript - Realtime location queries with Firebase
MIT License
1.44k stars 345 forks source link

Update firebase dependency #213

Closed samtstern closed 3 years ago

samtstern commented 3 years ago

Updates Firebase to the latest version. I don't think geofire has a history of doing breaking changes when we adopt a breaking underlying Firebase SDK but I am open to it.

ziyaddin commented 3 years ago

Reference type incompatibility between Firebase and GeoFire has been around for a long time.

samtstern commented 3 years ago

Reference type incompatibility between Firebase and GeoFire has been around for a long time.

Interesting what do you do to get around it?

coveralls commented 3 years ago

Coverage Status

Coverage increased (+0.5%) to 92.718% when pulling dad270063273c8de91b308b5c6fb0603e52667a0 on ss-update-firebase into 2b9bceb62f84851094122312572c48ef906d9326 on master.

ziyaddin commented 3 years ago

Interesting what do you do to get around it?

I have come up with this workaround:

import { database } from 'firebase-admin';  // or 'firebase' package
import { GeoFire } from 'geofire';
import DatabaseTypes from '@firebase/database-types';

const firebaseRef = database().ref();
const geofire = new GeoFire((firebaseRef as unknown) as DatabaseTypes.Reference)
samtstern commented 3 years ago

@ziyaddin thanks for sharing, that sounds like what I would do as well. The good news is that after this PR you should no longer have that conflict on the latest SDK.

ziyaddin commented 3 years ago

Great, thanks for fixing it.

samtstern commented 3 years ago

@puf please make sure to squash and merge in the future, all of my commits for this PR hit master and there was a lot of guess and check.