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

Filtering base interface selections is not type-aware #106

Closed
gmac opened this issue Nov 10, 2021 · 1 comment · Fixed by #108
Closed

Filtering base interface selections is not type-aware #106

gmac opened this issue Nov 10, 2021 · 1 comment · Fixed by #108
Labels
bug Something isn't working

Comments

@gmac
Copy link
Contributor

gmac commented Nov 10, 2021

Another crazy one along the lines of #87, and confirms the issue hypothesized in #91.

The Problem

During final selection filtering in the execution pipeline, type awareness seems to be lost on base interface selections (i.e.: fields selected from an interface without going through a typed fragment). For example, here myInterface is an interface type that offers myUnion as a base field:

query {
  myInterface {
    myUnion {
      ...on Product {
        name
        manufacturer {
          name
        }
      }
    }
  }
}

Stacking these abstract selections atop one another results in the following unsuccessful outcome... while the underlying queries look accurate, the final filtered result is off:

{
  "data": {
    "myInterface": {
      "myUnion": {
        "name": "iPhone",
        "manufacturer": {
          "_bramble_id": "1"
        },
        "_bramble_id": "1",
        "_bramble__typename": "Product"
      }
    }
  }
}

HOWEVER, this issue can be avoided by wrapping the second-level union in a typed fragment. The following selection works perfectly, implying that the problem is specifically with base interface selections:

query {
  myInterface {
    ...on Product {
      myUnion {
        ...on Product {
          name
          manufacturer {
            name
          }
        }
      }
    }
  }
}

Probable cause

I believe this requires a complimenting fix to the changes made in #91. While #91 assured that abstract types always have access to their type information, I believe that the execution pipeline must still be updated to leverage this base type information.

@gmac
Copy link
Contributor Author

gmac commented Nov 12, 2021

Actual cause

The myInterface field was being treated as a namespace, and the namespace handler block was modifying the original user selection during planning rather than adjusting a copy. This is addressed in #108.

The fact that myInterface was being treated as a namespace is debatably problematic, though not strictly broken. A proposed fix for supporting interfaces with first-class routing is in #107.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants