Opteo/google-ads-node

Upgrade to Google Ads API v17.1

Closed this issue · 5 comments

Google Ads API updated their version to 17.1 as of yesterday (Aug 7th, 2024). I was hoping to update this package, along with the Ads API package (https://github.com/Opteo/google-ads-api).

In order to do so, I see that the instructions give direction to pull the latest changes from gax-nodejs (https://github.com/Opteo/gax-nodejs). I also noticed that this project is behind by 80 commits from the Google Gax package. I tried to create a pull request for that package. But I can't, since I'm not a contributor.

Would we be able to get started on this update?

Also, I found a clue as to what may be causing the case issues that are mentioned in this comment:

# horrible hack but no easy way to resolve this
# requires preventing request parameters being compiled to snakeCase
# in this dependency: https://github.com/googleapis/gapic-generator-typescript/blob/master/templates/typescript_gapic/src/%24version/%24service_client.ts.njk
# i will personally buy a beer to whoever can solve this, good luck.

The final comment of an issue reported in the gapic-generator-typescript package give some insight. googleapis/gapic-generator-typescript#961. It references this change: Opteo/gax-nodejs@a1c226a

I don't think I understand enough of the code to definitively say whether this is the answer. But it definitely seems like a possibility. Let me know what you think.

I found out something else concerning the casing. There is an option in gapic-tools that allows for keeping the generated protos in snake_case. https://github.com/googleapis/gax-nodejs/blob/d74fa409897468af5dba78ef60bb5d4c9defacb6/tools/src/compileProtos.ts#L409 The only issue that I've seen with it, is that the tests are designed to check for camelCase.

In the attached links, others have asked about the snake_case & camelCase issues. From what I have researched, Google developers almost always say that camelCase is the standard casing for JavaScript. So they've purposefully coded any nodejs packages and builds to convert to camelCase. So I wonder if we may want to add support for each case, depending on whether the --keep-case param is set.

https://github.com/googleapis/gax-nodejs/blob/e6669acbf366534c74b8ad99691174288f73e60e/test/unit/transcoding.ts#L157

googleapis/gapic-generator-typescript#1126

googleapis/gax-nodejs#1312

Looks l like we could use Google's googleapis/gax-nodejs package directly. It should work because of this pull request: googleapis/gax-nodejs#1561

It's three months have passed since v17 release. Please upgrade to v17 or (better) to v18 which is around the corner.

Done :)

@Raymondo97, you're right that we can use the original gax-nodejs package instead of our fork thanks to their addition of the keep-case and force-number arguments, which they pass trough to pbjs. I've updated this lib to reflect this.