Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Different versions of Firebase in one bundle break the ability to log Timestamp to Firestore #8032

Closed
appsforartists opened this issue Feb 21, 2024 · 5 comments
Assignees
Labels
api: firestore new A new issue that hasn't be categoirzed as question, bug or feature request question

Comments

@appsforartists
Copy link

appsforartists commented Feb 21, 2024

Operating System

MacOS 14.3.1 (23D60)

Browser Version

121.0.6167.184 (Official Build) (arm64)

Firebase SDK Version

10.1.0

Firebase SDK Product:

Firestore

Describe your project's tooling

[email protected]
[email protected] ([email protected], [email protected])

Describe the problem

I'm trying to include a timestamp in a log. It works locally, but I see this error when I deploy:

FirebaseError: Function setDoc() called with invalid data. Unsupported field value: a custom pr object (found in field value.creationTime in document experiments/202402-test/meta/info)

Digging into the bundled JS, I realized that Vite (via esbuild) is mangling my names, and that pr is the value it chose for Timestamp. It doesn't appear that Vite allows you to prevent names from being mangled with esbuild, so I tried changing to terser. terser still mangled the name, but it chose at, so now the error is:

FirebaseError: Function setDoc() called with invalid data. Unsupported field value: a custom at object (found in field value.creationTime in document experiments/202402-test/meta/info)

Steps and code to reproduce issue

import {
  getApp,
} from 'firebase/app';

import {
  doc,
  getFirestore,
  setDoc,
  Timestamp,
} from 'firebase/firestore';

const db = getFirestore(getApp());
setDoc(
  doc(db, 'experiments/202402-test/meta/info'),
  {
    value: {
      creationTime: Timestamp.now(),
    }
  },
);

bundled with vite build

Expected result

Firestore should be resilient to common practices like minification.

I can understand the ambiguity around whether {seconds, nanoseconds} should be treated as a Timestamp or an object literal; however, there are solutions. The most correct one is probably to add a special property like "__firebaseBrand__": "Timestamp" and check it to determine how to store/present a value. Alternatively, you could check value.toString().startsWith('Timestamp'), since that class has a custom implementation of toString.

@appsforartists appsforartists added new A new issue that hasn't be categoirzed as question, bug or feature request question labels Feb 21, 2024
@appsforartists
Copy link
Author

…digging into this further, it seems that there might be two instances of Timestamp in the bundle. Not yet sure why - there's only one copy of the Firebase SDK in my project.

@appsforartists
Copy link
Author

at does indeed appear in the distributed source:

// node_modules/@firebase/firestore/dist/index.esm2017.js
class at {
    /**
     * Creates a new timestamp.

@dconeybe dconeybe self-assigned this Feb 21, 2024
@appsforartists
Copy link
Author

It looks like an unrelated package had a dependency on an old version of firebase (9.6.1).

Not sure why different parts of my project were resolving different versions of Firebase (and it's already later than I wanted to be working on this tonight), but changing the package.json in the unrelated dependency does seem to fix the issue.

@appsforartists
Copy link
Author

Alright, I think this is what happened:

  • Most of my Firebase logging is handled by one package in the project (firebreather), which had a correct dependency on 10.1.0.
  • Some old demos that are not part of this build had an outdated dependency on 9.6.1.
  • The app that was otherwise using firebreather for logging needed to import Timestamp directly from firebase/firestore, but forgot to add an explicit dependency on firebase/firestore to its package.json.
  • For some reason, vite serve was choosing the correct version (10.1.0) for both callsites, but vite build was choosing 10.1.0 for firebreather and 9.6.1 for the app.
  • Either adding the explicit dependency to the app's package.json or upgrading the unrelated demo package to use the same Firebase version as the rest of the monorepo fixed the issue.

@dconeybe
Copy link
Contributor

Oh that's great. Thanks for getting back to us with the root problem and the fix! I'm glad you got it figured out.

@appsforartists appsforartists changed the title Firestore rejects Timestamp if class name is mangled. Different versions of Firebase in one bundle break the ability to log Timestamp to Firestore Feb 21, 2024
@firebase firebase locked and limited conversation to collaborators Mar 23, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api: firestore new A new issue that hasn't be categoirzed as question, bug or feature request question
Projects
None yet
Development

No branches or pull requests

3 participants