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

Could fragments automatically be passed to their queried types? (React) #1163

Open
chestercharles opened this issue Sep 22, 2023 · 4 comments
Labels
Enhancement A new feature or improvement to Houdini's public API

Comments

@chestercharles
Copy link

Describe the feature

Using this example from the docs:

Fragments are defined inside of the useFragment hook:

src/components/ShowCard.tsx
import { graphql, useMutation } from "$houdini";

export function ShowCard(props: { show: ShowCardInfo }) {
  const data = useFragment(props.show, graphql(`
    fragment ShowCardInfo on Show {
        name
    }
  `))

  return (
    <div>
        {data.name}
    </div>
  )
}

And then can be passed to your graphql query:

src/routes/show/[id]/+page.gql
query ShowInfo($id: ID!) {
    show(id: $id) {
        ...ShowCardInfo
    }
}

And then threaded through to the component:

src/routes/show/[id]/+page.tsx
export default function ShowInfoView({ ShowInfo }) {
    return (
        <ShowCard show={ShowInfo.show} />
    )
}

It seems plausible for the compiler to detect the ShowCard's show prop fragment is on the graph type of the passed value. If the all of the fragments expected on a queried type were known by analyzing the components to which the type was passed, perhaps the compiler could automatically pass the fragments to that type in the query operation.

Would it be possible for this to eliminate the need to manually pass the fragment to the desired type in the src/routes/show/[id]/+page.gql?

I may be missing something, and there may be examples where the desired behavior would be ambiguous. But thought it was worth posting this thought.


Fantastic work! I really love how this framework integrates file-based routing and component-fragment colocation! ❤️

Criticality

None

@chestercharles chestercharles changed the title Could fragments automatically be passed to their queried types? Could fragments automatically be passed to their queried types? (React) Sep 22, 2023
@AlecAivazis
Copy link
Collaborator

Our first react issue and its not a bug!

I think you're onto something that I've been thinking about since I saw the isograph talk at GraphQL Conf. There should be a way for the compiler to handle this for us but I will probably need some time to really digest everything running through my head before I can give a better response. I'll use this issue as a place to organize my thoughts as things solidify a bit more.

@AlecAivazis
Copy link
Collaborator

AlecAivazis commented Sep 22, 2023

Okay so i couldn't figure out how the composition could automatically work and still maintain valid graphql. For example, if a parent query has 2 fields that both point to User, how does the compiler know which field to mix the fragment into? If we were willing to live outside of the graphql-spec, we could do something like isograph's resolver API but without that, it's hard to see.

edit: removed a bunch of content, the more I thought about this. the more I felt like I was hijacking your issue for something different. I'm still very interested in seeing what we can do to streamline this part of the fragment story

@AlecAivazis AlecAivazis added the Enhancement A new feature or improvement to Houdini's public API label Sep 23, 2023
@chestercharles
Copy link
Author

chestercharles commented Sep 23, 2023

if a parent query has 2 fields that both point to User, how does the compiler know which field to mix the fragment into?

Since the compiler passes the page query's result as a prop into the page component, it seems like it could trace where the value being passed into the child component's prop comes from, and determine that value's corresponding field in the page query.

So given two component with fragments on User:

export function AuthorCard(props: { author: AuthorInfo }) {
  const author = useFragment(props.author, graphql(`
      fragment AuthorInfo on User {
          favoriteWord
      }
  `))
  return <div>{author.favoriteWord}</div>
}
export function IllustratorCard(props: { illustrator: IllustratorInfo }) {
  const illustrator = useFragment(props.illustrator, graphql(`
      fragment IllustratorInfo on User {
          favoriteColor
      }
  `))
  return <div>{illustrator.favoriteColor}</div>
}

In the parent component, the compiler can see that book.author is passed to AuthorCard, so the AuthorInfo fragment should be applied to the book.author field, not the book.illustrator field.

export default function BookCredits({ BookInfo }) {
    return (
        <AuthorCard author={BookInfo.book.author} />
        <IllustratorCard illustrator={BookInfo.book.illustrator} />
    )
}

If the graph type of the value being passed to the prop doesn't overlap with the graph type the fragment expected, then I would imagine there would be some kind of compiler error.

In the +page.gql file, there would be no reference to the child fragments at all - just because the child components are rendered and their props match up with the values being passed in, those fragments would be applied.

However, I think I'm beginning to understand what you mean about maintaining valid GraphQL. If the fragments are automatically applied, then I'm not sure how to maintain valid GraphQL without selecting any fields in the parent query.

You could do something hacky like add __typename or something, but that doesn't feel great.

query BookInfo($id: ID!) {
  book(id: $id) {
    author {
      __typename
    }
    illustrator {
      __typename
    }
  }
}

There would have to be a cleaner way to write the page query.

But does this seem plausible, or do you still see some ambiguity?

@AlecAivazis
Copy link
Collaborator

Ah yea that could work in principle but I'm concerned about supporting the wide range of things that a user could do. The examples you show would be relatively straight forward but it would get tricky once you start including ternaries, hooks that could return fragments, etc.

My first thought about the __typename was to use some fragment that was sort of like an opt-in but that kind of defeats the purpose

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement A new feature or improvement to Houdini's public API
Projects
None yet
Development

No branches or pull requests

2 participants