Feature request: mergePartial
Hi team, I really love the work you're doing bringing protobufs to the browser!
Feature request:
One use case we have is using a Message<T> as the value of our internal state for some of the UI. This works really nicely because it's typed and can be marshalled/unmarshalled easily. However, we often want to deep merge another PartialMessage<T> into that state. Something like proto3.util.initPartial(), except instead of replacing nested values it merges them left to right.
Other things we have considered
- Using deepMerge from lodash. Works in
v0.2.xso long as you don't have any 64bit ints in your schema. Broken inv0.3.x - Attempted porting initPartial to do a mergePartial, but it was fairly complex. I might give it another go and open a PR.
Hi @ivanvanderbyl. Sorry for the delayed response. Can you provide me a quick example of what you're trying to do? I tried working up a test locally with lodash's merge and it seemed to work fine, but I just may not be understanding your scenario.
Also, related to lodash, what sort of issues were you seeing with it in v0.3.0?
HI @smaye81 — no worries at all. So it seems to work so long as the schema doesn't include any int64s. I'm not sure what it is about that data type but it isn't mergeable.
I've looked over the implementation of initPartial a bit and almost got a mergePartial working. I'd prefer this approach as it doesn't need to do as many property comparison checks as lo-dash merge, which will make it a lot faster.
Hey Ivan 👋
I think you're correct - a mergePartial that works on the schema information should be much faster than a generic one, like lo-dash.
Did you know that protobuf-ts has such a function? It was not implemented here, but I think it should be fairly straight-forward to port.
Here's the source: https://github.com/timostamm/protobuf-ts/blob/3.x/packages/runtime/src/reflection-merge-partial.ts And the tests: https://github.com/timostamm/protobuf-ts/blob/3.x/packages/runtime/spec/reflection-merge-partial.spec.ts
Sorry to dig out a somewhat old topic, but was there any progress on this? I am also looking forward to it!
I checked protobuf-ts and unfortunately it does not perform a deep merge either, at least not for repeated and mapped types.
@ygrandgirard, I'm not sure there's a consensus for how a merge should work.
In JavaScript, the very popular lodash.merge merges array elements, while the equally popular deepmerge appends array elements. If you search for "deepmerge" on npmjs.com, there are dozens of alternatives.
Because of it's binary serialization format, Protobuf has it's own semantics for merging, which appends repeated fields. You can try it out by parsing binary data into a message multiple times (see the fromBinary method on class Message). The Go implementation provides a proto.Merge to apply the same merging logic without going through serialization.
Which behavior do you expect? Are you looking for a lodash merge?
Hi @timostamm,
My mistake, I didn't check what lodash does first. I didn't think there could be several ways to deep-merge arrays.
I would personally expect it to do the same thing as decoding a concatenation of messages does. Since this merges the messages together -and is, I hope, well specified-, I think it could be great to have access to the same behavior without the serialization process.
@ygrandgirard, looks like at lease the two of us have consensus 🙂
I think the the upcoming version 2 can mitigate this problem. Instead of adding a method merge to the class Message, a function merge can do the job. This keeps the door open to also add a function mergeLikeLodash, and neither will increase the bundle size of your web appplication if you don't use it.
The reflection API of v2 makes the implementation quite simple. I gave this a quick stab, see script below. If you put this file in the v2 branch, you can run it with npx tsx packages/protobuf-test/src/merge.ts.
packages/protobuf-test/src/merge.ts
import {clone, create, type DescMessage, mergeFromBinary, type MessageShape, toBinary} from "@bufbuild/protobuf";
import {reflect, type ReflectMessage} from "@bufbuild/protobuf/reflect";
import {UserDesc} from "./gen/ts/extra/example_pb.js";
import assert from "node:assert";
/**
* Merge message `source` into message `target`.
*/
export function merge<Desc extends DescMessage>(
messageDesc: Desc,
target: MessageShape<Desc>,
source: MessageShape<Desc>,
): void {
reflectMerge(reflect(messageDesc, target), reflect(messageDesc, source));
}
function reflectMerge(target: ReflectMessage, source: ReflectMessage) {
for (const f of target.fields) {
if (!source.isSet(f)) {
continue;
}
switch (f.fieldKind) {
case "scalar":
case "enum":
target.set(f, source.get(f));
break;
case "message":
reflectMerge(target.get(f), source.get(f));
break;
case "list":
const list = target.get(f);
for (const e of source.get(f)) {
list.add(e);
}
break;
case "map":
const map = target.get(f);
for (const [k, v] of source.get(f)) {
map.set(k, v);
}
break;
}
}
}
const a = create(UserDesc, {
firstName: "Joe",
active: true,
locations: ["Paris"],
projects: {
"a": "A",
"b": "B",
},
});
const b = create(UserDesc, {
lastName: "Smith",
locations: ["New York", "Tokyo"],
projects: {
"a": "aaa",
"c": "C",
},
});
const expectedResult = clone(UserDesc, a);
mergeFromBinary(UserDesc, expectedResult, toBinary(UserDesc, b));
console.log("result from binary merge:", expectedResult);
const merged = clone(UserDesc, a);
merge(UserDesc, merged, b);
console.log("result from merge:", merged);
assert.deepStrictEqual(merged, expectedResult, "result from merge() is not equal to result from binary merge");
/*
Output:
result from binary merge: {
firstName: 'Joe',
lastName: 'Smith',
active: true,
locations: [ 'Paris', 'New York', 'Tokyo' ],
projects: { a: 'aaa', b: 'B', c: 'C' },
'$typeName': 'docs.User'
}
result from merge: {
firstName: 'Joe',
lastName: 'Smith',
active: true,
locations: [ 'Paris', 'New York', 'Tokyo' ],
projects: { a: 'aaa', b: 'B', c: 'C' },
'$typeName': 'docs.User'
}
*/
If you have suggestions for a better API, let us know.
FWIW Google's Java implementation provides merge options. I had implemented something similar when trying to add support for FieldMasks in protobuf-ts: https://github.com/timostamm/protobuf-ts/pull/576/files#diff-e04b7f4fd332304149f09e1fb00a2c4d2b7635c142703d536d7e97fa0566c106