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

fix: auto convert nested fields#438

Merged
alvarowolfx merged 6 commits intogoogleapis:mainfrom
alvarowolfx:fix-nested-auto-convert
Apr 8, 2024
Merged

fix: auto convert nested fields#438
alvarowolfx merged 6 commits intogoogleapis:mainfrom
alvarowolfx:fix-nested-auto-convert

Conversation

@alvarowolfx
Copy link
Copy Markdown
Contributor

@alvarowolfx alvarowolfx commented Apr 8, 2024

Auto converting types when automatically encoding JSON -> Protobuf was not working for nested structs (Arrays and Objects). This PR fix the behavior.

Related to #436 and #422

@alvarowolfx alvarowolfx requested a review from a team April 8, 2024 14:37
@product-auto-label product-auto-label Bot added size: m Pull request size is medium. api: bigquerystorage Issues related to the googleapis/nodejs-bigquery-storage API. labels Apr 8, 2024
@alvarowolfx alvarowolfx added the owlbot:run Add this label to trigger the Owlbot post processor. label Apr 8, 2024
@gcf-owl-bot gcf-owl-bot Bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Apr 8, 2024
Comment thread src/managedwriter/encoder.ts Outdated
value: any,
key: string,
ptype: protobuf.Type
): any | null {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

you are returning undefined but this line says any | null. it works because any allows anything, but maybe it's better to say any | undefined?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

ah good catch, I started using null and later moved to undefined and forgot to change the signature.

Copy link
Copy Markdown
Contributor

@alexander-fenster alexander-fenster left a comment

Choose a reason for hiding this comment

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

In general, I would think if it's possible to replace all any with something more specific, at least maybe something like number | string | {} - not sure if it's at all possible in this kind of code that really works with arbitrary objects, but might still find an issue or two.

@product-auto-label product-auto-label Bot added size: l Pull request size is large. and removed size: m Pull request size is medium. labels Apr 8, 2024
@alvarowolfx alvarowolfx added the owlbot:run Add this label to trigger the Owlbot post processor. label Apr 8, 2024
@gcf-owl-bot gcf-owl-bot Bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Apr 8, 2024
@product-auto-label product-auto-label Bot added size: m Pull request size is medium. and removed size: l Pull request size is large. labels Apr 8, 2024
@alvarowolfx alvarowolfx merged commit 0ba5b7d into googleapis:main Apr 8, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

api: bigquerystorage Issues related to the googleapis/nodejs-bigquery-storage API. size: m Pull request size is medium.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants