apollographql / react-apollo

:recycle: React integration for Apollo Client
https://www.apollographql.com/docs/react/
MIT License
6.85k stars 789 forks source link

subscribeToMore breaks going from 1.0.0-rc2 --> 1.0.0-rc3 #585

Closed dfee closed 7 years ago

dfee commented 7 years ago

Intended outcome: Return value of updateQuery in subscribeToMore is passed as new props to component.

Actual outcome: Return value is not passed as new props to component.

How to reproduce the issue: Below is some example code from a component that demonstrates the issue. It largely apes the example in the docs.

import React, { Component, PropTypes } from 'react';
import { compose } from 'react-apollo';
import { gql, graphql } from 'react-apollo';
import { socketConnect } from 'socket.io-react';
import cloneDeep from 'lodash/cloneDeep';

import events from './config/events';

const CurrentUserQuery = gql`
query {
    currentUser {
        firstName
        lastName
        pictureUrl
        thumbnailUrl
        location
    }
}
`

const CurrentUserSubscription = gql`
subscription {
    currentUser {
        firstName
        lastName
        pictureUrl
        thumbnailUrl
        location
    }
}
`

class Auth extends Component {
  static propTypes = {
    data: PropTypes.shape({
        loading: PropTypes.bool.isRequired,
        currentUser: PropTypes.shape({
            firstName: PropTypes.string.isRequired,
            lastName: PropTypes.string.isRequired,
            location: PropTypes.string.isRequired,
            pictureUrl: PropTypes.string.isRequired,
        }),
    }),
    socket: PropTypes.object.isRequired,
  };

    shouldComponentUpdate(nextProps, nextState) {
        console.log('pp', this.props);  // runs in 1.0.0-rc2, not in 1.0.0-rc3
        console.log('np', nextProps);
        return true;
    }

  componentWillReceiveProps(newProps) {
    console.info('newProps', newProps); // runs in 1.0.0-rc2, not in 1.0.0-rc3
    if (newProps.data.loading) return;
    if (this.subscription) return;

    this.subscription = newProps.data.subscribeToMore({
      document: CurrentUserSubscription,
      variables: {},

      // this is where the magic happens.
      updateQuery: (previousResult, { subscriptionData }) => {
        const newResult = cloneDeep(previousResult); // never mutate state!
        newResult.currentUser = subscriptionData.data.currentUser;
        console.info('newResult', newResult);
        console.info(`pr == nr: ${previousResult === newResult}`);
        return newResult;
      },
      onError: (err) => console.error(err),
    });

  }

  onLogout = () => {
    localStorage.removeItem('token');
    const tokenSetEvent = new CustomEvent(events.token_set);
    dispatchEvent(tokenSetEvent);
  }

  render() {
    console.info('render props:', this.props);  // runs a second time in 1.0.0-rc2, not in 1.0.0-rc3
    const { currentUser, loading } = this.props.data;
    if (loading === true) return <div>Loading...</div>;
    if (!!currentUser) {
      return (
        <div>
            <img alt='userPhoto' src={currentUser.thumbnailUrl}/>
            <div>
                <span>{currentUser.firstName} {currentUser.lastName}</span>
                <br/>
                <span>{currentUser.location}</span>
                <br/>
                <button onClick={this.onLogout}>Logout</button>
            </div>
        </div>
      )
    }
    else {
      // dummy code for the report
      return <button>Login</button>;
    }
  }
}

export default compose (
    graphql(CurrentUserQuery),
    socketConnect,
)(Auth);
helfer commented 7 years ago

@dfee: could you provide a reproduction using react-apollo-error-template? That would be much easier for us to run.

dfee commented 7 years ago

I'm still working through this, and I've cleaned up some things on my end… but my challenge has cleared up a little bit. I'm not sure it's related to a particular release (as mentioned above).

The problem that's persisting comes from trying to update relay style pages. The store is receiving the updates (according to symbols being displayed in logging).

I'll continue to push forward on getting relay to work in the localized environment – so you can take a look.

dfee commented 7 years ago

@helfer I've pretty much reached a dead end. I've got an example where I can extend edges from a relay-style store: https://github.com/dfee/react-apollo-error-template/tree/relay

However, I can't get this same code to work in my actual project. And, I can't figure out why. The updateQuery is indeed called, and I can see that the newResult looks good.

  componentWillReceiveProps(newProps) {
    if (!newProps.data.loading && !this.subscription) {
      this.subscription = newProps.data.subscribeToMore({
        document: STREAM_EVENT_SUBSCRIPTION,
        updateQuery: (previousResult, { subscriptionData }) => {
          const newResult = update(previousResult, {
            stream: {
              activities: {
                edges: {
                  $push: subscriptionData.data.newStreamActivity.edges,
                  // $set: [], //the only thing that actually works
                  // $splice: [[1,5]], // updates the store (as seen on the next call), but props don't change
                  // $push: [{  // also fails
                    // __typename: 'StreamActivityEdge',
                    // node: subscriptionData.data.newStreamActivity,
                  // }],
                }
              }
            }
          });
          debugger;
          return newResult;
        },
        onError: (err) => console.error(err),
      });
    }
  }

I can't figure out if this is because I'm using a Union type, Apollo needs an ID on activities. And, I can't understand why I can empty out the edges array by setting it to [], and that's the only thing that triggers new Props to be sent to the component.

For the sake of completeness, this is the component:

import update from 'immutability-helper';
import React, { Component, PropTypes } from 'react';
import { compose } from 'react-apollo';
import { gql, graphql } from 'react-apollo';

