firebase / firebase-js-sdk

Firebase Javascript SDK
https://firebase.google.com/docs/web/setup
Other
4.82k stars 885 forks source link

Firestore no longer return { _lat, _long } for a GeoPoint object inside a document #3089

Closed yzalvov closed 4 years ago

yzalvov commented 4 years ago

Environment

Problem

Firestore no longer return { _lat, _long } params for a GeoPoint object inside a document. Firebase 7.8.2 was fine, but since 7.9.0 firestore returns different versions of those params names instead (depending on the firebase version you have). For 7.14.4 it goes as { Ic: 55.760181, wc: 37.453117 } for example.

I guess it has something to do with a crippled look of firebase.firestore() service object when console.log( firebase.firestore() ) it.

Instead of its usual normal look in the console:

Firestore {_firebaseApp: FirebaseAppImpl, _queue: AsyncQueue, INTERNAL: Object, _databaseId: DatabaseId, _persistenceKey: "[DEFAULT]", …}

It goes crippled like:

t {BT: FirebaseAppImpl, qT: t, INTERNAL: Object, Nc: t, jT: "[DEFAULT]", …}

Relevant Code:

– Fine behaviour with firebase 7.8.2 (GitHub repo)Problem with the same code but firebase 7.14.4 (GitHub repo)

* StackBlitz fails with Error: firebase.firestore is not a function. so GitHub repos instead.

import * as firebase from "firebase/app";
import "firebase/firestore";

firebase.initializeApp({
  apiKey: "AIzaSyBNHCyZ-bpv-WA-HpXTmigJm2aq3z1kaH8",
  authDomain: "jscore-sandbox-141b5.firebaseapp.com",
  databaseURL: "https://jscore-sandbox-141b5.firebaseio.com",
  projectId: "jscore-sandbox-141b5",
  storageBucket: "jscore-sandbox-141b5.appspot.com",
  messagingSenderId: "280127633210",
});

// Reproduce Issue below

const docRef = firebase.firestore().collection("geo_test").doc("geo_doc");

async function runGeoDocTest() {
  try {
    let result = await docRef.get();
    if (result.exists) {
      console.log("Found doc: ", result.data());
      return;
    }
    const location = new firebase.firestore.GeoPoint(55.760181, 37.453117);
    result = await docRef.set({ location });
    console.log("Created doc: ", result);
  } catch (error) {
    console.log(error);
  }
}

runGeoDocTest();

console.log(firebase.firestore());
dconeybe commented 4 years ago

Thank you for reporting this issue, @yzalvov. The description and reproduction steps are fantastic and have allowed me to reproduce the issue just as you described. My first instinct is that our minification configuration changed, causing the _lat and _long fields here of the GeoPoint class to get mangled. I'm starting to investigate this issue and will report back with my findings.

I do have one question though. Is this issue causing a bug in your application? Or is it just making debugging more challenging due to the mangled property names?

yzalvov commented 4 years ago

Hi Denver. I’m glad to help. Well, actually I got this issue today as a critical bug in my React Native application after upgrading the Expo SDK to v37, which upped firebase to 7.9.0. One of my components failed to render markers on a map because of this issue.

I downed the firebase to 7.8.2 and am fine at the moment.

dconeybe commented 4 years ago

Okay, thank you for confirming. Any properties whose names are prefixed with an underscore are not part of the public API and are subject to backwards-incompatible changes. In this case, _lat and _long are private implementation details of the class. The latitude and longitude of a GeoPoint are meant to be accessed via the public latitude and longitude properties, respectively. Are you able to modify your code to use those two public properties instead?

https://firebase.google.com/docs/reference/js/firebase.firestore.GeoPoint#latitude

yzalvov commented 4 years ago

That’s the problem. I can see the latitude and longitude props in the browser environment, as API docs suggest. But they have never been returned to me inReact Native. So I use what’s available.

I guess there’s something to be investigated on the Expo SDK and React Native side. Which is strange to me as no other discrepancies in firestore returns were observed.

dconeybe commented 4 years ago

I agree that there may be something that needs to change in React Native Firebase or the Expo SDK. Using the private _lat and _long properties was never supported and now that they have been renamed it has seems to have broken things. Unfortunately, we will not be undoing this name mangling so the usages of those properties needs to be moved to the public latitude and longitude properties. I am not personally familiar with either of these SDKs so I'm afraid that I cannot be of much help. If you end up logging a ticket against one of the other SDKs, include the link here if you can. For now, I will close this issue since there is no action to be taken by the Firebase SDK team. Feel free to continue the discussion here though if you disagree.