Make Document.field() nullable #1927
Conversation
| field(path: FieldPath): FieldValue | undefined { | ||
| return this.data.field(path); | ||
| field(path: FieldPath): FieldValue | null { | ||
| return this.data.field(path) || null; |
There was a problem hiding this comment.
It seems like this PR replaces 2 || nulls with 1 || null which is technically better, I guess... except now we have two different field() methods (one on Document and one on ObjectValue) that treat nonexistence differently (one as null, one as undefined) which seems worse.
If we're going to make this change, I think we should probably at least plumb it down into ObjectValue.field() as well...
There was a problem hiding this comment.
I added two more "|| null" in #1928 and made an internal value of type "FieldValue | undefined" instead of "undefined" (https://github.com/firebase/firebase-js-sdk/pull/1928/files#diff-af007bc1ca0cf1509f29df38bed7d308R693) which I think is pretty ugly.
I also changed ObjectValue.
Small cleanup PR to make us do less of
doc.field(path) || null.Note that the Public API still returns
undefinedfor missing fields (to differentiate from null), but the private API doesn't need to since it returns a wrapped FieldValue.