Add experimental options to equals() to include extensions and others #1029
Conversation
| return (msg.getUnknown() ?? []) | ||
| .map((uf) => registry.getExtensionFor(msg.desc, uf.no)) | ||
| .filter((e) => !!e) | ||
| .filter((e, index, arr) => arr.indexOf(e) === index); |
There was a problem hiding this comment.
This expression seems like a tautology. What is this trying to filter out? Is this just removing duplicates (where arr.indexOf(e) might return an earlier index)?
There was a problem hiding this comment.
Yes, it's removing duplicates. This expression is widely used, for better or worse.
| const mapA = a.get(f); | ||
| const mapB = b.get(f); | ||
| const keys: unknown[] = []; | ||
| for (const k of mapA.keys()) { | ||
| if (!mapB.has(k)) { | ||
| return false; | ||
| } | ||
| keysA.push(k); | ||
| keys.push(k); | ||
| } | ||
| for (const k of mb.keys()) { | ||
| if (!ma.has(k)) { | ||
| for (const k of mapB.keys()) { | ||
| if (!mapA.has(k)) { | ||
| return false; | ||
| } | ||
| } |
There was a problem hiding this comment.
You have a more efficient/concise way to test this below, for extensions: instead of scanning through both sets of keys, check that they both have the same number of keys and then just scan a, verifying that each key is also present in b.
There was a problem hiding this comment.
This is a facade to access an object like a map type. I think the logic is trying to avoid some expensive operations. I've added a comment.
timostamm
deleted the
tstamm/Add-experimental-options-to-equals()-to-include-extensions-and-others
branch
The function
equalsfrom@bufbuild/protobufcompares two messages of the same type.This PR adds an options argument to enable comparison of extensions, unknown fields, and semantic comparison of
google.protobuf.Any:For now, the feature is an experimental API, and it's marked
@private. This means it's possible to use the API, but at your own risk: It is possible that we change the API, or remove it again.