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

FakeFirestore doesn't declare FieldValue #23

Closed
AverageHelper opened this issue May 5, 2020 · 7 comments · Fixed by #24
Closed

FakeFirestore doesn't declare FieldValue #23

AverageHelper opened this issue May 5, 2020 · 7 comments · Fixed by #24

Comments

@AverageHelper
Copy link
Contributor

Description

I'm not quite sure what class is meant to mock over firebase.firestore (is that FakeFirestore?), but whatever it is doesn't seem to declare FieldValue.

Steps to reproduce

  1. Install and configure firestore-jest-mock and Jest.
npm install --save-dev jest firestore-jest-mock@latest
  1. Have source code that uses firebase.firestore.FieldValue. In my case, I have a file that exports an object with a few Firebase helper types:
module.exports = {
  firestore: firebase.firestore(),
  Timestamp: firebase.firestore.Timestamp,
  TIMESTAMP: firebase.firestore.FieldValue.serverTimestamp(),
  increment: firebase.firestore.FieldValue.increment,
  arrayUnion: firebase.firestore.FieldValue.arrayUnion,
  arrayRemove: firebase.firestore.FieldValue.arrayRemove
};
  1. Call mockFirebase in tests, before importing source files under test.
const { mockFirebase } = require("firestore-jest-mock");
mockFirebase({
  database: {
    ...
  }
});
  1. Run jest

Expected result

My tests should run, and fail for different reasons (failed assertions, etc).

Actual result

No tests ran, and this error appeared in the console:

TypeError: Cannot read property 'serverTimestamp' of undefined

   7 |   firestore: firebase.firestore(),
   8 |   Timestamp: firebase.firestore.Timestamp,
>  9 |   TIMESTAMP: firebase.firestore.FieldValue.serverTimestamp(),
     |                                            ^
  10 |   increment: firebase.firestore.FieldValue.increment,
  11 |   arrayUnion: firebase.firestore.FieldValue.arrayUnion,
  12 |   arrayRemove: firebase.firestore.FieldValue.arrayRemove,

Environment

  • Node version: 8.17.0
  • NPM version: 6.14.4
  • Jest version: 25.5.3
  • firebase-admin version: 8.11.0
@AverageHelper
Copy link
Contributor Author

AverageHelper commented May 5, 2020

I've tried importing "firebase-admin" before "firestore-jest-mock" and calling mockFirebase, and set FieldValue on the new firestore instance, but that doesn't work at all the way I'd hoped it would 😅

@AverageHelper
Copy link
Contributor Author

To note how I use FieldValue, I use its arrayUnion, arrayRemove, increment, delete, and serverTimestamp functions. As for testing, I'm okay just checking the mock that they were called, since my code doesn't (or I suppose shouldn't) rely on or confirm updates made to the database in the same function.

@AverageHelper
Copy link
Contributor Author

I would love to start a PR and work on this myself, but I cannot seem to figure out how to get my Node package to install local packages. (I mean, it installs just fine and points to the right file, but my tests behave as though no mock was made, whereas when I use your package by name and version, the above behavior occurs to indicate that the mock was successful).

@nicollite
Copy link

I think the error happend because FakeFirestore does't have the following fields implemented: FieldValue, arrayUnion, arrayRemove, increment, delete and FieldValue.serverTimestamp.
So when jest overides firestore this fields don't exists in FakeFirestore so the error is that FieldValue is undefined so it doesn't has serverTimestamp

@AverageHelper
Copy link
Contributor Author

Okay, I've got an implementation that tests well so far. PR soon.

@AverageHelper
Copy link
Contributor Author

A bit of an issue in the firestore stub. If I'm reading it right, the firebase.firestore mock is implemented this way:

firestore() {
  return new FakeFirestore(overrides.database);
},

The canonical firestore.FieldValue is defined statically on the firestore type. I'm having a hard time finding examples online, but I'm not sure that JavaScript functions can have static properties defined on them.

I'm not finding many answers readily online. Would something like the following work?

firestore: class extends FakeFirestore {
  constructor() {
    super(overrides.database);
  }
},

The idea is to expose both the constructor and static methods, so would this work if I have the FieldValue class defined as a static property of FakeFirestore?

@AverageHelper
Copy link
Contributor Author

AverageHelper commented May 6, 2020

Turns out that classes need the new keyword. We cannot use new firestore(), as the canonical syntax does not support this.

Solution: declare the firestore property as a named function, and set a static method inside its body! Like so:

firestore: function firestoreConstructor() {
  firestoreConstructor.FieldValue = FakeFirestore.FieldValue;  // The magic sauce
  return new FakeFirestore(overrides.database);
},

That seems to work in my testing, and Travis CI agrees. 😁

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants