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

Loading data inside of hooks VS loading data outside of hooks? #4618

Open
gajus opened this issue Feb 15, 2024 · 1 comment
Open

Loading data inside of hooks VS loading data outside of hooks? #4618

gajus opened this issue Feb 15, 2024 · 1 comment

Comments

@gajus
Copy link

gajus commented Feb 15, 2024

Related Loom discussing an internal PR: https://www.loom.com/share/670823d870084f7b920d368802463761

Would love to have some input from Relay's team about the best practices here.

TLDR is that we want to standardize how we fetch data and we are debating between 2 patterns:

Loading data inside of hooks

export const useCopyProfileUrl = ({
  userProfileRef,
  utmTag,
}: {
  userProfileRef: useCopyProfileUrl_userProfile$key;
  utmTag?: {
    utm_campaign: string;
    utm_medium: string;
    utm_source: string;
  };
}) => {
  const userProfile = useFragment<useCopyProfileUrl_userProfile$key>(
    graphql`
      fragment useCopyProfileUrl_userProfile on UserProfile {
        displayUsername
      }
    `,
    userProfileRef,
  );
  // [..]
  • Pro: data fetching is co-localized with consumption. In theory, if Relay ESLint plugin worked, this would provide us protection against data over-fetching.
  • Con: need to add useCopyProfileUrl_userProfile fragment to every parent data loader/fragment
  • Con: additional suspense may cause unexpected issues with state management [components losing internal state]
  • Con: a bit harder to test

Loading data outside of hooks

const profile = useFragment<ShareProfileMenuButton_userProfile$key>(
  graphql`
    fragment ShareProfileMenuButton_userProfile on UserProfile {
      displayUsername
    }
  `,
  userProfileRef,
);

const { onCopyProfileLink } = useCopyProfileUrl({
  userProfile: {
    displayUsername: profile.displayUsername,
  },
  utmTag: {
    utm_campaign: 'social_sharing',
    utm_medium: 'independent_share',
    utm_source: 'copy_link',
  },
});
  • Pro: encapsulation
  • Pro: portability
  • Pro: ease of testing
  • Con: the need to explicitly map every property (if we want to retain static analyzes for over-fetching protection)
  • Con: easy to over-fetch since the former cannot be enforced

What's the recommended path and what are your arguments for it?

@kawazoe
Copy link

kawazoe commented May 29, 2024

I'm currently working on a medium scale application that uses the first pattern. This is arguably the "correct" way of using Relay. You will want to push this pattern as far as it brings you benefits.

Here's an idea of how far our team pushed it.

In our app, we have a form that lets you edit data from a graphql query. This form spans dozens of individual routes, has a completely different UI on mobile, and includes well over 200 fields, all with their own validations and invariant to respect. Each field get its own hook, with its own fragment to fetch the initial value, setup validations, getter/setters, and render function. Some fields request a single value in their fragment. Others might manage entire collections of fields, themselves containing data and fragments for other fields to use. Something like this:

function useFieldA(key) {
  const data = useFragment(graphql`fragment FieldA { a }`, key);
  // setup field state
  // setup additional getter/setters
  // setup validations
  // setup a render function
  return { the field's public API };
}

Any time a business rule or validation needs to interact with the value of multiple fields, we create a new hook to name that invariant. This hook have its own fragment which composes the other two from the fields, and adds the necessary logic for the invariant.

function useFieldABiggerThanB(key) {
  const data = useFragment(graphql`fragment FieldABiggerThanB { ...FieldA ...FieldB }`, key);
  const { value: a } = useFieldA(data);
  const { value: b } = useFieldB(data);

  return a > b; // or maybe trigger a query in an effect that sets another field... who knows!
}

The advantages for this architecture is massive. It means that we can safely add more complexity, encapsulted into their own functions, without changing existing code and thus risk breaking things. It means tests are hyper focused to the rule described by the given hook, and only query for the data they need. It also means that anyone can use the public API of any field, at any point in the multiple pages our form spans, and they can be sure that all invariants garantied by the hook will be respected.

Because fields are initialized with a fragment, you need to provide it every time you want to use the field:

const data = useFragment(graphql`fragment Comp { ...FieldA ...FieldB ...FieldAnB }`);
const { render: a } = useFieldA(data);
const { render: b } = useFieldB(data);
const makeBold = useFieldAnB(data);

return (
  <div>
    {a()}
    {b(makeBold)}
  </div>
);

You could put the call to useFieldAnB in the middle and it wouldn't change a thing. In fact, you can even call it twice and it still wouldn't change a thing. You could even call it in an other page, if that's where you need the data, and never explicitely call useFieldA or useFieldB, and it still wouldn't change a thing. This is because that fragment garanties that the field always have all the data it needs to initialize its state and check its invariants.

Most importantly, and this is why I said this is the "correct" way of using relay, it also means that no component is aware of the entire schema of the form! They are only aware of their dependencies, but not the details of those dependencies. This is literally DI for your data where fragment keys acts as interfaces. If you have a page that uses 8 components, each with their own fragment, using 12 more components and hooks each, again with their own fragments, each with 4 more hooks, you are hiding 440 dependencies from that top level page. This is huge! That's 440 less reasons for that page to break due to involuntary coupling, just because you used fragments all the way down to each of your fields. Now imagine if you add another layer of components and hooks to the pile...

With that being said, it's not roses all the way down. There are popular tools that you might want to use, like formik, react-hook-form, or yop, which will simply not work with this architecture. All of them suffer from the same problem: they require the code that initialize them to have complete knowledge of the schema of the form. This means that they fundamentally require a single function to know all of the 448 dependencies; typically either to describe a type/schema, default values, on submit callbacks. We had to let them go, and instead pivot to our own in-house form tooling to support this model which took a lot of effort to get right.

TLDR: Use the first pattern until it causes you more pain than it brings benefits. If you can't spend weeks building your own tooling to support this pattern, don't fight the paradigm of the tool you want to use, pick the other pattern. If you can, then you'll never believe how easy it is to maintain your code, and how fast you get to add new features in such a decoupled codebase.

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