Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fields with reserved Javascript keywords are not prefixed with pb_ in AsObject types #20

Closed
Crevil opened this issue Apr 5, 2018 · 1 comment

Comments

@Crevil
Copy link
Contributor

Crevil commented Apr 5, 2018

Protobuf prefixes pb_ to fields with the name of a reserved Javascript keyword when calling the toObject() method, but the generated TypeScript typings Message.AsObject don't seem to take this into account. It's documented in this comment and the list of keywords used is here.

With the following proto file from the examples I've added a function field.

syntax = "proto3";

package com.book;

message Book {
    int64 isbn = 1;
    string title = 2;
    string author = 3;
    string function = 4;
}

The generated Javascript for toObject() becomes the following. Notice the pb_ prefix on the function field.

proto.com.book.Book.toObject = function(includeInstance, msg) {
  var f, obj = {
    isbn: jspb.Message.getFieldWithDefault(msg, 1, 0),
    title: jspb.Message.getFieldWithDefault(msg, 2, ""),
    author: jspb.Message.getFieldWithDefault(msg, 3, ""),
    pb_function: jspb.Message.getFieldWithDefault(msg, 4, "")
  };

  if (includeInstance) {
    obj.$jspbMessageInstance = msg;
  }
  return obj;
};
}

And this is the TypeScript for the return value of toObject():

export namespace Book {
    export type AsObject = {
        isbn: number,
        title: string,
        author: string,
        function: string,
    }
}

I can see that the AsObject type is defined in src/lib/template/partial/message.hbs lines 61 to 81 and I guess it should be trivial to verify each field name against the list of reserved keywords and prefix as necessary before handing the data to the template.

I'll be happy to provide a PR with a fix, but do you handle any other special cases at the moment, so I can make it in the same manner?

@agreatfool
Copy link
Owner

Sorry for the late reply.

Just merged PR & the latest new version 2.2.5 has been released.

Thank you for your Issue & PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants