-
Notifications
You must be signed in to change notification settings - Fork 243
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: enums not working - Expected enum value, got "<string>" #2534
go: enums not working - Expected enum value, got "<string>" #2534
Comments
The jsii runtime expects that enum values will be marshaled in the following form The go code generator renders enums like this: type StringEnum string
const (
StringEnumA StringEnum = "A"
StringEnumB StringEnum = "B"
StringEnumC StringEnum = "C"
) The current bindings implementation simply passes the values of enums through to the javascript code. This approach has the following downsides:
It seems like the idiom of using an alias and constants for enums is common in Go, so #1 is likely less of an issue for Go developers, and maybe we don't have to solve for it. In order to solve #2, we need two things:
We can address the type information problem by simply rendering the fully qualified member name in the consts: const (
StringEnumA StringEnum = "jsii-calc.StringEnum/A"
StringEnumB StringEnum = "jsii-calc.StringEnum/B"
StringEnumC StringEnum = "jsii-calc.StringEnum/C"
) But then, in the generated bindings of all code paths that can accept/return enums (methods, properties and structs), we will need to use the jsii type information to identify that an enum is passed in, and serialize it appropriately. AlternativeWould it make sense to model enums like this: type StringEnum struct {
StringEnum string `json:"$jsii.enum"`
}
func StringEnumA() StringEnum {
return StringEnum{StringEnum: "jsii-calc.StringEnum/A"}
}
func StringEnumB() StringEnum {
return StringEnum{StringEnum: "jsii-calc.StringEnum/B"}
}
func StringEnumC() StringEnum {
return StringEnum{StringEnum: "jsii-calc.StringEnum/C"}
} This will address both #1 and #2, but seems less idiomatic for Go. |
So we can keep the existing approach and fix number downside #2 by code generating custom json serialization implementations alongside the enum types like so: type JsiiEnum struct {
StringEnum string `json:"$jsii.enum"`
}
type MyEnum string
const (
Val1 MyEnum = "Val1"
Val2 MyEnum = "Val2"
)
var myEnumFqn = "jsii-calc.MyEnum"
var myEnumToId = map[string]MyEnum{
"jsii-calc.MyEnum/Val1": Val1,
"jsii-calc.MyEnum/Val2": Val2,
}
// Custom marshal serializes MyEnum vals as "{jsii.enum: fqn}" format
func (s MyEnum) MarshalJSON() ([]byte, error) {
fqn := fmt.Sprintf("%s/%s", myEnumFqn, string(s))
return json.Marshal(JsiiEnum{StringEnum: fqn})
}
// Custom unmarshal deserializes "{jsii.enum: fqn}" format
func (s *MyEnum) UnmarshalJSON(b []byte) error {
var j JsiiEnum
err := json.Unmarshal(b, &j)
if err != nil {
return err
}
val, ok := myEnumToId[j.StringEnum]
if !ok {
return errors.New("Invalid enum value")
}
*s = val
return nil
} I think this would be my preferred solution as it feels the most idiomatic though does require some significant increase in generated code. Most of the logic for the bodies of Unmarshal/Marshal implementations could be abstracted. If we decide that this is too complex or has other problems I'm not considering, I believe option #2 (the free floating functions for each option) would be my preference here. I don't see a lot of functional difference between the two besides the actual "values" of each option within the Enums feels more obscured by the functions and therefore more hidden from the user. That feels better to me when these values are FQNs are otherwise something the user may not know about or understand. This probably is largely inconsequential though. |
@MrArnoldPalmer thanks for the inputs. I'll try to take the approach you proposed. |
@MrArnoldPalmer I've implemented your suggestion in #2535. I did have to modify the function |
@RomainMuller brought up that the custom marshaller approach won't work if we set/get the enum through an I recon this means that we will need to take the approach described in "Alternative" above in which enum members are rendered as functions that return objects that intrinsically include the Okay, continuing to explore... |
I think it'll end up something along the lines of what is done for object references: jsii/packages/@jsii/go-runtime/jsii-runtime-go/runtime.go Lines 204 to 224 in 0368d42
|
Enum values are represented in the jsii kernel as `{ "$jsii.enum": "fqn/member" }`, so we need to marshal/unmarshall them based on this encoding. Change the type registry to capture a mapping between FQN to enum member consts and type to enum FQNs. Fixes #2534 Co-authored-by: Romain Marcadier <[email protected]> --- 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
|
🐛 Bug Report
Affected Languages
Golang
TypeScript
orJavascript
Python
Java
C#
,F#
, ...)General Information
What is the problem?
Enums are not working.
Consider the following jsii code packaged as
github.com/eladb/go-enum-repro/enumrepro
:And the consuming go code:
When running this program, we get the following error:
The text was updated successfully, but these errors were encountered: