Skip to content
This repository was archived by the owner on Jan 18, 2026. It is now read-only.

fix: use optional chaining for routing parameters#1138

Merged
alexander-fenster merged 1 commit intomainfrom
undefined
May 23, 2022
Merged

fix: use optional chaining for routing parameters#1138
alexander-fenster merged 1 commit intomainfrom
undefined

Conversation

@alexander-fenster
Copy link
Contributor

Fixes googleapis/google-cloud-node-core#572. The generated code for routing headers should use optional chaining (?.) instead of just dots for nested field names. Currently, it breaks compilation for Cloud Run v2 client library.

The change includes a small refactor: it's not correct to concatenate a dot-separated name in the generator code because, by design, the generator has no knowledge of the target language. All language specific stuff must be limited to the templates. So now the generator will only keep the list of camelCased field names, that will be joined (with ?.) in the template.

@alexander-fenster alexander-fenster requested a review from a team as a code owner May 21, 2022 02:05
{%- for param in paramArray %}
if((typeof request.{{ param.fieldRetrieve }} !== "undefined") && RegExp('{{ param.messageRegex | safe }}').test(request.{{ param.fieldRetrieve }}!)){
Object.assign(routingParameter, { {{ param.fieldSend }}: request.{{ param.fieldRetrieve }}!.match(RegExp('{{ param.namedSegment | safe }}'))![0]})}
if ((typeof request.{{ param.fieldRetrieve.join('?.') }} !== "undefined") && RegExp('{{ param.messageRegex | safe }}').test(request.{{ param.fieldRetrieve.join('?.') }}!)){
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, the old behavior we just had the object access baked into the string, and now we actually use an array type?

Seems like a good refactor to me.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, the generator keeps the parts as an array, and we only merge them with ?. in the template.

@alexander-fenster alexander-fenster merged commit b1aa357 into main May 23, 2022
@alexander-fenster alexander-fenster deleted the undefined branch May 23, 2022 17:54
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

protos for cloud run service generate invalid client

3 participants