Treat null values the same as undefined when generating protobuf mess…

…ages (#299)
timostamm · May 16, 2022 · 44b0fe4 · 44b0fe4
1 parent c219f16
commit 44b0fe4
Show file tree

Hide file tree

Showing 2 changed files with 27 additions and 3 deletions.
diff --git a/packages/runtime/spec/reflection-merge-partial.spec.ts b/packages/runtime/spec/reflection-merge-partial.spec.ts
@@ -51,6 +51,16 @@ describe('reflectionMergePartial()', () => {
             expect(target[string_field.localName]).toBe("");
         });
 
+        it('null value does not overwrite existing value', () => {
+            const target = reflectionCreate(messageInfo);
+            target[string_field.localName] = "hello";
+            const source = {
+                [string_field.localName]: null as unknown as string
+            };
+            reflectionMergePartial(messageInfo, target, source);
+            expect(target[string_field.localName]).toBe("hello");
+        });
+
         it('omitted value does not overwrite existing value', () => {
             const target = reflectionCreate(messageInfo);
             target[string_field.localName] = "hello";
@@ -102,6 +112,20 @@ describe('reflectionMergePartial()', () => {
             });
         });
 
+        describe('and source field null', () => {
+            const source: object = {child: null};
+            it('does not touch target', () => {
+                const target: any = {child: 123};
+                reflectionMergePartial(messageInfo, target, source);
+                expect(target.child).toBe(123);
+            });
+            it('does not call child handler', () => {
+                reflectionMergePartial(messageInfo, {}, source);
+                expect(childHandler.create).not.toHaveBeenCalled();
+                expect(childHandler.mergePartial).not.toHaveBeenCalled();
+            });
+        });
+
         describe('and target field empty', () => {
             it('calls child handler´s create()', () => {
                 const source = {child: {other_msg_fake_field: true}};

diff --git a/packages/runtime/src/reflection-merge-partial.ts b/packages/runtime/src/reflection-merge-partial.ts
@@ -29,20 +29,20 @@ export function reflectionMergePartial<T extends object>(info: MessageInfo, targ
 
         if (field.oneof) {
             const group = input[field.oneof] as UnknownOneofGroup | undefined; // this is the oneof`s group in the source
-            if (group === undefined) { // the user is free to omit
+            if (group == undefined) { // the user is free to omit
                 continue; // we skip this field, and all other members too
             }
             fieldValue = group[name]; // our value comes from the the oneof group of the source
             output = (target as UnknownMessage)[field.oneof] as UnknownOneofGroup; // and our output is the oneof group of the target
             output.oneofKind = group.oneofKind; // always update discriminator
-            if (fieldValue === undefined) {
+            if (fieldValue == undefined) {
                 delete output[name]; // remove any existing value
                 continue; // skip further work on field
             }
         } else {
             fieldValue = input[name]; // we are using the source directly
             output = target as UnknownMessage; // we want our field value to go directly into the target
-            if (fieldValue === undefined) {
+            if (fieldValue == undefined) {
                 continue; // skip further work on field, existing value is used as is
             }
         }