Skip to content

Make TransformResult non-nullable #4538

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

Merged
merged 4 commits into from
Feb 26, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 18 additions & 22 deletions packages/firestore/src/model/mutation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -91,9 +91,9 @@ export class MutationResult {
* containing field transforms has been committed. Contains one FieldValue
* for each FieldTransform that was in the mutation.
*
* Will be null if the mutation did not contain any field transforms.
* Will be empty if the mutation did not contain any field transforms.
*/
readonly transformResults: Array<ProtoValue | null> | null
readonly transformResults: Array<ProtoValue | null>
) {}
}

Expand Down Expand Up @@ -434,18 +434,16 @@ function applySetMutationToRemoteDocument(
// remote document the server has accepted the mutation so the precondition
// must have held.
let newData = mutation.value;
if (mutationResult.transformResults) {
const transformResults = serverTransformResults(
mutation.fieldTransforms,
maybeDoc,
mutationResult.transformResults
);
newData = transformObject(
mutation.fieldTransforms,
newData,
transformResults
);
}
const transformResults = serverTransformResults(
mutation.fieldTransforms,
maybeDoc,
mutationResult.transformResults
);
newData = transformObject(
mutation.fieldTransforms,
newData,
transformResults
);

return new Document(mutation.key, mutationResult.version, newData, {
hasCommittedMutations: true
Expand Down Expand Up @@ -519,13 +517,11 @@ function applyPatchMutationToRemoteDocument(
return new UnknownDocument(mutation.key, mutationResult.version);
}

const transformResults = mutationResult.transformResults
? serverTransformResults(
mutation.fieldTransforms,
maybeDoc,
mutationResult.transformResults
)
: [];
const transformResults = serverTransformResults(
mutation.fieldTransforms,
maybeDoc,
mutationResult.transformResults
);
const newData = patchDocument(mutation, maybeDoc, transformResults);
return new Document(mutation.key, mutationResult.version, newData, {
hasCommittedMutations: true
Expand Down Expand Up @@ -701,7 +697,7 @@ function applyDeleteMutationToRemoteDocument(
mutationResult: MutationResult
): NoDocument {
debugAssert(
mutationResult.transformResults == null,
mutationResult.transformResults.length === 0,
'Transform results received by DeleteMutation.'
);

Expand Down
7 changes: 1 addition & 6 deletions packages/firestore/src/remote/serializer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,6 @@ import {
Target as ProtoTarget,
TargetChangeTargetChangeType as ProtoTargetChangeTargetChangeType,
Timestamp as ProtoTimestamp,
Value as ProtoValue,
Write as ProtoWrite,
WriteResult as ProtoWriteResult
} from '../protos/firestore_proto_api';
Expand Down Expand Up @@ -698,11 +697,7 @@ function fromWriteResult(
version = fromVersion(commitTime);
}

let transformResults: ProtoValue[] | null = null;
if (proto.transformResults && proto.transformResults.length > 0) {
transformResults = proto.transformResults;
}
return new MutationResult(version, transformResults);
return new MutationResult(version, proto.transformResults || []);
}

export function fromWriteResults(
Expand Down
2 changes: 1 addition & 1 deletion packages/firestore/test/unit/local/local_store.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,7 @@ class LocalStoreTester {
const mutationResults = [
new MutationResult(
ver,
options.transformResult ? [options.transformResult] : null
options.transformResult ? [options.transformResult] : []
)
];
const write = MutationBatchResult.from(batch, ver, mutationResults);
Expand Down
2 changes: 1 addition & 1 deletion packages/firestore/test/unit/model/mutation.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -582,7 +582,7 @@ describe('Mutation', () => {

const mutationResult = new MutationResult(
version(7),
/*transformResults=*/ null
/*transformResults=*/ []
);
const docV7Unknown = unknownDoc('collection/key', 7);
const docV7Deleted = deletedDoc('collection/key', 7, {
Expand Down
2 changes: 1 addition & 1 deletion packages/firestore/test/util/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,7 @@ export function deleteMutation(keyStr: string): DeleteMutation {
export function mutationResult(
testVersion: TestSnapshotVersion
): MutationResult {
return new MutationResult(version(testVersion), /* transformResults= */ null);
return new MutationResult(version(testVersion), /* transformResults= */ []);
}

export function bound(
Expand Down