[Modern] mutation underfetching/overfetching

[jardakotesovec]

I know that concept of fatQuery is not in modern. But I struggle how to organize the schemas in mutations to make sure I am not under/over fetching. When some object is just updated, its safe just to ask for everything that might have changed.. and thats fine most of the time - in worst case its just some overfetching.

But what if new object is added to the list? How do I know which components are using which fields? And what happens if I don't fetch all relevant fields which some component requires - does it show broken data or somehow refetch?

What strategy you use, just ask always for all fields to avoid under fetching?

Sorry not posting this on SO, but I think its actionable and eventually should be closed with some documentation PR.

[julienp]

I'm struggling with the same issue. It's not clear to me what to do here. Even ignoring potential overfetching, if I just fetch everything when a new object is added to a list/connection, it still means I have to remember to modify the mutation fragment AND wherever I use the fragment when fields are added or removed.

[qgerome]

I encounter the same problem (and I guess that at a certain point everyone does). How did you solve that when you removed the Fat Queries ?

[dpehrson]

I've spent about a week working with a new app using relay modern so I can figure it out before upgrading my main web app and this is one of the things that I am also completely lost on.

Is the assumption that every mutation needs to be aware of the data requirements of every component that may fetch fields it could modify? If so, am I supposed to keep these in sync manually or reference dozens (or hundreds) of component fragments in the mutation query?

I feel like there has to be something I am missing because I just can't connect the dots on how I'm supposed to know what data a mutation should be fetching from the mutation response and how to specify/maintain that in a scalable way.

Followup on Oct 1: I am still unfortunately completely lost on this. I'm not sure whether it's a documentation issue, an I'm-a-bit-dense issue, or both, but as someone who has been doing relay classic for about a year, mutations in relay modern are as clear as mud, I can't find any way of expressing my mutations that updates my store properly that will scale.

Followup on Oct 17: Still haven't found any solution to this problem, and at this point I am not aware how it would be possible to build a maintainable application with relay modern. I realize this is likely a limitation of my knowledge rather than the library, but I will likely be giving up and switching back to relay classic until better documentation/guides are available.

[stevehollaar]

This is a current pain-point for us as well in Relay Modern. Specifically when adding a new item to a list in a mutation, we have to manually verify that any fragment container component that renders that item stays in sync with the mutation response.

A concrete example in a real codebase is: this AddComment mutation payload must stay in sync with this Comment fragment container. If a required field is added to the fragment container but not the mutation, the list will be underfetched after the mutation, causing runtime errors. This issue is compounded when there are multiple components rendering the same item, with different data requirements, and multiple mutations that can change it.

[dpehrson]

@stevehollaar I did find out that you can apparently reference component fragments within your mutation query the same way you would while composing component fragments. Unfortunately I have no idea whether this is something that is actually a good idea.

[stevehollaar]

@dpehrson Ya that's true, and it seems like the best option right now, but it's not a complete solution:

• it dictates how you structure your fragments/components, e.g. you would need to create a FragmentContainer for the individual Comment, instead of having a CommentList container (which should be perfectly valid)
• you still need to remember to include all possible fragments in the mutation query

[dpehrson]

@stevehollaar It is also a problem because you may only need to fetch a subset of the component's overall fragment, which means the component needs to split it's fragments into sub-fragments. I had varying luck on getting relay-compiler to play nice with that and backed out.

This is still a completely unsolved problem for me unfortunately which leaves me a bit stuck since relay-classic is no longer supported and I don't understand how to make relay-modern work in a real application.

[jardakotesovec]

My approach (not ideal at all..) so far is for mutations that add new things to ask for all fields. Thats overfetching.. but in most cases thats not big deal and better than miss some field. And to ask for everything I have in one file fragments for all types I need with all fields, so I have only one place where I have to keep in sync with schema. I use these 'full fragments' only for these specific mutations which add new items. When mutation is updating something its usually clear what fields to ask for.

So it looks for example like this:

const mutation = graphql`
    mutation AddWorkspaceMutation($input: addWorkspaceInput!) {
        addWorkspace(input: $input) {
            workspace {
                ...FullFragments_workspace
            }     
        }
    }
`;

This approach works fine if you don't have recursive schema.

And example from FullFragments.js:

const workspaceFragment = graphql`
    fragment FullFragments_workspace on Workspace{
        id
        name
        projects {
            ...FullFragments_project 
        }
    }
`;

[dpehrson]

@jardakotesovec Thanks for providing an example solution to the problem. I tested out something similar and came to the same conclusion.

In the case that more than one component is interested in the new workspace do you just add multiple fragments to the mutation query?

[jardakotesovec]

@dpehrson Nope, with this approach I don't intend to share fragments between components and mutations because of the reasons mentioned above.

I have many components which need something from workspace and each has its own fragment and when I am adding new workspace I just fetch all fields (deeply) regardless what the components really need - so I am overfetching.. but in larger project where its impossible to keep it in sync manually it's relatively ok for time being.

I hope that eventually it will be possible with some static analysis to reduce the fields only to subset which is really used in components fragments. And of course curious to hear how the FB internally deal with this problem and if they are looking to some solutions.

[slicejunk]

until a better approach is found I have defined a new createPaginationContainer that keeps track of all connections (defined with @connection(key: "...") then created a new mutation config:

{
type: 'CONNECTION_REFETCH',
connectionKeys:[{
key: 'myConnectionKey',
filters: {
...
}
}]
}

/* @flow */
import * as React             from 'react';
import Relay                  from 'react-relay';
import PropTypes              from 'prop-types';
import _                      from 'lodash';

const PAGINATION_STEP = 10;

import connectionEventEmitter from './emitter';

type RefetchableConnection = {
  count: number,
  name : string,
};

const connectionEventEmitter = new EventEmitter();

const refetchConnection = (name: string) => {
  connectionEventEmitter.emit(name);
};

const CONNECTION_REFETCH = 'CONNECTION_REFETCH';

const {commitMutation: _commitMutation} = Relay;

export const commitMutation = (environment: Relay.Environment, config: Object): Promise<Object> =>
  _commitMutation(
      environment,
      {
        ...config,
          updater(proxyStore: Object): void {
            if (config.updater != null) {
              config.updater(proxyStore);
            }
            const {configs} = config;
            _.forEach(configs, (item: Object) => {
              if (item.type === CONNECTION_REFETCH) {
                const {connectionKeys} = item;
                _.forEach(connectionKeys, ({key} : {key: string}) => {
                  refetchConnection(key);
                });
              }
            });
          },
        }
      );

const findConnection = (fragment, path) =>
  path.reduce(
    (acc: Object, name: string) =>
      fragment.selections.find(
        (selection) => (selection.alias === name)
      )
    , fragment);

const getFragmentsConnections = (fragments: () => Object): Array<RefetchableConnection> => {
  const fragmentObj = _.mapValues(fragments, (value) => value());
  const connections = [];
  _.forEach(fragmentObj, (fragment: Object) => {
    const {metadata} = fragment;
    if (metadata != null) {
      const {connection} = metadata;
      connection.forEach(
        ({count, path} : {count: string, path: Array<string>}) => {
          const {argumentDefinitions} = fragment;
          const countArgumentDefinition = argumentDefinitions.find(({name}) => (name === count));
          const step =  _.get(countArgumentDefinition, 'defaultValue', PAGINATION_STEP);
          const connection = findConnection(fragment, path);
          const {name} = connection;
          const connectionName = name.replace(/^__/, '').replace(/_connection$/, '');
          connections.push({
            name: connectionName,
            count: step,
          });
        }
      );
    }
  });
  return connections;
};

const decorate = (createContainer: Function) =>
  <Props>(Component: React.ComponentType<any>, fragments: Object, ...args: any) => {
    const BaseContainer = createContainer(Component, fragments, ...args);
    const connections = getFragmentsConnections(fragments);

    return class RelayWithConnectionEmitter extends React.PureComponent<Props, {loading: boolean}> {
      static contextTypes = BaseContainer.contextTypes;

      state = {
        loading: false,
      }

      _subscriptions = [];
      _container;

      componentDidMount(): void {
        this._subscriptions = connections.map(
          ({name, count}) =>
            connectionEventEmitter.addListener(
              name,
              () => this.refetch(count)
            )
        );
      }

      componentWillUnmountMount(): void {
        this._subscriptions.forEach((subscription: Object) => {
          subscription.remove();
        });
      }

      setRef = (ref: any) => {
        this._container = ref;
      }

      refetch = (step: number = PAGINATION_STEP) => {
        this.setState({loading: true}, () => {
          this._container._refetchConnection(
            step,
            () => {
              this.setState({loading: false});
            },
            null, // variables
          );
        });
      }

      render = (): React.Node =>
        <BaseContainer
          {...this.props}
          ref     = {this.setRef}
          loading = {this.state.loading}
        />;
    };
  };

export const createPaginationContainer = decorate(Relay.createPaginationContainer);

export const patchRelay = () => {
  Relay.createPaginationContainer = createPaginationContainer;
  Relay.commitMutation = commitMutation;
};

It would be nice to get comments from @jardakotesovec @julienp @qgerome @dpehrson @stevehollaar

the cons is that it fires new requests after the mutation is completed another approach could be to reimplement the fat query.

[matthewoates]

This is an issue when new nodes are added via subscription updates as well. I was hoping relay would refetch this missing data automatically with the Node interface, but it appears that it doesn't.

The overfetching approach breaks down quickly when these nodes may rely on many different types of associated data.

[mormahr]

Any news on this? And how are you supposed to handle this for more complex types, like types having fields with arguments?

[matthewoates]

According to my friend @alangenfeld who is the manager for the GraphQL team:

most places at FB will either
* if the mutation / subscription is well scoped, fetch only what changed
* if not, refer to UI fragments in the mutations / subscriptions so that you fetch everything you might need (potential overfetch)

Depending on your application, it might also make sense to just load IDs, and use another query-renderer to refetch these entities by ID.

[idris]

@matthewoates thanks for asking your friend! Going off of that second bullet point in your comment.. the ideal situation here would be that it automatically collects all related fragments on the objects that may change in your mutations (similar to how Fat Queries used to work in Relay 1). The issue there is that Apollo and Relay have decided to move to static queries only, and if the query is static, you obviously can't change it based on what's mounted at the time.

Could be interesting to just have a tool that warns you about fragments/fields that your mutation may be under-fetching (by looking at all of the other static queries and comparing to your mutation query).

[matthewoates]

@idris: I'm having trouble thinking through this. Is it possible with the current APIs to know which fragments will be used for new nodes from mutations / subscriptions before that data is available? For instance, if a mutation adds a User, and there's a component that requests that user's followers (specifically that one user), the client doesn't know if it should fetch the followers for that user if it doesn't yet have the ID. You only know if that data is needed once you know the ID of the new data from the mutation/subscription.

Another idea might be to have a component that understands which data should be present, and if that data isn't present, it refetches. That introduces a lot of complexity though. (need to define what data should be available, ensure you don't get into infinite refetching loops, and probably a few other issues) As far as I can tell, this is the only bulletproof solution, even though it does introduce more overhead and complexity.

[mormahr]

I'm currently reading through the experimental "Rendering Partially Cached Data" section and i noticed the following:

To do this, we rely on the ability of fragment containers to suspend. A fragment container will suspend if any of the data it declared locally is missing during render, and is currently being fetched.

As i'm not familiar with the underlying architecture, my question would be if we can tap into this to prevent underfetched data during rendering. There could be a method similar to refetchQuery where you can specify a query to be executed if data is missing. This could also be synthesized for fragments on Node types. I see potential performance problems with sibling components (lists) and child components. Both could be mitigated, with child components probably being the easiest, as i'd just refetch the regular fragment, the same way as you'd do on a refetch or pagination container. Sibling components could be solved by either catching this one level higher (for example at the pagination container) or even generally catching at the query level. I'm not sure if this would work as i suspect that update notifications after the initial fetch are passed from relay to the fragment components directly instead of rerendering the whole QueryRenderer tree. Maybe this could be solved by backtracking or referencing the query via context, if that's not already been done.

My reasoning is that while a mutation should generally fetch all required data, it's sometimes very hard to do this if you have multiple component trees on a single page. Lets say the app has a sidebar with a widget, that creates a node, and inserts this edge into one or multiple connections (different filters, etc.), and the main content view displays a list backed by such a connection. This can now lead to errors if you don't fetch the list row's fragment in the widget mutation. This problem is even worse for associated fields and connections on the mutated node. It's not really feasable to fetch all possible connections / args / associated fields that might be rendered anywhere in the app. Lets say i create a new customer entity via a shortcut with an overlay UI and the main content view beneath it displays a list of customers with a column for how much revenue they made in a specific interval. Now i'd have to keep track of the parameters of that other unrelated view to know what to fetch, and for slow mutations this could even change while the mutation is executing.

Ideally a QueryRenderer would have a setting / policy that tells it to refetch in case the new data is no longer covering the query. This could also be useful for subscriptions. Another possibility would be to at least throw a specific exception if data is missing in a fragment, because right now it just generates random js property access exceptions that are hard to track and don't allow for programatically catching and refetching at the query-level, to implement this in userland.

To reference an earlier comment in this thead @matthewoates:

