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

Add option for exhaustive union queries #225

Open
amiralies opened this issue Dec 15, 2020 · 7 comments
Open

Add option for exhaustive union queries #225

amiralies opened this issue Dec 15, 2020 · 7 comments

Comments

@amiralies
Copy link

Following discussions in discord I think it makes sense to have an option to do exhaustive union queries.

let's say I have a union type

union T = A | B | C

I want to get a warning/error when I try to query this type without covering all branches.

# C is missing
{
  queryT {
    ... on A {
      __typename
    }
    ... on B {
      __typename
    }
  }
}

How to enable this option?

We can have an option on bsconfig { "exhaustiveUnions": true } and also a directive @exahustive for cases when people don't want to do exhaustive all union queries. we can also have a @nonexhaustive directive for suppressing warning/error on a single query when the project wide option is enabled.

@amiralies
Copy link
Author

I'm interested in working on this, though I'm not very familiar with the codebase, @jfrolich is this possible for you to share guide /tips on what should be done?

@jfrolich
Copy link
Collaborator

Hey, yes that would be amazing. Unfortunately I don't have a lot of time with a newborn. It would be great to have some kind of guide at some point. Most of the difficulty in the codebase is dealing with the OCaml parse tree (ast). I recently started to create rei files to have a clearer public interface with types. I don't really know where to start explaining. I would suggest to dive in and try to change a tiny thing and go from there.

@jfrolich
Copy link
Collaborator

I would also have to dive in to see how the codebase is tracking exhaustiveness (has been some time ago).

@jfrolich
Copy link
Collaborator

Probably good to look at generate_poly_variant_union_decoder in output_bucklescript_parser.re

@amiralies
Copy link
Author

Thank you, I will take a look

@amiralies
Copy link
Author

amiralies commented Jan 23, 2021

Looking at native code, it seems when querying a union non exhaustivly it generates another variant (Nonexhaustive) alongside variants associated with union type arms.
What do you think about adding this to rescript side instead of throwing warning/error.
For sake of backward compatibility and customization we can only enable this via an option.

@jfrolich
Copy link
Collaborator

Yeah I think it makes sense to have that instead of FutureAddedValue when it's not exhaustive.

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

No branches or pull requests

2 participants