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

[Sketch] Mad science idea for auto-binding @module components #4762

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

captbaritone
Copy link
Contributor

Today if you use 3D you need to pass the fragment you got, and your loader to <MatchContainer /> to render it. Match container looks up the module via your loader, adds the fragment spread to the props you pass to <MatchContainer /> and uses those together to render your component.

There's a major problem with that which is that the props you pass to MatchContainer are not typesafe because it is not currently aware of the prop types of the component(s) it might render.

There are other ways we could address this, but I wanted to explore the idea of having Relay return a pre-bound component for you. I think this is not a good direction for Relay today but some folks were interested so I wanted to share.

Code would end up looking something like this:

 function TodoRootComponent() {
    const data = useLazyLoadQuery(graphql`
    query ModuleAutoBindTestQuery {
      me {
        ...UserNameComponentFragment_user
          # NOTE: I think there's a path toward making the name optional
          @module(name: "UserNameComponent")
          @alias(as: "UserNameComponent")
      }
    }`, {});
    if (data.me == null) {
      return null;
    }

    return <data.me.UserNameComponent greeting="Hello" />;
  }

Pros

  1. We could automatically derive the type for the prebound component and its props in the generated artifact for your query/fragment
  2. You don't have to import the react component, or pass the fragment spread to it as a prop
  3. Maybe all fragments would want to work this way to avoid having to import components
  4. If all fragments worked this way, Relay could automatically opt all conditional fragments into 3D (maybe?)

Cons

There are considerable downsides (Which I think outweigh the pros today) to consider:

  1. Ensuring referential stability of the derived react component across fragment reads would require some tricks
  2. The derived types for the pre-bound component while correct, are very confusing due to the type magic
  3. Hiding the fact that you are importing components is nice, but having imports be explicit has benefits too (tooling, code review, etc)
  4. Unclear exactly how this would work with @match where there may be one of many components that match. How do you make props typesafe there? Require the user to refine which type they got? Require them to all match? Would need to figure that out.
  5. Would require Relay compiler know how to import the @module component. Which would require some contract like fragments and components must be defined in the same file and the component must be the default export. Hard to enforce today since Relay still does not fully parse JS (soon maybe?).

Acknowledgement: While not directly inspired by Isograph, I think this arrives at the same type of API as Isograph, which is cool! @rbalicki2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants