flutterfire
flutterfire copied to clipboard
🐛 [firestore] BatchWrite.update does not implement type-safe read & write operations.
Bug report
WriteBatch.update does not implement withConverter Type handling.
This seems like a bug to me since WriteBatch.set does implement withConverter Type handling, and it appears that implementing the same in the update method has been overlooked.
Some may say this should be a Feature Request rather than a Bug Report. If that turns out to be the general consensus I'll change it.
Steps to reproduce
Steps to reproduce the behavior:
- Use a Flutter app configured for Firestore access.
- Define
withConverterToFirestore & FromFirestore functions to provide type-safe read & write operations for a data class. - Attempt to perform
BatchWrite.setandBatchWrite.updateusingwithConverterand supplying the relevant data Object.
Result
BatchWrite.set has no issues.
BatchWrite.update results in error "The argument type '[Object]' can't be assigned to the parameter type 'Map<String, dynamic>'."
Expected behavior
Both BatchWrite.set and BatchWrite.update using withConverter should be allowed.
Additional context
Current implementation for BatchWrite.set:
void set<T>(
DocumentReference<T> document,
T data, [
SetOptions? options,
]) {
assert(
document.firestore == _firestore,
'the document provided is from a different Firestore instance',
);
Map<String, dynamic> firestoreData;
if (document is _JsonDocumentReference) {
firestoreData = data as Map<String, dynamic>;
} else {
final withConverterDoc = document as _WithConverterDocumentReference<T>;
firestoreData = withConverterDoc._toFirestore(data, options);
}
return _delegate.set(
document.path,
_CodecUtility.replaceValueWithDelegatesInMap(firestoreData)!,
options,
);
}
Current implementation for BatchWrite.update:
void update(DocumentReference document, Map<String, dynamic> data) {
assert(
document.firestore == _firestore,
'the document provided is from a different Firestore instance',
);
return _delegate.update(
document.path,
_CodecUtility.replaceValueWithDelegatesInMap(data)!,
);
}
Thanks for the report. Treating this as an enhancement to support set and update to support withConverter.
/cc @Lyokone