Skip to content
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

Go Optional Function Argument Support #2442

Closed
MrArnoldPalmer opened this issue Jan 20, 2021 · 1 comment · Fixed by #2705
Closed

Go Optional Function Argument Support #2442

MrArnoldPalmer opened this issue Jan 20, 2021 · 1 comment · Fixed by #2705
Assignees
Labels
effort/large Large work item – several weeks of effort feature-request A feature should be added or improved. p1

Comments

@MrArnoldPalmer
Copy link
Contributor

The Go runtime and code generation don't currently support TS functions with optional arguments. In JSII libraries currently, most methods that accept complex data structures use interfaces with optional properties. These are currently handled "correctly" implicitly because when these are "zero values" in go, they are serialized as undefined and therefore not passed over the wire.

A more explicit approach will likely have to be taken here. A couple of options that have been discussed previously.

  1. Wrapper types. This would cause us to wrap all optional types in some wrapper type. This however has the huge downside of when a type goes from required to optional in TS, a breaking change would occur in the generated Go code. This is likely untenable.

  2. Pointers everywhere. All types in all public APIs would be converted from T to *T. This would allow users to pass null pointers for optional values. This also has some huge downsides. The first being, users can also pass null pointers to non-optional types and the go compiler wouldn't complain, these would manifest as a runtime crash. Additionally, increased verbosity is an issue. In Go you cannot pass a pointer to a primitive type anonymously. ie callThing(&"MyString"). Instead the user would have to allocate a variable with "MyString" and specify a pointer to that. This however could be mitigated by creating wrapper types from "pointer to a thing".

@MrArnoldPalmer MrArnoldPalmer added feature-request A feature should be added or improved. effort/medium Medium work item – a couple days of effort p2 labels Jan 20, 2021
@MrArnoldPalmer MrArnoldPalmer mentioned this issue Jan 20, 2021
2 tasks
@MrArnoldPalmer MrArnoldPalmer added effort/large Large work item – several weeks of effort and removed effort/medium Medium work item – a couple days of effort labels Jan 20, 2021
@MrArnoldPalmer MrArnoldPalmer self-assigned this Jan 20, 2021
@MrArnoldPalmer MrArnoldPalmer added p1 and removed p2 labels Mar 16, 2021
@mergify mergify bot closed this as completed in #2705 Mar 18, 2021
mergify bot pushed a commit that referenced this issue Mar 18, 2021
Adds support for passing `nil` to jsii nullable types in generated go code.

Changes go code generation to always use "pointer types" for all struct fields, method arguments, and method return values. Update runtime type conversion to correctly handle serializing and deserializing to these pointer types where necessary.

Removes unnecessary functionality for resolving scoped interface names for generated go types as each generated type now has only one public name. Adds support for resolving `scopedReferenceName` to easily generate "pointer types" for complex data structures from a type reference.

[RFC](aws/aws-cdk-rfcs#297)
fix: #2442
fix: #2671

---

By submitting this pull request, I confirm that my contribution is made under the terms of the [Apache 2.0 license].

[Apache 2.0 license]: https://www.apache.org/licenses/LICENSE-2.0
@github-actions
Copy link
Contributor

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
effort/large Large work item – several weeks of effort feature-request A feature should be added or improved. p1
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant