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

Implement FieldValue.increment() #92

Merged
merged 3 commits into from
Apr 7, 2020

Conversation

kimroen
Copy link
Contributor

@kimroen kimroen commented Mar 28, 2020

Implements FieldValue.increment() in order to resolve #91

Hopefully it's useful as a reference if nothing else!

src/unit-test.js Outdated
@@ -604,6 +604,7 @@ QUnit.module('Unit | mock-cloud-firestore', (hooks) => {
name: firebase.firestore.FieldValue.delete(),
dad: db.collection('users').doc('user_b'),
modifiedOn: firebase.firestore.FieldValue.serverTimestamp(),
activeYears: mockFirebase.firestore.FieldValue.increment(-1),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm referencing mockFirebase here because I was not able to update the firebase dependency to a version that contains this without running into build issues that I wasn't able to resolve without too much digging. If we can get that working, that would make this more correct.

Copy link
Owner

Choose a reason for hiding this comment

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

What's the build error you're getting?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It says that core-js is not found for importing. As far as I can tell, this has something to do with polyfills and Babel

Copy link
Owner

@mikkopaderes mikkopaderes Apr 2, 2020

Choose a reason for hiding this comment

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

I've investigated it and it's because of some recent babel changes. I've fixed that in the master branch. Could you try rebasing this PR from that? Once you do, firebase should now work which is what's actually you should use for the tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you, I have now rebased and updated the tests to use firebase.firestore.FieldValue.increment(), and things seem to be working 👍

Copy link
Owner

Choose a reason for hiding this comment

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

Thank you @kimroen

@kimroen kimroen force-pushed the implement-increment branch from 919a4ed to a4301bf Compare April 6, 2020 11:13
@kimroen kimroen requested a review from mikkopaderes April 6, 2020 11:14
@mikkopaderes mikkopaderes merged commit d37c130 into mikkopaderes:master Apr 7, 2020
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 this pull request may close these issues.

Add support for FieldValue.increment()
2 participants