-
Notifications
You must be signed in to change notification settings - Fork 15
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
Fix NodeJS metadata type #159
Conversation
assert.Contains(t, string(testResource), "public readonly metadata!: pulumi.Output<outputs.meta.v1.ObjectMeta>;", "expected metadata output type") | ||
assert.Contains(t, string(testResource), "metadata?: pulumi.Input<inputs.meta.v1.ObjectMeta>;", "expected metadata input type") |
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.
These were previously pulumi.Output<ObjectMeta>
and pulumi.Input<ObjectMeta>
respectively.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #159 +/- ##
==========================================
- Coverage 78.56% 75.98% -2.58%
==========================================
Files 17 17
Lines 723 966 +243
==========================================
+ Hits 568 734 +166
- Misses 87 163 +76
- Partials 68 69 +1 ☔ View full report in Codecov by Sentry. |
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.
Thanks for the fix. #143 was trying to reduce the scope of changes to just the underlying implementation details, so not much thought was put towards this UX. The changes makes sense to me and I agree that it should be safe.
All of our languages except NodeJS use
SchemaPackageWithObjectMetaType
. As a result, Node exposes only input types for object metadata which makes it awkward to consume downstream.I'm assuming #143 didn't change this for backwards-compatibility reasons, or maybe it was overlooked. I think it is safe to continue generating the
ObjectMeta
type alias (in case users are importing it) but use the input/output types (matching the upstream k8s types) on resources.Edit:
So this seems intentional. For whatever reason, using this with Node has the effect of generating input/output types matching upstream's types, which seems desirable.
Fixes #158