Custom Converter does not work when using update method

[inaridiy]

Environment details

• OS: Windows 10 Pro 21H2
• Node.js version: v16.17.0
• npm version: 8.15.0
• @google-cloud/firestore version:6.3.0

Steps to reproduce

1. execute DocumentReference.update method with custom converter set.

Demonstration

// user is instanceOf User Class
await db
      .collection("user")
      .doc(user.address.value)
      .withConverter(UserConverter({ includeCreatedAt: false }))
      .update(user);
// Update() requires either a single JavaScript object or an alternating list of field/value pairs that can be followed by an optional precondition. Value for argument "dataOrField" is not a valid Firestore document. Couldn't serialize object of type "User". Firestore doesn't support JavaScript objects with custom prototypes (i.e. objects that were created via the "new" operator).

Since User is incompatible with FieldValue, it must be converted using UserConverter, but it seems that the update method does not use Converter and evaluates it directly.

I have used create and set methods in other places, but have not seen the same problem.

Survey

Within the WriteBatch Class The ref._converter was called inside the create and set methods, but not inside the update method.

Where Converter is called

nodejs-firestore/dev/src/write-batch.ts

Lines 197 to 199 in 80cfca9

	const firestoreData = ref._converter.toFirestore(
	data as firestore.WithFieldValue<T>
	);

nodejs-firestore/dev/src/write-batch.ts

Lines 332 to 340 in 80cfca9

	let firestoreData: firestore.DocumentData;
	if (mergeLeaves || mergePaths) {
	// Cast to any in order to satisfy the union type constraint on
	// toFirestore().
	// eslint-disable-next-line @typescript-eslint/no-explicit-any
	firestoreData = (ref._converter as any).toFirestore(data, options);
	} else {
	firestoreData = ref._converter.toFirestore(data as T);
	}

Converter is not called within Update

nodejs-firestore/dev/src/write-batch.ts

Lines 425 to 434 in 80cfca9

	update<T = firestore.DocumentData>(
	documentRef: firestore.DocumentReference<T>,
	dataOrField: firestore.UpdateData<T> | string | firestore.FieldPath,
	...preconditionOrValues: Array<
	| {lastUpdateTime?: firestore.Timestamp}
	| unknown
	| string
	| firestore.FieldPath
	>
	): WriteBatch {

[MarkDuckworth]

Thanks for bringing this to our attention. I'll have to look at this a little more, but my first thought is that update is expecting a different type of object UpdateData<T>, which is an object with all of the properties of T and allows you to set properties using dot notation. So, this might be expected.

[MarkDuckworth]

Relates to #1745. You might be interested in reviewing that thread for a different perspective on this. We're also looking at this internally.

[dconeybe]

Closing as a duplicate of #1745.