fix: adhere more closely to the language guide for proto3 default values

Only write singular fields when they are non-default values (unless in
repeated fields).  Always write optional values even when they are non-default
values.

Updates test suite to compare generated bytes to protobuf.js to ensure compatibility.

Also adds a README section on differences between protons and protobuf.js

fixes #43

packages/protons-runtime/src/codec.ts

@@ -12,6 +12,7 @@ export enum CODEC_TYPES {
 
 export interface EncodeOptions {
   lengthDelimited?: boolean
+  writeDefaults?: boolean
 }
 
 export interface EncodeFunction<T> {

packages/protons-runtime/src/codecs/enum.ts

@@ -20,7 +20,7 @@ export function enumeration <T> (v: any): Codec<T> {
   }
 
   const decode: DecodeFunction<number | string> = function enumDecode (reader) {
-    const val = reader.uint32()
+    const val = reader.int32()
 
     return findValue(val)
   }

packages/protons-runtime/src/decode.ts

@@ -1,25 +1,9 @@
 import type { Uint8ArrayList } from 'uint8arraylist'
 import type { Codec } from './codec.js'
-import pb from 'protobufjs'
-
-const Reader = pb.Reader
-
-// monkey patch the reader to add native bigint support
-const methods = [
-  'uint64', 'int64', 'sint64', 'fixed64', 'sfixed64'
-]
-methods.forEach(method => {
-  // @ts-expect-error
-  const original = Reader.prototype[method]
-  // @ts-expect-error
-  Reader.prototype[method] = function (): bigint {
-    return BigInt(original.call(this).toString())
-  }
-})
+import { reader } from './reader.js'
 
 export function decodeMessage <T> (buf: Uint8Array | Uint8ArrayList, codec: Codec<T>): T {
-  const reader = Reader.create(buf instanceof Uint8Array ? buf : buf.subarray())
+  const r = reader(buf instanceof Uint8Array ? buf : buf.subarray())
 
-  // @ts-expect-error
-  return codec.decode(reader)
+  return codec.decode(r)
 }

packages/protons-runtime/src/encode.ts

@@ -1,25 +1,9 @@
 import type { Codec } from './codec.js'
-import pb from 'protobufjs'
-
-const Writer = pb.Writer
-
-// monkey patch the writer to add native bigint support
-const methods = [
-  'uint64', 'int64', 'sint64', 'fixed64', 'sfixed64'
-]
-methods.forEach(method => {
-  // @ts-expect-error
-  const original = Writer.prototype[method]
-  // @ts-expect-error
-  Writer.prototype[method] = function (val: bigint): pb.Writer {
-    return original.call(this, val.toString())
-  }
-})
+import { writer } from './writer.js'
 
 export function encodeMessage <T> (message: T, codec: Codec<T>): Uint8Array {
-  const w = Writer.create()
+  const w = writer()
 
-  // @ts-expect-error
   codec.encode(message, w, {
     lengthDelimited: false
   })

packages/protons-runtime/src/index.ts

@@ -18,6 +18,8 @@ export {
 
 export { enumeration } from './codecs/enum.js'
 export { message } from './codecs/message.js'
+export { reader } from './reader.js'
+export { writer } from './writer.js'
 export type { Codec, EncodeOptions } from './codec.js'
 
 export interface Writer {

packages/protons-runtime/src/reader.ts

@@ -0,0 +1,22 @@
+import pb from 'protobufjs/minimal.js'
+import type { Reader } from './index.js'
+
+const PBReader = pb.Reader
+
+// monkey patch the reader to add native bigint support
+const methods = [
+  'uint64', 'int64', 'sint64', 'fixed64', 'sfixed64'
+]
+methods.forEach(method => {
+  // @ts-expect-error
+  const original = PBReader.prototype[method]
+  // @ts-expect-error
+  PBReader.prototype[method] = function (): bigint {
+    return BigInt(original.call(this).toString())
+  }
+})
+
+export function reader (buf: Uint8Array): Reader {
+  // @ts-expect-error class is monkey patched
+  return PBReader.create(buf)
+}

packages/protons-runtime/src/writer.ts

@@ -0,0 +1,22 @@
+import pb from 'protobufjs/minimal.js'
+import type { Writer } from './index.js'
+
+const PBWriter = pb.Writer
+
+// monkey patch the writer to add native bigint support
+const methods = [
+  'uint64', 'int64', 'sint64', 'fixed64', 'sfixed64'
+]
+methods.forEach(method => {
+  // @ts-expect-error
+  const original = PBWriter.prototype[method]
+  // @ts-expect-error
+  PBWriter.prototype[method] = function (val: bigint): pb.Writer {
+    return original.call(this, val.toString())
+  }
+})
+
+export function writer (): Writer {
+  // @ts-expect-error class is monkey patched
+  return PBWriter.create()
+}

packages/protons/README.md

@@ -12,6 +12,7 @@
 
 - [Install](#install)
 - [Usage](#usage)
+- [Differences from protobuf.js](#differences-from-protobufjs)
 - [Contribute](#contribute)
 - [License](#license)
 - [Contribute](#contribute-1)
@@ -59,6 +60,21 @@ console.info(decoded.message)
 // 'hello world'
 ```
 
+## Differences from protobuf.js
+
+This module uses the internal reader/writer from `protobuf.js` as it is highly optimised and there's no point reinventing the wheel.
+
+It does have one or two differences:
+
+1. Supports `proto3` semantics only
+2. All 64 bit values are represented as `BigInt`s and not `Long`s (e.g. `int64`, `uint64`, `sint64` etc)
+3. Unset `optional` fields are set on the deserialized object forms as `undefined` instead of the default values
+4. `singular` fields set to default values are not serialized and are set to default values when deserialized if not set - protobuf.js [diverges from the language guide](https://github.com/protobufjs/protobuf.js/issues/1468#issuecomment-745177012) around this feature
+
+## Missing features
+
+Some features are missing `OneOf`, `Map`s, etc due to them not being needed so far in ipfs/libp2p. If these features are important to you, please open PRs implementing them along with tests comparing the generated bytes to `protobuf.js` and `pbjs`.
+
 ## Contribute
 
 Feel free to join in. All welcome. Open an [issue](https://github.com/ipfs/protons/issues)!

packages/protons/src/index.ts

@@ -36,21 +36,21 @@ const types: Record<string, string> = {
 }
 
 const encoderGenerators: Record<string, (val: string) => string> = {
-  bool: (val) => `writer.bool(${val})`,
-  bytes: (val) => `writer.bytes(${val})`,
-  double: (val) => `writer.double(${val})`,
-  fixed32: (val) => `writer.fixed32(${val})`,
-  fixed64: (val) => `writer.fixed64(${val})`,
-  float: (val) => `writer.float(${val})`,
-  int32: (val) => `writer.int32(${val})`,
-  int64: (val) => `writer.int64(${val})`,
-  sfixed32: (val) => `writer.sfixed32(${val})`,
-  sfixed64: (val) => `writer.sfixed64(${val})`,
-  sint32: (val) => `writer.sint32(${val})`,
-  sint64: (val) => `writer.sint64(${val})`,
-  string: (val) => `writer.string(${val})`,
-  uint32: (val) => `writer.uint32(${val})`,
-  uint64: (val) => `writer.uint64(${val})`
+  bool: (val) => `w.bool(${val})`,
+  bytes: (val) => `w.bytes(${val})`,
+  double: (val) => `w.double(${val})`,
+  fixed32: (val) => `w.fixed32(${val})`,
+  fixed64: (val) => `w.fixed64(${val})`,
+  float: (val) => `w.float(${val})`,
+  int32: (val) => `w.int32(${val})`,
+  int64: (val) => `w.int64(${val})`,
+  sfixed32: (val) => `w.sfixed32(${val})`,
+  sfixed64: (val) => `w.sfixed64(${val})`,
+  sint32: (val) => `w.sint32(${val})`,
+  sint64: (val) => `w.sint64(${val})`,
+  string: (val) => `w.string(${val})`,
+  uint32: (val) => `w.uint32(${val})`,
+  uint64: (val) => `w.uint64(${val})`
 }
 
 const decoderGenerators: Record<string, () => string> = {
@@ -89,6 +89,24 @@ const defaultValueGenerators: Record<string, () => string> = {
   uint64: () => '0n'
 }
 
+const defaultValueTestGenerators: Record<string, (field: string) => string> = {
+  bool: (field) => `${field} !== false`,
+  bytes: (field) => `(${field} != null && ${field}.byteLength > 0)`,
+  double: (field) => `${field} !== 0`,
+  fixed32: (field) => `${field} !== 0`,
+  fixed64: (field) => `${field} !== 0n`,
+  float: (field) => `${field} !== 0`,
+  int32: (field) => `${field} !== 0`,
+  int64: (field) => `${field} !== 0n`,
+  sfixed32: (field) => `${field} !== 0`,
+  sfixed64: (field) => `${field} !== 0n`,
+  sint32: (field) => `${field} !== 0`,
+  sint64: (field) => `${field} !== 0n`,
+  string: (field) => `${field} !== ''`,
+  uint32: (field) => `${field} !== 0`,
+  uint64: (field) => `${field} !== 0n`
+}
+
 function findTypeName (typeName: string, classDef: MessageDef, moduleDef: ModuleDef): string {
   if (types[typeName] != null) {
     return types[typeName]
@@ -341,32 +359,20 @@ export interface ${messageDef.name} {
         return ''
       }).filter(Boolean).join('\n')
 
-    const ensureRequiredFields = Object.entries(fields)
-      .map(([name, fieldDef]) => {
-        // make sure required fields are set
-        if (!fieldDef.optional && !fieldDef.repeated) {
-          return `
-        if (obj.${name} == null) {
-          throw new Error('Protocol error: value for required field "${name}" was not found in protobuf')
-        }`
-        }
-
-        return ''
-      }).filter(Boolean).join('\n')
-
     interfaceCodecDef = `
   let _codec: Codec<${messageDef.name}>
 
   export const codec = (): Codec<${messageDef.name}> => {
     if (_codec == null) {
-      _codec = message<${messageDef.name}>((obj, writer, opts = {}) => {
+      _codec = message<${messageDef.name}>((obj, w, opts = {}) => {
         if (opts.lengthDelimited !== false) {
-          writer.fork()
+          w.fork()
         }
 ${Object.entries(fields)
       .map(([name, fieldDef]) => {
         let codec: string = encoders[fieldDef.type]
         let type: string = fieldDef.type
+        let typeName: string = ''
 
         if (codec == null) {
           const def = findDef(fieldDef.type, messageDef, moduleDef)
@@ -379,31 +385,84 @@ ${Object.entries(fields)
             type = 'message'
           }
 
-          const typeName = findTypeName(fieldDef.type, messageDef, moduleDef)
+          typeName = findTypeName(fieldDef.type, messageDef, moduleDef)
           codec = `${typeName}.codec()`
         }
 
-        return `
-        if (obj.${name} != null) {${
-            fieldDef.rule === 'repeated'
-? `
-          for (const value of obj.${name}) {
-            writer.uint32(${(fieldDef.id << 3) | codecTypes[type]})
-            ${encoderGenerators[type] == null ? `${codec}.encode(value, writer)` : encoderGenerators[type]('value')}
+        let valueTest = `obj.${name} != null`
+
+        // proto3 singular fields should only be written out if they are not the default value
+        if (!fieldDef.optional && !fieldDef.repeated) {
+          if (defaultValueTestGenerators[type] != null) {
+            valueTest = `opts.writeDefaults === true || ${defaultValueTestGenerators[type](`obj.${name}`)}`
+          } else if (type === 'enum') {
+            // handle enums
+            valueTest = `opts.writeDefaults === true || (obj.${name} != null && __${fieldDef.type}Values[obj.${name}] !== 0)`
+          }
+        }
+
+        function createWriteField (valueVar: string) {
+          const id = (fieldDef.id << 3) | codecTypes[type]
+
+          let writeField = `w.uint32(${id})
+          ${encoderGenerators[type] == null ? `${codec}.encode(${valueVar}, w)` : encoderGenerators[type](valueVar)}`
+
+          if (type === 'message') {
+            // message fields are only written if they have values
+            moduleDef.imports.add('writer')
+
+            writeField = `const mw = writer()
+          ${typeName}.codec().encode(${valueVar}, mw, {
+            lengthDelimited: false,
+            writeDefaults: ${Boolean(fieldDef.repeated).toString()}
+          })
+          const buf = mw.finish()`
+
+            if (fieldDef.repeated) {
+              writeField += `
+
+          w.uint32(${id})
+          w.bytes(buf)`
+            } else {
+              writeField += `
+
+          if (buf.byteLength > 0) {
+            w.uint32(${id})
+            w.bytes(buf)
           }`
-: `
-          writer.uint32(${(fieldDef.id << 3) | codecTypes[type]})
-          ${encoderGenerators[type] == null ? `${codec}.encode(obj.${name}, writer)` : encoderGenerators[type](`obj.${name}`)}`
+            }
+          }
+
+          return writeField
         }
-        }${fieldDef.optional
-? ''
-: ` else {
-          throw new Error('Protocol error: required field "${name}" was not found in object')
-        }`}`
+
+        let writeField = createWriteField(`obj.${name}`)
+
+        if (fieldDef.repeated) {
+          writeField = `
+          for (const value of obj.${name}) {
+          ${
+              createWriteField('value')
+                .split('\n')
+                .map(s => {
+                  const trimmed = s.trim()
+
+                  return trimmed === '' ? trimmed : `  ${s}`
+                })
+                .join('\n')
+            }
+          }
+          `.trim()
+        }
+
+        return `
+        if (${valueTest}) {
+          ${writeField}
+        }`
 }).join('\n')}
 
         if (opts.lengthDelimited !== false) {
-          writer.ldelim()
+          w.ldelim()
         }
       }, (reader, length) => {
         const obj: any = {${createDefaultObject(fields, messageDef, moduleDef)}}
@@ -448,7 +507,7 @@ ${Object.entries(fields)
               reader.skipType(tag & 7)
               break
           }
-        }${ensureArrayProps !== '' ? `\n\n${ensureArrayProps}` : ''}${ensureRequiredFields !== '' ? `\n${ensureRequiredFields}` : ''}
+        }${ensureArrayProps !== '' ? `\n\n${ensureArrayProps}` : ''}
 
         return obj
       })
@@ -551,7 +610,9 @@ export async function generate (source: string, flags: Flags) {
 
   let lines = [
     '/* eslint-disable import/export */',
+    '/* eslint-disable complexity */',
     '/* eslint-disable @typescript-eslint/no-namespace */',
+    '/* eslint-disable @typescript-eslint/no-unnecessary-boolean-literal-compare */',
     ''
   ]
 
@@ -574,6 +635,7 @@ export async function generate (source: string, flags: Flags) {
   ]
 
   const content = lines.join('\n').trim()
+  const outputPath = pathWithExtension(source, '.ts', flags.output)
 
-  await fs.writeFile(pathWithExtension(source, '.ts', flags.output), content + '\n')
+  await fs.writeFile(outputPath, content + '\n')
 }