Skip to content
This repository was archived by the owner on Nov 20, 2025. It is now read-only.

fix: do not use #private#1454

Merged
danielbankhead merged 1 commit intomainfrom
no-private
Aug 31, 2022
Merged

fix: do not use #private#1454
danielbankhead merged 1 commit intomainfrom
no-private

Conversation

@alexander-fenster
Copy link
Copy Markdown
Contributor

Adding a # private method is technically a breaking change. Our packing tests across all client libraries are now failing with

node_modules/google-auth-library/build/src/auth/googleauth.d.ts:67:5 - error TS18028: Private identifiers are only available when targeting ECMAScript 2015 and higher.

67     #private;
       ~~~~~~~~

(in CI, this looks like a pack-n-play TypeScript test failure as a part of system tests)

Let's not be that modern and use the regular private method instead.

@alexander-fenster alexander-fenster requested review from a team, feywind and ruyadorno August 31, 2022 23:02
@product-auto-label product-auto-label bot added the size: s Pull request size is small. label Aug 31, 2022
@danielbankhead danielbankhead merged commit 6c30274 into main Aug 31, 2022
@danielbankhead danielbankhead deleted the no-private branch August 31, 2022 23:14
gcf-merge-on-green bot pushed a commit that referenced this pull request Aug 31, 2022
@ruyadorno
Copy link
Copy Markdown

Oh that's interesting, private class fields have been on Node.js since v12. I believe this is more of a matter of reconfiguring our TypeScript compiler to a little more modern level of ECMAScript support rather than being an unsupported feature.

That said, there's maybe an argument to be made on what is the more JS-idiommatic way of expressing the intent of a private method/property vs TS-idiommatic and I'm not sure where the collective consensus within the larger JavaScript ecosystem sits on that.

Anyways just wanted to highlight that it might be possible to support #private notations and that it might be more familiar to a lot of JS devs and hear your thoughts on it @alexander-fenster (I might be missing more context here, feel free to correct me if I'm making any wrong assumption) 😊

@danielbankhead
Copy link
Copy Markdown
Contributor

@ruyadorno no problem, we just need to update gts & TypeScript in https://github.com/google/pack-n-play (the repo needs some love).

@DesAWSume
Copy link
Copy Markdown

a year after, this issue bothers me.
`node_modules/google-auth-library/build/src/util.d.ts:134:5 - error TS18028: Private identifiers are only available when targeting ECMAScript 2015 and higher.

134 #private;
~~~~~~~~

Found 1 error.`

@danielbankhead
Copy link
Copy Markdown
Contributor

@DesAWSume the error message explains what to do; simply target es2015+ in TypeScript:

@DesAWSume
Copy link
Copy Markdown

Oh~. 2nd look. I have a tsconfig under src folder, which my yarn build command will target to it.
My bad. It fixed for me~

GautamSharda pushed a commit to googleapis/google-cloud-node-core that referenced this pull request Oct 9, 2025
GautamSharda pushed a commit to googleapis/google-cloud-node-core that referenced this pull request Oct 21, 2025
GautamSharda pushed a commit to googleapis/google-cloud-node-core that referenced this pull request Oct 30, 2025
miguelvelezsa pushed a commit to googleapis/google-cloud-node-core that referenced this pull request Nov 6, 2025
miguelvelezsa pushed a commit to googleapis/google-cloud-node-core that referenced this pull request Nov 6, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

size: s Pull request size is small.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants