Skip to content

Commit

Permalink
fix: remove writing default values (#88)
Browse files Browse the repository at this point in the history 
With correct singular/optional handling it's not necessary to write
default values.

This was previously implemented to write default values for repeated
message fields where all fields were singular but those messages are
written as zero-length buffers and their fields are set to the
default value during decoding so there's no reason to have their
values in the buffer.
  • Loading branch information
@achingbrain
achingbrain authored Feb 8, 2023
1 parent 198e9a7 commit 078c62f
Show file tree
Hide file tree
Showing 10 changed files with 148 additions and 151 deletions.
51 changes: 24 additions & 27 deletions packages/protons/src/index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,22 +38,22 @@ const types: Record<string, string> = {
uint64: 'bigint'
}

const encoderGenerators: Record<string, (val: string, includeDefault: boolean) => string> = {
bool: (val, includeDefault) => `w.bool(${val}${includeDefault ? ' ?? false' : ''})`,
bytes: (val, includeDefault) => `w.bytes(${val}${includeDefault ? ' ?? new Uint8Array(0)' : ''})`,
double: (val, includeDefault) => `w.double(${val}${includeDefault ? ' ?? 0' : ''})`,
fixed32: (val, includeDefault) => `w.fixed32(${val}${includeDefault ? ' ?? 0' : ''})`,
fixed64: (val, includeDefault) => `w.fixed64(${val}${includeDefault ? ' ?? 0n' : ''})`,
float: (val, includeDefault) => `w.float(${val}${includeDefault ? ' ?? 0' : ''})`,
int32: (val, includeDefault) => `w.int32(${val}${includeDefault ? ' ?? 0' : ''})`,
int64: (val, includeDefault) => `w.int64(${val}${includeDefault ? ' ?? 0n' : ''})`,
sfixed32: (val, includeDefault) => `w.sfixed32(${val}${includeDefault ? ' ?? 0' : ''})`,
sfixed64: (val, includeDefault) => `w.sfixed64(${val}${includeDefault ? ' ?? 0n' : ''})`,
sint32: (val, includeDefault) => `w.sint32(${val}${includeDefault ? ' ?? 0' : ''})`,
sint64: (val, includeDefault) => `w.sint64(${val}${includeDefault ? ' ?? 0n' : ''})`,
string: (val, includeDefault) => `w.string(${val}${includeDefault ? ' ?? \'\'' : ''})`,
uint32: (val, includeDefault) => `w.uint32(${val}${includeDefault ? ' ?? 0' : ''})`,
uint64: (val, includeDefault) => `w.uint64(${val}${includeDefault ? ' ?? 0n' : ''})`
const encoderGenerators: Record<string, (val: string) => string> = {
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> = {
Expand Down Expand Up @@ -395,29 +395,26 @@ export interface ${messageDef.name} {
} else if (!fieldDef.optional && !fieldDef.repeated) {
// proto3 singular fields should only be written out if they are not the default value
if (defaultValueTestGenerators[type] != null) {
valueTest = `opts.writeDefaults === true || ${defaultValueTestGenerators[type](`obj.${name}`)}`
valueTest = `${defaultValueTestGenerators[type](`obj.${name}`)}`
} else if (type === 'enum') {
// handle enums
valueTest = `opts.writeDefaults === true || (obj.${name} != null && __${fieldDef.type}Values[obj.${name}] !== 0)`
valueTest = `obj.${name} != null && __${fieldDef.type}Values[obj.${name}] !== 0`
}
}

function createWriteField (valueVar: string): (includeDefault: boolean) => string {
function createWriteField (valueVar: string): () => string {
const id = (fieldDef.id << 3) | codecTypes[type]
let defaultValue = ''

if (fieldDef.enum) {
const def = findDef(fieldDef.type, messageDef, moduleDef)

if (!isEnumDef(def)) {
throw new Error(`${fieldDef.type} was not enum def`)
}

defaultValue = Object.keys(def.values)[0]
}

let writeField = (includeDefault: boolean): string => `w.uint32(${id})
${encoderGenerators[type] == null ? `${codec}.encode(${valueVar}${includeDefault ? ` ?? ${typeName}.${defaultValue}` : ''}, w)` : encoderGenerators[type](valueVar, includeDefault)}`
let writeField = (): string => `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. But if a message
Expand All @@ -437,7 +434,7 @@ export interface ${messageDef.name} {
writeField = () => `
for (const [key, value] of obj.${name}.entries()) {
${
createWriteField('{ key, value }')(false)
createWriteField('{ key, value }')()
.split('\n')
.map(s => {
const trimmed = s.trim()
Expand All @@ -452,7 +449,7 @@ export interface ${messageDef.name} {
writeField = () => `
for (const value of obj.${name}) {
${
createWriteField('value')(false)
createWriteField('value')()
.split('\n')
.map(s => {
const trimmed = s.trim()
Expand All @@ -468,7 +465,7 @@ export interface ${messageDef.name} {

return `
if (${valueTest}) {
${writeField(valueTest.includes('opts.writeDefaults === true'))}
${writeField()}
}`
}).join('\n')

Expand Down
4 changes: 2 additions & 2 deletions packages/protons/test/fixtures/basic.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,9 @@ export namespace Basic {
w.string(obj.foo)
}

if (opts.writeDefaults === true || (obj.num != null && obj.num !== 0)) {
if ((obj.num != null && obj.num !== 0)) {
w.uint32(16)
w.int32(obj.num ?? 0)
w.int32(obj.num)
}

if (opts.lengthDelimited !== false) {
Expand Down
40 changes: 20 additions & 20 deletions packages/protons/test/fixtures/bitswap.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,29 +57,29 @@ export namespace Message {
w.fork()
}

if (opts.writeDefaults === true || (obj.block != null && obj.block.byteLength > 0)) {
if ((obj.block != null && obj.block.byteLength > 0)) {
w.uint32(10)
w.bytes(obj.block ?? new Uint8Array(0))
w.bytes(obj.block)
}

if (opts.writeDefaults === true || (obj.priority != null && obj.priority !== 0)) {
if ((obj.priority != null && obj.priority !== 0)) {
w.uint32(16)
w.int32(obj.priority ?? 0)
w.int32(obj.priority)
}

if (obj.cancel != null) {
w.uint32(24)
w.bool(obj.cancel)
}

if (opts.writeDefaults === true || (obj.wantType != null && __WantTypeValues[obj.wantType] !== 0)) {
if (obj.wantType != null && __WantTypeValues[obj.wantType] !== 0) {
w.uint32(32)
Message.Wantlist.WantType.codec().encode(obj.wantType ?? Message.Wantlist.WantType.Block, w)
Message.Wantlist.WantType.codec().encode(obj.wantType, w)
}

if (opts.writeDefaults === true || (obj.sendDontHave != null && obj.sendDontHave !== false)) {
if ((obj.sendDontHave != null && obj.sendDontHave !== false)) {
w.uint32(40)
w.bool(obj.sendDontHave ?? false)
w.bool(obj.sendDontHave)
}

if (opts.lengthDelimited !== false) {
Expand Down Expand Up @@ -152,9 +152,9 @@ export namespace Message {
}
}

if (opts.writeDefaults === true || (obj.full != null && obj.full !== false)) {
if ((obj.full != null && obj.full !== false)) {
w.uint32(16)
w.bool(obj.full ?? false)
w.bool(obj.full)
}

if (opts.lengthDelimited !== false) {
Expand Down Expand Up @@ -215,14 +215,14 @@ export namespace Message {
w.fork()
}

if (opts.writeDefaults === true || (obj.prefix != null && obj.prefix.byteLength > 0)) {
if ((obj.prefix != null && obj.prefix.byteLength > 0)) {
w.uint32(10)
w.bytes(obj.prefix ?? new Uint8Array(0))
w.bytes(obj.prefix)
}

if (opts.writeDefaults === true || (obj.data != null && obj.data.byteLength > 0)) {
if ((obj.data != null && obj.data.byteLength > 0)) {
w.uint32(18)
w.bytes(obj.data ?? new Uint8Array(0))
w.bytes(obj.data)
}

if (opts.lengthDelimited !== false) {
Expand Down Expand Up @@ -299,14 +299,14 @@ export namespace Message {
w.fork()
}

if (opts.writeDefaults === true || (obj.cid != null && obj.cid.byteLength > 0)) {
if ((obj.cid != null && obj.cid.byteLength > 0)) {
w.uint32(10)
w.bytes(obj.cid ?? new Uint8Array(0))
w.bytes(obj.cid)
}

if (opts.writeDefaults === true || (obj.type != null && __BlockPresenceTypeValues[obj.type] !== 0)) {
if (obj.type != null && __BlockPresenceTypeValues[obj.type] !== 0) {
w.uint32(16)
Message.BlockPresenceType.codec().encode(obj.type ?? Message.BlockPresenceType.Have, w)
Message.BlockPresenceType.codec().encode(obj.type, w)
}

if (opts.lengthDelimited !== false) {
Expand Down Expand Up @@ -387,9 +387,9 @@ export namespace Message {
}
}

if (opts.writeDefaults === true || (obj.pendingBytes != null && obj.pendingBytes !== 0)) {
if ((obj.pendingBytes != null && obj.pendingBytes !== 0)) {
w.uint32(40)
w.int32(obj.pendingBytes ?? 0)
w.int32(obj.pendingBytes)
}

if (opts.lengthDelimited !== false) {
Expand Down
4 changes: 2 additions & 2 deletions packages/protons/test/fixtures/circuit.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,9 +95,9 @@ export namespace CircuitRelay {
w.fork()
}

if (opts.writeDefaults === true || (obj.id != null && obj.id.byteLength > 0)) {
if ((obj.id != null && obj.id.byteLength > 0)) {
w.uint32(10)
w.bytes(obj.id ?? new Uint8Array(0))
w.bytes(obj.id)
}

if (obj.addrs != null) {
Expand Down
Loading

0 comments on commit 078c62f

Please sign in to comment.