Apollo-client: Sometimes the same fragment is added to a query twice

Created on 12 Nov 2016 · 11Comments · Source: apollographql/apollo-client

I have a parent container List that renders a list of child container Items. I also have pagination working (i.e., a fetchMore property that implements an updateQuery function).

However, if I change the value of one of the query variables (e.g., a "text" filter) then paging causes an error (the Item fragment is included twice as below).

From my experience with Relay, the parent query variables should be used to parameterize the fragment, but I don't know if that's relevant here, or possible (e.g., ${Item.getFragment('item', { filter: variables.filter })})

ApolloError: "GraphQL error: There can only be one fragment named "ItemFragment".

query GetItems($text: String, $offset: Int, $count: Int) {
  items(text: $text, offset: $offset, count: $count) {
    id
    ...ItemFragment
    __typename
  }
}

fragment ItemFragment on Item {
  id
  title
  __typename
}

fragment ItemFragment on Item { // Only happens when $text is changed.
  id
  title
  __typename
}

Unrelated, but one disadvantage of the convention to put fragments as static members of the component is you can't reference these from static propTypes {} member functions.

🐞 bug

Source

richburdon

👍1

All 11 comments

Unrelated, but one disadvantage of the convention to put fragments as static members of the component is you can't reference these from static propTypes {} member functions.

Could you file this on react-apollo please?

The above looks like a bug, where we are accidentally appending the fragment twice.

Can you reproduce some of your code here? Where you are running the query and setting the variables, etc?

stubailo on 12 Nov 2016

Thanks @stubailo (other issue here https://github.com/apollostack/react-apollo/issues/323):

Child Item:

export const ItemFragments = {
  item: new Fragment(gql`
    fragment ItemFragment on Item {
      id
      title
    }
  `)
};

export class Item extends React.Component {
  static propTypes = {
    item: ItemFragments.item.propType
  };
  render() {
    let { item } = this.props;
    return (
      <div>{ this.props.title }</div>
    );
  }
}

Parent List:

const GetItemsQuery = gql`
  query GetItems($text: String, $offset: Int, $count: Int) { 
    items(text: $text, offset: $offset, count: $count) {
      id
      ...ItemFragment
    }
  }
`;

  graphql(GetItemsQuery, {
    options: ({ text }) => {
      return {
        fragments: ItemFragments.item.fragments(),
        variables: {
          text: text,
          offset: 0,
          count: 10
        }
      };
    },

    props({ ownProps: { text }, data: { loading, items, fetchMore } }) {
      return {
        loading,
        text,
        items,
        fetchMoreItems() {
          return fetchMore({
            variables: {
              text: text,
              offset: items.length
            },
            updateQuery: (previousResult, { fetchMoreResult }) => {
              return _.assign({}, previousResult, {
                items: [...previousResult.items, ...fetchMoreResult.data.items]
              });
            }
          });
        }
      }
    }
  }),

NOTE: I was expecting to be able to parameterize "...ItemFragment" so that optional parts of the the parent's query can be passed down; for example something like:

  ...ItemFragment({ text })

richburdon on 12 Nov 2016

Temporary workaround:

networkInterface.use([{
  applyMiddleware({ request }, next) {
    let definitions = {};
    request.query.definitions = _.filter(request.query.definitions, (definition) => {
      let name = definition.name.value;
      if (definitions[name]) {
        return false;
      } else {
        definitions[name] = true;
        return true;
      }
    });

    next();
  }
}]);

richburdon on 12 Nov 2016

🎉1

Great workaround.

If you could submit a failing test that would be super helpful as well: https://github.com/apollostack/apollo-client/blob/dc8cc0063d93a8c15a90771bc88183bc058b5e1b/test/fetchMore.ts

stubailo on 12 Nov 2016

OK. I'll try to wrap my head around that and have a stab at it (might be closer to Monday though).

Thanks for the comments.

richburdon on 12 Nov 2016

🎉1

Yeah, we won't be able to get to a fix before Monday either. Thanks for the really detailed report!

stubailo on 12 Nov 2016

I had the same issue. The workaround for me was to explicitly specify the same query and fragments in the call to fetchMore.

The problem is here:

ObservableQuery.prototype.fetchMore = function (fetchMoreOptions) {
        var _this = this;
        return Promise.resolve()
            .then(function () {
            var qid = _this.queryManager.generateQueryId();
            var combinedOptions = null;
            if (fetchMoreOptions.query) {
                combinedOptions = fetchMoreOptions;
            }
            else {
                var variables = assign({}, _this.variables, fetchMoreOptions.variables);
                combinedOptions = assign({}, _this.options, fetchMoreOptions, {
                    variables: variables,
                });
            }

            var fullQuery = getFromAST_1.addFragmentsToDocument(combinedOptions.query, combinedOptions.fragments);
            combinedOptions = assign({}, combinedOptions, {
                query: fullQuery,
                forceFetch: true,
            });
             ........

In the else statement _this.options already contains a full query that includes fragments. So when we call addFragmentsToDocument we add the same fragments again resulting in duplicate fragments which causes validation errors.

Initially, the fragments were already added to the query in the ApolloClient.watchQuery.

When a variable for a query changes the GraphQL.subscribeToQuery is called but since we already have a subscription we do not create a new query but instead call ObservableQuery.setOptionson the existing one that does not reset the options.

yurigenin on 12 Nov 2016

Looks like the fix would be in here: https://github.com/apollostack/apollo-client/blob/372bca9881b374caf56d0cbd0795a88dcc13ec0f/src/queries/getFromAST.ts#L150

stubailo on 14 Nov 2016

@stubailo are you suggesting that we "fix" this by checking whether a fragment with the same name already exists on the query? The other fix would be not to attach it twice in the first place. I'm leaning towards a simple fix for now, because we'll soon want to refactor how we transform queries anyway.