const STREAM_EVENT_QUERY = gql`
query {
  stream {
    id
    activities(last:10) {
      totalCount
      pageInfo {
        hasNextPage
        hasPreviousPage
        startCursor
        endCursor
      }
      edges {
        cursor
        node {
          __typename
          ... on Action {
            id
            variety
            createdAt
            user {
              id
              firstName
              lastName
            }
          }
          ... on Message {
            id
            text
            createdAt
            user {
              id
              firstName
              lastName
            }
          }
          ... on Task {
            id
            text
            createdAt
            user {
              id
              firstName
              lastName
            }
          }
        }
      }
    }
  }
}
`

const STREAM_EVENT_SUBSCRIPTION = gql`
subscription {
  newStreamActivity {
    __typename
    edges {
      cursor
      node {
        __typename
        ... on Action {
          id
          variety
          createdAt
          user {
            id
            firstName
            lastName
          }
        }
        ... on Message {
          id
          text
          createdAt
          user {
            id
            firstName
            lastName
          }
        }
        ... on Task {
          id
          text
          createdAt
          user {
            id
            firstName
            lastName
          }
        }
      }
    }
  }
}
`

class Stream extends Component {
  static propTypes = {
    data: PropTypes.shape({
      loading: PropTypes.bool.isRequired,
      stream: PropTypes.shape({
        id: PropTypes.string.isRequired,
        activities: PropTypes.shape({
          totalCount: PropTypes.number.isRequired,
          pageInfo: PropTypes.shape({
            hasNextPage: PropTypes.bool.isRequired,
            hasPreviousPage: PropTypes.bool.isRequired,
            startCursor: PropTypes.string.isRequired,
            endCursor: PropTypes.string.isRequired,
          }).isRequired,
          edges: PropTypes.arrayOf(
              PropTypes.shape({
                __typename: PropTypes.string.isRequired,
                cursor: PropTypes.string.isRequired,
                node: PropTypes.oneOfType([
                  PropTypes.shape({  // Action
                    __typename: PropTypes.string.isRequired,
                    id: PropTypes.string.isRequired,
                    variety: PropTypes.string.isRequired,
                    createdAt: PropTypes.string.isRequired,
                    user: PropTypes.shape({
                      id: PropTypes.string.isRequired,
                      firstName: PropTypes.string.isRequired,
                      lastName: PropTypes.string.isRequired,
                    }).isRequired,
                  }),
                  PropTypes.shape({  // Message
                    __typename: PropTypes.string.isRequired,
                    id: PropTypes.string.isRequired,
                    text: PropTypes.string.isRequired,
                    createdAt: PropTypes.string.isRequired,
                    user: PropTypes.shape({
                      id: PropTypes.string.isRequired,
                      firstName: PropTypes.string.isRequired,
                      lastName: PropTypes.string.isRequired,
                    }).isRequired,
                  }),
                  PropTypes.shape({  // Action
                    __typename: PropTypes.string.isRequired,
                    id: PropTypes.string.isRequired,
                    text: PropTypes.string.isRequired,
                    createdAt: PropTypes.string.isRequired,
                    user: PropTypes.shape({
                      id: PropTypes.string.isRequired,
                      firstName: PropTypes.string.isRequired,
                      lastName: PropTypes.string.isRequired,
                    }).isRequired,
                  }),
                ]).isRequired,
              }),
            ).isRequired,
          }),
        })
    }),
  };

  constructor(props) {
    super(props);
    this.subscription = null;
  }

  componentWillReceiveProps(newProps) {
    if (!newProps.data.loading && !this.subscription) {
      this.subscription = newProps.data.subscribeToMore({
        document: STREAM_EVENT_SUBSCRIPTION,
        updateQuery: (previousResult, { subscriptionData }) => {
          const newResult = update(previousResult, {
            stream: {
              activities: {
                edges: {
                  $push: subscriptionData.data.newStreamActivity.edges,
                  // $push: [{
                    // __typename: 'StreamActivityEdge',
                    // node: subscriptionData.data.newStreamActivity,
                  // }],
                }
              }
            }
          });
          debugger;
          return newResult;
        },
        onError: (err) => console.error(err),
      });
    }
  }

  renderActivity = (activity) => {
    const createdAt = new Date(activity.createdAt);
    const name = `${activity.user.firstName} ${activity.user.lastName}`
    if (activity.__typename === 'Action') {
      const renderedActivity = activity.variety === 'join' ? 'joined' : 'unknowned';
      return (
        <li key={activity.id}>
          {name} {renderedActivity} on {createdAt.toUTCString()}
        </li>
      )
    }
    else if (activity.__typename === 'Task') {
      return (
        <li key={activity.id}>
          {name} is working on {activity.text} at {createdAt.toUTCString()}
        </li>
      )
    }
  }

  render() {
    const { stream, loading } = this.props.data;
    if (loading === true) return <div>Loading...</div>;
    return (
      <ul>
        { stream.activities.edges.map(edge => this.renderActivity(edge.node)) }
      </ul>
    );
  }
}

export default compose (
    graphql(STREAM_EVENT_QUERY),
)(Stream);
dfee commented 7 years ago

I've pretty much ruled out that activities doesn't need an id. Or, at least that's not the problem.

dfee commented 7 years ago

I was able to find the bug (finally!). I'm closing this as I can point to a more specific issue: https://github.com/apollographql/apollo-client/issues/1555

helfer commented 7 years ago

Thanks for the reproduction @dfee! Let's get it fixed asap! :)

helfer commented 7 years ago

Btw, this could have something to do with Union types, if that's what you're using in your queries.

dfee commented 7 years ago

I am using Union types in my query. Is that problematic?