and use another query-renderer to refetch these entities by ID.

How is that exactly handled / what do you mean by it exactly?

ensure you don't get into infinite refetching loops

How would you get into infinite refetching loops with this approach? Load A triggers load B, and the underlying data changed in between, so load B causes load A to refetch again, or am i missing something?

[sibelius]

Relay 8 (https://github.com/facebook/relay/releases/tag/v8.0.0) let you invalidate a record (https://relay.dev/docs/en/relay-store#invalidaterecord-void), so relay would refetch it when needed

[akomm]

Does it also work with removal? Tried it with removal and the removed record was neither removed from store, nor unlinked.

@sibelius I think its not working like that. The docs state:

This will cause any query that references this record to be considered stale until the next time it is refetched

Which sounds to me: when you use local-and-network refetch policy, the query will be considered stale, when you EXPLICITLY refetch it, it will not use local cache. Refetch does not happen just because you call invalidate on the record. It just marks queries that use the record for later. So it does not solve the issue discussed here.

There is also the option, that the docs are incorrect and confusing and you are right, but then it must be buggy or not working with removal.

Btw. I am the guy from discord ;)

[wasd171]

We've been using the approach of reusing the UI fragments in the mutation's output and it was manually manageable for some time, but it feels like this wouldn't scale well. One of our products is a fairly heavy dashboard with strongly interconnected entities.

Solutions that I see at the moment are:

0

In case you can safely determine the changes that you mutation introduces (like updating a name) – just fetch those fields + id. However I don't think that this thread would've existed if all our mutations were that simple :)

1

"Complete mutation payload" – like @jardakotesovec has mentioned in https://github.com/facebook/relay/issues/1995#issuecomment-333782978 to keep all fragments organized into "umbrella" full fragments – apart from overfetching it also means that fragment cannot fetch nested fields like

fragment UserCard_user on User {
    id
    name
    avatar {
        id
        url
    }
}

fragment FullFragments_user on User {
    ...UserCard_user
}

fragment FullFragments_image on Image {
    # possible to miss what was requested by UserCard_user!
}

instead it will need to be

fragment UserAvatar_image on Image {
    id
    url
}

fragment UserCard_user on User {
    id
    name
    avatar {
        ...UserAvatar_image
    }
}

fragment FullFragments_user on User {
    ...UserCard_user
}

fragment FullFragments_image on Image {
    ...UserAvatar_image
}

This would force to break everything into tiny components with a ton of fragments, which again will make it difficult to maintain and to not forget to include the fragments into the "umbrella" fragment + I'd expect issues with recursive types.

2

"Reasonable mutation payload + suspense" – do your best to guess the fields that could be modified by your mutation, return this payload, prefer useLazyLoadQuery with Suspence fallback components. Monitor the Suspense loader and in case it becomes rendered you should be able to track the missing data for the corresponding query and add it to the mutation's payload. Downsides:

• Instead of performing static analysis prior to deployment you do it in runtime and in worst case show a loader to your customers.
• Most likely you don't want to refetch the whole query because a field somewhere deep is missing (refetchable fragments + Suspense + automatically fetching the data through node query would've been ideal –   @sibelius is it somehow possible to determine when and because of which fragment partially cached query is being refetched, cancel the request, and fetch it manually through node?), this means that you need to introduce a waterfall of loaders because instead of a single query per page you are maintaining a set of them. Waterfall of loaders could be avoided with rendering partially cached data but it's highly experimental and likely to change + you will still be refetching the whole query which can be heavy

  3

  "Lean mutation payload" – mutations only return data they need to finish the mutation, like whether the user's login was successful. All data updates are handled through subscriptions: lists for sure know what fragment they expect to subscribe to in order to add an item. Break the UI into reasonable blocks and subscribe to their updates using the same fragments that you use to fetch their data though query. In case the UI component that requires additional data (like a popup with a separate query) was never rendered it will never subscribe to the changes = no overfetching. Downsides:

• Data will not be updated instantaneously – could be reasonable for your use case and you can probably determine the data that needs immediate update and include it in the payload
• You will need to carefully orchestrate those subscriptions on the backend – however this feels like an easier task, you can use state machines, mobx, database triggers, etc to somewhat reliably predict when do you need to send an update through the subscription

I couldn't say that I am completely happy with any of those solutions. It's been well over half a year since the previous comment, I hope that someone could share their experience in how they managed to get it working or (equally important!) what approaches did not prove themselves to be production-ready

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[ghost]

+1 just like in the good old days to keep this from going stale.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.