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

fix(temporary): reorder imports to work around protobuf.js problem #108

Merged
merged 1 commit into from
Dec 6, 2023

Conversation

vchudnov-g
Copy link
Collaborator

Apparently, protobuf.js is not currently importing the Any type when any.proto is last in the import list, but the import does work if any.proto is higher in the import list. Pending a fix for this in protobuf.js, this change imports any.proto earlier to avoid triggering the problem. When protobuf.js is fixed, we can revert the change to maintain alphabetical ordering of imports.

Apparently protobuf.js is not currently importing the Any type when
any.proto is last in the import list, but the import does work if
any.proto is higher in the import list. Pending a fix for this in
protobuf.js, this change imports any.proto earlier to avoid triggering
the problem. When protobuf.js if fixed, we can revert the change.
@vchudnov-g vchudnov-g requested review from a team as code owners December 6, 2023 00:00
@vchudnov-g vchudnov-g merged commit 74c0ede into main Dec 6, 2023
5 checks passed
@vchudnov-g vchudnov-g deleted the any-workaround-nodejs branch December 6, 2023 00:02
vchudnov-g added a commit that referenced this pull request Jul 31, 2024
This commit restores proto imports in the intended order, undoing the
workaround in #108 due
to protobufjs/protobuf.js#1954 now that the
solution protobufjs/protobuf.js#1960 is
implemented.
vchudnov-g added a commit that referenced this pull request Oct 10, 2024
…,ListValue,Struct} (#131)

The converter will now recognize a `format` specifier in certain schemas. Specifically, the following combinations are now recognized and result in a protobuf field whose type is the specified `format`:

| Discovery `"type"` | Discovery `"format"`        |
|--------------------|-----------------------------|
| `object`           | `google.protobuf.Struct`    |
| `any`              | `google.protobuf.Value`     |
| `array`            | `google.protobuf.ListValue` |

Note that, as was the case previously, other schemas with `"type" = "any"` and any other `format`, including none, are not accepted unless they are in an `error.details` field. (Googlers,  see also cl/683528084.)
 
Incidentally, this PR also restores proto imports in the intended order, undoing the workaround in #108 due to protobufjs/protobuf.js#1954 now that the solution protobufjs/protobuf.js#1960 is implemented.
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

Successfully merging this pull request may close these issues.

2 participants