feat: support parsing of complex options#1744
feat: support parsing of complex options#1744alexander-fenster merged 28 commits intoprotobufjs:masterfrom
Conversation
* test: adding test for pbjs static code generation * fix: fromObject should not initialize oneof members
This option skips generation of service clients. Co-authored-by: Alexander Fenster <fenster@google.com>
* deps: set @types/node to star version When using `protobuf.js` as a dependency in a project it is important that `@types/node` package gets de-duped and has the same version as for the rest of the modules in the project. Otherwise, typing conflicts could happen as they do between v13 and v14 node types. * fix: use @types/node >=13.7.0 * fix: use @types/node >=13.7.0 Co-authored-by: Alexander Fenster <fenster@google.com> Co-authored-by: Alexander Fenster <github@fenster.name>
Co-authored-by: xiaoweili <xiaoweili@tencent.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
|
@alexander-fenster Will be glad if you could take a look at this |
|
+1 |
|
In the meanwhile this fix is published as https://www.npmjs.com/package/@cldcvr/protobufjs because I needed it to make https://www.npmjs.com/package/@cldcvr/protobufjs-typescript-gen |
alexander-fenster
left a comment
There was a problem hiding this comment.
Thank you! This will go into v7.0 that we are going to publish next week.
|
@NikhilVerma Would you please fix linting, or grant the contributors write access to your branch? Thank you! |
|
@alexander-fenster Thank you! I've added you as the collaborator to my repo |
|
@alexander-fenster I've fixed the lint issues thanks! |
|
Hi @NikhilVerma, I switched your PR to be against Also, the CI should be green now so we are good to go after some extra 👀 . Thank you! |
|
Actually, the tests now fail with do you have an idea why is that? I can also take a look but it will take me longer to figure out. |
|
@alexander-fenster While checking I realised I had written some unnecessary code, it already handled arrays just didn't handle them in all conditions. So I've simplified the code and the tests pass now with the simpler code. |
|
@alexander-fenster Amazing! thanks for the collaboration :) Looking forward to v7 |
|
I'm not sure the issue was fixed though. I have the following Protobuf: message AwesomeMessage {
option (option.extension) = {
scopes: [SCOPE_PRODUCT]
visibility: VISIBILITY_PUBLIC
};
repeated Label labels = 1;
LabelSource labels_source = 2 [(option.field).visibility=VISIBILITY_INTERNAL];
}and decoding still fails with error |
|
@emarkk Are you using v7.0.0? I just tried this on v7 and it worked fine syntax = "proto3";
message AwesomeMessage {
option (option.extension) = {
scopes: [SCOPE_PRODUCT]
visibility: VISIBILITY_PUBLIC
};
repeated Label labels = 1;
LabelSource labels_source = 2 [(option.field).visibility=VISIBILITY_INTERNAL];
}
enum LabelSource {
LABEL_SOURCE_UNKNOWN = 0;
LABEL_SOURCE_USER = 1;
LABEL_SOURCE_AUTO = 2;
}
enum Label {
LABEL_UNKNOWN = 0;
LABEL_USER = 1;
LABEL_AUTO = 2;
} |
|
Ok, my bad. It was just a cache issue. Works fine with v7. Thanks a lot! |
This allows parsing of complex options and fixes #1743