Skip to content

Commit

Permalink
Merge pull request #45 from join-com/fix-undefined-strings
Browse files Browse the repository at this point in the history
fix: apply .asInterface() always
  • Loading branch information
castarco authored Jul 14, 2021
2 parents 989e8b8 + aae3e81 commit 9a5c7a6
Show file tree
Hide file tree
Showing 9 changed files with 179 additions and 28 deletions.
31 changes: 11 additions & 20 deletions internal/generator/runner_generate_class.go
Original file line number Diff line number Diff line change
Expand Up @@ -181,14 +181,14 @@ func (r *Runner) generateTypescriptClassField(
}

func (r *Runner) generateTypescriptClassPatchedMethods(generatedFileStream *protogen.GeneratedFile, messageSpec *descriptorpb.DescriptorProto, requiredFields bool, hasEnums bool) {
r.generateAsInterfaceMethod(generatedFileStream, messageSpec, requiredFields, hasEnums)
r.generateAsInterfaceMethod(generatedFileStream, messageSpec, requiredFields)
r.generateFromInterfaceMethod(generatedFileStream, messageSpec, requiredFields, hasEnums)

r.generateDecodePatchedMethod(generatedFileStream, messageSpec, requiredFields)
r.generateEncodePatchedMethod(generatedFileStream, messageSpec, requiredFields, hasEnums)
}

