-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
[codegen/nodejs] - Dynamic provider config getters #7530
Conversation
ccd4146
to
9ee034e
Compare
Diff for pulumi-random with merge commit ba1aec3 |
pkg/codegen/internal/test/testdata/provider-config-schema/nodejs/config/vars.ts
Outdated
Show resolved
Hide resolved
Diff for pulumi-azuread with merge commit d10b7e6 |
Diff for pulumi-random with merge commit d10b7e6 |
Diff for pulumi-random with merge commit 18eb428 |
Diff for pulumi-azuread with merge commit 18eb428 |
Diff for pulumi-gcp with merge commit d10b7e6 |
Diff for pulumi-azure with merge commit d10b7e6 |
Diff for pulumi-gcp with merge commit 18eb428 |
Diff for pulumi-azure with merge commit 18eb428 |
Diff for pulumi-aws with merge commit d10b7e6 |
Diff for pulumi-aws with merge commit 18eb428 |
Diff for pulumi-random with merge commit dcfc39f |
Diff for pulumi-azuread with merge commit dcfc39f |
Diff for pulumi-gcp with merge commit dcfc39f |
Diff for pulumi-azure with merge commit dcfc39f |
Diff for pulumi-aws with merge commit dcfc39f |
bdc563e
to
909fc53
Compare
Diff for pulumi-azuread with merge commit 544d47c |
Diff for pulumi-random with merge commit 544d47c |
Diff for pulumi-gcp with merge commit 544d47c |
Diff for pulumi-random with merge commit 0507380 |
Diff for pulumi-azuread with merge commit 0507380 |
Diff for pulumi-azure with merge commit 544d47c |
Diff for pulumi-gcp with merge commit 0507380 |
Diff for pulumi-azure with merge commit 0507380 |
Diff for pulumi-aws with merge commit 544d47c |
Diff for pulumi-aws with merge commit 0507380 |
// Specifically, this doesn't work right if the first value is set to false but the default value | ||
// is true. | ||
configFetch += " || " + v | ||
configFetch += " ?? " + v |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's null coalescing operator? Awesome.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wunderbar, I am learning some TypeScript tricks here, thanks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
With the TypeScript upgrade, it's worth noting that it's no longer possible to set these values (given the new TypeScript => JavaScript transpiled code), so it'll no longer be possible to do things like this. But this change addresses the problem that that's working around, so there's no longer a need to do that.
It's great not to be able to do things like that. |
Description
Nodejs part of #5582
Checklist