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 runtime type registration #2561

Closed
RomainMuller opened this issue Feb 11, 2021 · 1 comment · Fixed by #2567
Closed

Go runtime type registration #2561

RomainMuller opened this issue Feb 11, 2021 · 1 comment · Fixed by #2567
Assignees
Labels
effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. language/go Regarding GoLang bindings module/runtime Issues affecting the `jsii-runtime`

Comments

@RomainMuller
Copy link
Contributor

RomainMuller commented Feb 11, 2021

🚀 Feature Request

We need to re-work the type handling, moving way from the impMap passed to the runtimes Invoke, StaticInvoke, Get and StaticGet calls, and instead have a client-wide type registry.

This is necessary to enable returning the correct dynamic type in several occasions:

  • Declared type is ParentClass, dynamic value is ChildClass extends ParentClass
  • Returning values that implement a collection of interfaces
  • ...

This may also be necessary to be able to correctly handle enum values; and to be able to re-hydrate structs correctly.

@RomainMuller RomainMuller added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels Feb 11, 2021
@RomainMuller RomainMuller self-assigned this Feb 11, 2021
@RomainMuller RomainMuller added effort/small Small work item – less than a day of effort language/go Regarding GoLang bindings module/runtime Issues affecting the `jsii-runtime` and removed needs-triage This issue or PR still needs to be triaged. labels Feb 12, 2021
RomainMuller added a commit that referenced this issue Feb 12, 2021
The previous convention of passing of `implementationMap` values at the
call sites for value-returning calls (`Get`, `Invoke`, and their static
counterparts) did not provide sufficient information to properly convert
certain values (JavaScript returning a descendant of the declared return
type, decoding enum members, ability to represent values that implement
multiple interfaces in JavaScript, ...).

In order to address this shortcomings, this mechanism was replaced with
a new `func init(){}` block emitted in each package, which is
responsible for registering the generated types with the runtime
library. This allows the `client` to determine the correct go type to
use when converting arbitrary values returned from JavaScript, as it has
visibility on the complete set of available types, by fully qualified
name.

This minimally changes the conversion of JavaScript values into go
values, so as to minimize the footprit of this change. Of course,
subsequent effort will be made to fully leverage the potential of the
extended information (i.e: correctly processing enum members, ...).

Fixes #2561
RomainMuller added a commit that referenced this issue Feb 12, 2021
The previous convention of passing of `implementationMap` values at the
call sites for value-returning calls (`Get`, `Invoke`, and their static
counterparts) did not provide sufficient information to properly convert
certain values (JavaScript returning a descendant of the declared return
type, decoding enum members, ability to represent values that implement
multiple interfaces in JavaScript, ...).

In order to address this shortcomings, this mechanism was replaced with
a new `func init(){}` block emitted in each package, which is
responsible for registering the generated types with the runtime
library. This allows the `client` to determine the correct go type to
use when converting arbitrary values returned from JavaScript, as it has
visibility on the complete set of available types, by fully qualified
name.

This minimally changes the conversion of JavaScript values into go
values, so as to minimize the footprit of this change. Of course,
subsequent effort will be made to fully leverage the potential of the
extended information (i.e: correctly processing enum members, ...).

Fixes #2561
@mergify mergify bot closed this as completed in #2567 Feb 17, 2021
mergify bot pushed a commit that referenced this issue Feb 17, 2021
The previous convention of passing of `implementationMap` values at the
call sites for value-returning calls (`Get`, `Invoke`, and their static
counterparts) did not provide sufficient information to properly convert
certain values (JavaScript returning a descendant of the declared return
type, decoding enum members, ability to represent values that implement
multiple interfaces in JavaScript, ...).

In order to address this shortcomings, this mechanism was replaced with
a new `func init(){}` block emitted in each package, which is
responsible for registering the generated types with the runtime
library. This allows the `client` to determine the correct go type to
use when converting arbitrary values returned from JavaScript, as it has
visibility on the complete set of available types, by fully qualified
name.

This minimally changes the conversion of JavaScript values into go
values, so as to minimize the footprit of this change. Of course,
subsequent effort will be made to fully leverage the potential of the
extended information (i.e: correctly processing enum members, ...).

Fixes #2561

---

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/small Small work item – less than a day of effort feature-request A feature should be added or improved. language/go Regarding GoLang bindings module/runtime Issues affecting the `jsii-runtime`
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant