-
Notifications
You must be signed in to change notification settings - Fork 153
Improved error messages for custom types #162
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
Conversation
Codecov Report
@@ Coverage Diff @@
## master #162 +/- ##
=====================================
Coverage 100% 100%
=====================================
Files 12 12
Lines 1729 1731 +2
=====================================
+ Hits 1729 1731 +2
Continue to review full report at Codecov.
|
src/validate.js
Outdated
default: | ||
return new Error( | ||
"Firestore doesn't support serialization of JavaScript objects " + | ||
"that were constructed using the 'new' operator." |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
|
||
assert.throws(() => { | ||
firestore | ||
.doc('collectionId/documentId') | ||
.set({foo: Firestore.FieldPath.documentId()}); | ||
}, /Cannot use "FieldPath" as a Firestore type./); | ||
}, /Cannot use object of type "FieldPath" as a Firestore value./); |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks!
This addresses #143
This PR changes two of our error messages:
There might be much better ways to express this, and the easiest way to figure out if the changes here make sense is to look at the test cases that show these error messages.
Note that the code tested in
order.js
is not reachable from customer code. The tests exists to allow for 100% coverage (touches linenodejs-firestore/src/order.js
Line 82 in 463fbfc