func (r *Runner) generateAsInterfaceMethod(generatedFileStream *protogen.GeneratedFile, messageSpec *descriptorpb.DescriptorProto, requiredFields bool, hasEnums bool) {
func (r *Runner) generateAsInterfaceMethod(generatedFileStream *protogen.GeneratedFile, messageSpec *descriptorpb.DescriptorProto, requiredFields bool) {
className := strcase.ToCamel(messageSpec.GetName())
r.P(generatedFileStream, "public asInterface(): I"+className+" {")
r.indentLevel += 2
Expand All @@ -198,29 +198,23 @@ func (r *Runner) generateAsInterfaceMethod(generatedFileStream *protogen.Generat
hasRepeated = hasRepeated || (fieldSpec.GetLabel() == descriptorpb.FieldDescriptorProto_LABEL_REPEATED)
}

if hasEnums {
r.P(generatedFileStream, "const message = {")
r.indentLevel += 2

r.P(generatedFileStream, "...this,")
for _, fieldSpec := range messageSpec.GetField() {
r.generatePatchedInterfaceField(generatedFileStream, fieldSpec, messageSpec, requiredFields)
}

r.indentLevel -= 2
r.P(generatedFileStream, "}")
} else {
r.P(generatedFileStream, "const message = { ...this }")
r.P(generatedFileStream, "const message = {")
r.indentLevel += 2
r.P(generatedFileStream, "...this,")
for _, fieldSpec := range messageSpec.GetField() {
r.generatePatchedInterfaceField(generatedFileStream, fieldSpec, messageSpec, requiredFields)
}
r.indentLevel -= 2
r.P(generatedFileStream, "}")

var fieldAssignment string
var loopInnerIfClause string
if hasRepeated {
fieldAssignment = " const field = message[fieldName as keyof I"+className+"]"
fieldAssignment = " const field = message[fieldName as keyof I" + className + "]"
loopInnerIfClause = "field == null || Array.isArray(field) && field.length === 0"
} else {
fieldAssignment = ""
loopInnerIfClause = "message[fieldName as keyof I"+className+"] == null"
loopInnerIfClause = "message[fieldName as keyof I" + className + "] == null"
}

r.P(
Expand Down Expand Up @@ -410,9 +404,6 @@ func (r *Runner) generatePatchedInterfaceField(
if !ok || nestedMessageSpec == nil {
utils.LogError("Unable to retrieve message spec for " + fieldTypeName)
}
if fieldTypeName != ".google.protobuf.Timestamp" && !r.messageHasEnumsOrDates(nestedMessageSpec) {
return
}

if confirmRequired {
if fieldTypeName == ".google.protobuf.Timestamp" {
Expand Down
8 changes: 6 additions & 2 deletions tests/__tests__/generated/Flavors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,9 @@ export namespace Flavors {
public emails!: Email[]

public asInterface(): IUserProfile {
const message = { ...this }
const message = {
...this,
}
for (const fieldName of Object.keys(message)) {
const field = message[fieldName as keyof IUserProfile]
if (field == null || (Array.isArray(field) && field.length === 0)) {
Expand Down Expand Up @@ -107,7 +109,9 @@ export namespace Flavors {
public userId?: UserId

public asInterface(): IUserRequest {
const message = { ...this }
const message = {
...this,
}
for (const fieldName of Object.keys(message)) {
if (message[fieldName as keyof IUserRequest] == null) {
// We remove the key to avoid problems with code making too many assumptions
Expand Down
102 changes: 102 additions & 0 deletions tests/__tests__/generated/Regressions.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
// GENERATED CODE -- DO NOT EDIT!
/* eslint-disable @typescript-eslint/no-non-null-assertion */

import * as protobufjs from 'protobufjs/light'

// eslint-disable-next-line @typescript-eslint/no-namespace
export namespace Regressions {
interface ConvertibleTo<T> {
asInterface(): T
}

export interface IReg01Inner {
value?: string
}

export interface IReg01Outer {
inner?: IReg01Inner
}

@protobufjs.Type.d('regressions_Reg01Inner')
export class Reg01Inner
extends protobufjs.Message<Reg01Inner>
implements ConvertibleTo<IReg01Inner>, IReg01Inner
{
@protobufjs.Field.d(1, 'string', 'optional')
public value?: string

public asInterface(): IReg01Inner {
const message = {
...this,
}
for (const fieldName of Object.keys(message)) {
if (message[fieldName as keyof IReg01Inner] == null) {
// We remove the key to avoid problems with code making too many assumptions
delete message[fieldName as keyof IReg01Inner]
}
}
return message
}

public static fromInterface(this: void, value: IReg01Inner): Reg01Inner {
return Reg01Inner.fromObject(value)
}

public static decodePatched(
this: void,
reader: protobufjs.Reader | Uint8Array
): IReg01Inner {
return Reg01Inner.decode(reader).asInterface()
}

public static encodePatched(
this: void,
message: IReg01Inner,
writer?: protobufjs.Writer
): protobufjs.Writer {
return Reg01Inner.encode(message, writer)
}
}

@protobufjs.Type.d('regressions_Reg01Outer')
export class Reg01Outer
extends protobufjs.Message<Reg01Outer>
implements ConvertibleTo<IReg01Outer>, IReg01Outer
{
@protobufjs.Field.d(1, Reg01Inner, 'optional')
public inner?: Reg01Inner

public asInterface(): IReg01Outer {
const message = {
...this,
inner: this.inner?.asInterface(),
}
for (const fieldName of Object.keys(message)) {
if (message[fieldName as keyof IReg01Outer] == null) {
// We remove the key to avoid problems with code making too many assumptions
delete message[fieldName as keyof IReg01Outer]
}
}
return message
}

public static fromInterface(this: void, value: IReg01Outer): Reg01Outer {
return Reg01Outer.fromObject(value)
}

public static decodePatched(
this: void,
reader: protobufjs.Reader | Uint8Array
): IReg01Outer {
return Reg01Outer.decode(reader).asInterface()
}

public static encodePatched(
this: void,
message: IReg01Outer,
writer?: protobufjs.Writer
): protobufjs.Writer {
return Reg01Outer.encode(message, writer)
}
}
}
18 changes: 15 additions & 3 deletions tests/__tests__/generated/Test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,9 @@ export namespace Foo {
public title?: string

public asInterface(): INested {
const message = { ...this }
const message = {
...this,
}
for (const fieldName of Object.keys(message)) {
if (message[fieldName as keyof INested] == null) {
// We remove the key to avoid problems with code making too many assumptions
Expand Down Expand Up @@ -227,7 +229,9 @@ export namespace Foo {
public id?: number

public asInterface(): IRequest {
const message = { ...this }
const message = {
...this,
}
for (const fieldName of Object.keys(message)) {
if (message[fieldName as keyof IRequest] == null) {
// We remove the key to avoid problems with code making too many assumptions
Expand Down Expand Up @@ -272,7 +276,9 @@ export namespace Foo {
public optionalField?: number

public asInterface(): IRequiredPropertiesTest {
const message = { ...this }
const message = {
...this,
}
for (const fieldName of Object.keys(message)) {
if (message[fieldName as keyof IRequiredPropertiesTest] == null) {
// We remove the key to avoid problems with code making too many assumptions
Expand Down Expand Up @@ -454,6 +460,8 @@ export namespace Foo {
fieldEnumRepeated: this.fieldEnumRepeated?.map(
(e) => Role_Enum[e]! as Role
),
message: this.message?.asInterface(),
messageRepeated: this.messageRepeated?.map((o) => o.asInterface()),
timestamp:
this.timestamp != null
? new Date(
Expand All @@ -464,6 +472,10 @@ export namespace Foo {
timestampRepeated: this.timestampRepeated?.map(
(ts) => new Date((ts.seconds ?? 0) * 1000 + (ts.nanos ?? 0) / 1000000)
),
otherPkgMessage: this.otherPkgMessage?.asInterface(),
otherPkgMessageRepeated: this.otherPkgMessageRepeated?.map((o) =>
o.asInterface()
),
}
for (const fieldName of Object.keys(message)) {
const field = message[fieldName as keyof ITest]
Expand Down
4 changes: 3 additions & 1 deletion tests/__tests__/generated/common/Common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,9 @@ export namespace Common {
public latsName?: string

public asInterface(): IOtherPkgMessage {
const message = { ...this }
const message = {
...this,
}
for (const fieldName of Object.keys(message)) {
if (message[fieldName as keyof IOtherPkgMessage] == null) {
// We remove the key to avoid problems with code making too many assumptions
Expand Down
4 changes: 3 additions & 1 deletion tests/__tests__/generated/google/protobuf/Empty.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,9 @@ export namespace GoogleProtobuf {
implements ConvertibleTo<IEmpty>, IEmpty
{
public asInterface(): IEmpty {
const message = { ...this }
const message = {
...this,
}
for (const fieldName of Object.keys(message)) {
if (message[fieldName as keyof IEmpty] == null) {
// We remove the key to avoid problems with code making too many assumptions
Expand Down
4 changes: 3 additions & 1 deletion tests/__tests__/generated/google/protobuf/Timestamp.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,9 @@ export namespace GoogleProtobuf {
public nanos?: number

public asInterface(): ITimestamp {
const message = { ...this }
const message = {
...this,
}
for (const fieldName of Object.keys(message)) {
if (message[fieldName as keyof ITimestamp] == null) {
// We remove the key to avoid problems with code making too many assumptions
Expand Down
17 changes: 17 additions & 0 deletions tests/__tests__/proto/regressions.proto
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
syntax = "proto3";

package regressions;

// Makes protoc happy (useful for Golang code generation, not for TS)
option go_package = "github.com/join-com/protoc-gen-ts/regressions";


// Regression 01

message Reg01Inner {
string value = 1;
}

message Reg01Outer {
Reg01Inner inner = 1;
}
19 changes: 19 additions & 0 deletions tests/__tests__/regressions.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import { Regressions } from './generated/Regressions'

describe('regressions', () => {
it('01. decodes missing nested fields as undefined when there are no enums', () => {
// Background: before this test was written, calling .asInterface() for nested objects was
// done only when there were enum fields at some level of the object.

const originalA: Regressions.IReg01Outer = {}
const bufferA = Regressions.Reg01Outer.encodePatched(originalA).finish()
const reconstructedA = Regressions.Reg01Outer.decodePatched(bufferA)

const originalB: Regressions.IReg01Outer = { inner: {} }
const bufferB = Regressions.Reg01Outer.encodePatched(originalB).finish()
const reconstructedB = Regressions.Reg01Outer.decodePatched(bufferB)

expect(reconstructedA.inner?.value).toBeUndefined()
expect(reconstructedB.inner?.value).toBeUndefined()
})
})

0 comments on commit 9a5c7a6

Please sign in to comment.