facebook / facebook/relay

`enable_typename_discriminated_unions` feature bug - abstract inline fragment selections incorrectly merged onto concrete union arms

Open
#5,395 1 comment 0 reactions 0 assignees View on GitHub
Dominant language
Rust
Stars
19k
Forks
1.9k
PR merge metrics
No merged PRs in 30d

Description

I am trying out enable_typename_discriminated_unions. Thank you, @morrys and relay team for pushing this feature forward!

I found a bug. Sharing some repros. I would like to hear your thoughts on what we think the expected output should be.

Bug: When typegen handles a fragment on an abstract type that mixes abstract and concrete inline fragments, it builds incorrect discriminated union arms:

### Case 1 :
Fields from an abstract spread (e.g. ... on Named { name }) are merged onto concrete arms that do not implement that abstract type (e.g. Page).

Input
```
fragment AbstractConcreteMixedInline on Actor {
__typename
id
... on Named { name } # abstract type - only User implements Named (not Page)
... on User { firstName }
... on Page { subscribeStatus }
}
```

Output - name incorrectly on Page

```
| { __typename: "Page"; id: string; name?: string; subscribeStatus: string }
| { __typename: "User"; id: string; name?: string; firstName: string }
| { __typename: "%other" }

Expected Output

```
{ __typename: "User"; id: string; name?: string; firstName?: string }
| { __typename: "Page"; id: string; subscribeStatus?: string } // no name
| { __typename: "%other"; id: string }
```

Reference [test](https://github.com/Lalitha-Iyer/relay/blob/d8acd4ccf603a27f4ab4c90f02b1a61f15f1745f/compiler/crates/relay-typegen/tests/generate_typescript/fixtures/abstract-concrete-mixed-inline.expected)

### Case 2:
Concrete arms implied only by an abstract spread can be missing (e.g., no User arm when ... on Named is selected but ... on User is not).

Input
```
fragment AbstractNamedWithConcretePage on Actor {
__typename
id
... on Named { name }
... on Page { subscribeStatus }
}
```
Actual Output - User arm missing; name on Page

```
{ __typename: "Page"; id: string; name?: string; subscribeStatus: string }
{ __typename: "%other" }
```

Expected Output
```
{ __typename: "User"; id: string; name?: string } # this can be a union eg: __typename: User | FooNamedType
| { __typename: "Page"; id: string; subscribeStatus?: string } // no name
| { __typename: "%other"; id: string }
```

Reference [test](https://github.com/Lalitha-Iyer/relay/blob/d8acd4ccf603a27f4ab4c90f02b1a61f15f1745f/compiler/crates/relay-typegen/tests/generate_typescript/fixtures/abstract-named-with-concrete-page.expected)

Contributor guide

Open the contributing guide

Research direction

Start with compiler/crates/relay-typegen/tests/generate_typescript/fixtures/abstract-concrete-mixed-inline.expected and abstract-named-with-concrete-page.expected, then trace the typegen path that produces them. Reproduce both cases and compare abstract and concrete arm selection with the expected outputs. Done means abstract fields appear only on compatible arms and concrete arms implied by abstract spreads are present.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust, typescript
Domain
compilers
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
68/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.