handle ignoreUndefinedProperties in set(merge: true) #4347
Conversation
Previously any field in the document would be propagated into the FieldMask regardless of whether or not it had an undefined value, which led to the client acting as if the user had passed FieldValue.delete(), which wasn't intended. Fixes googleapis/nodejs-firestore#1392 for the JS SDK.
🦋 Changeset detectedLatest commit: f211ccf The changes in this PR will be included in the next version bump. This PR includes changesets to release 8 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Changeset File Check ✅
|
Size Analysis Report |
| @@ -398,6 +398,15 @@ apiDescribe('`undefined` properties', (persistence: boolean) => { | |||
| }); | |||
| }); | |||
|
|
|||
| it('are ignored in set({ merge: true })', () => { | |||
| return withTestDocAndSettings(persistence, settings, async doc => { | |||
| await doc.set({ foo: 'foo', 'bar': 'unchanged' }); | |||
There was a problem hiding this comment.
I am surprised the pre-commit hook didn't drop the quotes around bar.
| // If the input is undefined it can never participate in the fieldMask. With | ||
| // `ignoreUndefinedProperties` set to false, `parseScalarValue` will reject | ||
| // an undefined value. | ||
| if (context.path && input !== undefined) { |
There was a problem hiding this comment.
It might be cleaner to pull the undefined check from parseScalarValue into this function. Something like:
if (lookslikeJson) { ... }
else if (input instanceof FieldValue) { ... }
else if (input == undefined && context.ignoreUndefinedProperties) {
return null;
} else { ... }
Optional.
|
Hi folks, should this work in [email protected]? |
|
It should, but you may need to delete your package lock file before re-installing your dependencies. The fix for |
|
Awesome, works - much appreciated |
Previously any field in the document would be propagated into the
FieldMask regardless of whether or not it had an undefined value, which
led to the client acting as if the user had passed FieldValue.delete(),
which wasn't intended.
Fixes googleapis/nodejs-firestore#1392 for the JS SDK.