fix: treat nested enums as enums and not messages (#87)

The `defineModule` function wasn't marking nested enums as such so
update it to recursively look at nested subtypes when marking types
as optional, messages and/or enums.

packages/protons/src/index.ts

@@ -420,11 +420,11 @@ export interface ${messageDef.name} {
           ${encoderGenerators[type] == null ? `${codec}.encode(${valueVar}${includeDefault ? ` ?? ${typeName}.${defaultValue}` : ''}, w)` : encoderGenerators[type](valueVar, includeDefault)}`
 
         if (type === 'message') {
-          // message fields are only written if they have values
+          // message fields are only written if they have values. But if a message
+          // is part of a repeated field, and consists of only default values it
+          // won't be written, so write a zero-length buffer if that's the case
           writeField = () => `w.uint32(${id})
-          ${typeName}.codec().encode(${valueVar}, w, {
-            writeDefaults: ${Boolean(fieldDef.repeated).toString()}
-          })`
+          ${typeName}.codec().encode(${valueVar}, w)`
         }
 
         return writeField
@@ -620,30 +620,39 @@ function defineModule (def: ClassDef): ModuleDef {
     }
   }
 
-  defineMessage(defs)
+  function updateTypes (defs: Record<string, ClassDef>, parent?: ClassDef): void {
+    for (const className of Object.keys(defs)) {
+      const classDef = defs[className]
 
-  // set enum/message fields now all messages have been defined
-  for (const className of Object.keys(defs)) {
-    const classDef = defs[className]
+      if (classDef.nested != null) {
+        updateTypes(classDef.nested, classDef)
+      }
+
+      if (classDef.fields != null) {
+        for (const name of Object.keys(classDef.fields)) {
+          const fieldDef = classDef.fields[name]
+          if (types[fieldDef.type] == null) {
+            const def = findDef(fieldDef.type, classDef, moduleDef)
 
-    if (classDef.fields != null) {
-      for (const name of Object.keys(classDef.fields)) {
-        const fieldDef = classDef.fields[name]
-        if (types[fieldDef.type] == null) {
-          const def = findDef(fieldDef.type, classDef, moduleDef)
-          fieldDef.enum = isEnumDef(def)
-          fieldDef.message = !fieldDef.enum
-
-          if (fieldDef.message && !fieldDef.repeated) {
-            // the default type for a message is unset so they are always optional
-            // https://developers.google.com/protocol-buffers/docs/proto3#default
-            fieldDef.optional = true
+            fieldDef.enum = isEnumDef(def)
+            fieldDef.message = !fieldDef.enum
+
+            if (fieldDef.message && !fieldDef.repeated) {
+              // the default type for a message is unset so they are always optional
+              // https://developers.google.com/protocol-buffers/docs/proto3#default
+              fieldDef.optional = true
+            }
           }
         }
       }
     }
   }
 
+  defineMessage(defs)
+
+  // set enum/message fields now all messages have been defined
+  updateTypes(defs)
+
   for (const className of Object.keys(defs)) {
     const classDef = defs[className]
 

packages/protons/test/index.spec.ts → packages/protons/test/compatibility.spec.ts

@@ -14,6 +14,7 @@ import { Optional, OptionalEnum } from './fixtures/optional.js'
 import { Peer } from './fixtures/peer.js'
 import { Singular, SingularEnum } from './fixtures/singular.js'
 import { AllTheTypes, AnEnum } from './fixtures/test.js'
+import { Message as Bitswap } from './fixtures/bitswap.js'
 
 function longifyBigInts (obj: any): any {
   const output = {
@@ -258,7 +259,18 @@ describe('encode', () => {
       }]
     }
 
-    testEncodings(obj, Peer, './test/fixtures/peer.proto', 'Peer')
+    const buf = Peer.encode(obj)
+    const out = Peer.decode(buf)
+
+    expect(obj).to.deep.equal(out)
+
+    testEncodings(obj, Peer, './test/fixtures/peer.proto', 'Peer', {
+      // protobuf.js encodes default values when it shouldn't
+      compareBytes: false,
+
+      // pbjs doesn't encode repeated fields properly
+      comparePbjs: false
+    })
   })
 
   it('decodes enums with values that are not 0-n', () => {
@@ -533,4 +545,33 @@ describe('encode', () => {
       comparePbjs: false
     })
   })
+
+  it('should round trip deeply nested messages with defaults', () => {
+    const input = {
+      wantlist: {
+        entries: [{
+          block: Buffer.from([]),
+          priority: 0,
+          cancel: false,
+          wantType: Bitswap.Wantlist.WantType.Block,
+          sendDontHave: false
+        }],
+        full: false
+      },
+      blocks: [],
+      payload: [{
+        prefix: Buffer.from([]),
+        data: Buffer.from([])
+      }],
+      blockPresences: [{
+        cid: Buffer.from([]),
+        type: Bitswap.BlockPresenceType.Have
+      }],
+      pendingBytes: 0
+    }
+    const buf = Bitswap.encode(input)
+    const output = Bitswap.decode(buf)
+
+    expect(output).to.deep.equal(input)
+  })
 })

packages/protons/test/fixtures/bitswap.proto

@@ -0,0 +1,43 @@
+// from https://github.com/ipfs/go-bitswap/blob/master/message/pb/message.proto
+syntax = "proto3";
+
+message Message {
+
+  message Wantlist {
+    enum WantType {
+      Block = 0;
+      Have = 1;
+    }
+
+    message Entry {
+      bytes block = 1;           // the block cid (cidV0 in bitswap 1.0.0, cidV1 in bitswap 1.1.0)
+      int32 priority = 2;        // the priority (normalized). default to 1
+      optional bool cancel = 3;           // whether this revokes an entry
+      WantType wantType = 4;     // Note: defaults to enum 0, ie Block
+      bool sendDontHave = 5;     // Note: defaults to false
+    }
+
+    repeated Entry entries = 1;  // a list of wantlist entries
+    bool full = 2;               // whether this is the full wantlist. default to false
+  }
+
+  message Block {
+    bytes prefix = 1;            // CID prefix (cid version, multicodec and multihash prefix (type + length)
+    bytes data = 2;
+  }
+
+  enum BlockPresenceType {
+    Have = 0;
+    DontHave = 1;
+  }
+  message BlockPresence {
+    bytes cid = 1;
+    BlockPresenceType type = 2;
+  }
+
+  Wantlist wantlist = 1;
+  repeated bytes blocks = 2;     // used to send Blocks in bitswap 1.0.0
+  repeated Block payload = 3;    // used to send Blocks in bitswap 1.1.0
+  repeated BlockPresence blockPresences = 4;
+  int32 pendingBytes = 5;
+}