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

Exposing the set of members of emitted enumerations #934

Conversation

blast-hardcheese
Copy link
Contributor

This is needed for more dynamic mapping of known values in enumerations. Example:

val (notInterested, interested) = categories.values.partition(user.preferredCateogry.contains _)

Again, I'm still onboarding onto GraphQL workflows, so if this PR doesn't make sense, thank you for your patience.

@@ -380,6 +380,10 @@ object ClientWriter {
.map(v => s"""case ${typedef.name}.${safeEnumValue(v.enumValue)} => __EnumValue("${v.enumValue}")""")
.mkString("\n")}
}

val values: Vector[${enumName}] = Vector(${typedef.enumValuesDefinition
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about a Set instead of Vector?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's possible to go from Vector to Set but not the other way around, I believe this is why existing enumeration implementations expose their members as sequences (scala enumerations and scalapb being two examples)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with you that people shouldn't depend on the ordering of members of an enumeration, but preventing them from doing so at all is just waiting for someone to come and ask for that feature, then breaking all consumers who are already using whatever implementation we decide on here.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I don't think it's right to expect any kind of order but I don't have strong feelings about it. Fine to merge like this.

@ghostdogpr ghostdogpr merged commit 540fcbf into ghostdogpr:master Jun 24, 2021
@blast-hardcheese blast-hardcheese deleted the expose-all-enum-members-as-vector branch June 24, 2021 04:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants