-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Use of GraphQL interfaces causes component to always fetch data from server even if it should already be cached in redux #933
Comments
@TSMMark We'll need a bit more information than that to figure out what's going on. I assume you didn't somehow turn on |
Not sure what you'd need so here's the whole file: // @flow
import React, { Component } from 'react'
import { ListView, View } from 'react-native'
import StatusUpdatedNotification from './notifications/StatusUpdatedNotification'
import NewAdminMessageNotification from './notifications/NewAdminMessageNotification'
import A2VConversionFailedNotification from './notifications/A2VConversionFailedNotification'
import ArtistUpdatedNotification from './notifications/ArtistUpdatedNotification'
import AuthErrorNotification from './notifications/AuthErrorNotification'
import DeploymentConfirmedNotification from './notifications/DeploymentConfirmedNotification'
import DeploymentScheduledNotification from './notifications/DeploymentScheduledNotification'
import MergeCompletedNotification from './notifications/MergeCompletedNotification'
import MergeConflictNotification from './notifications/MergeConflictNotification'
import NewMergeRequestNotification from './notifications/NewMergeRequestNotification'
import NewMessageNotification from './notifications/NewMessageNotification'
import ProcessingFinishedNotification from './notifications/ProcessingFinishedNotification'
import PublisherNewCommentNotification from './notifications/PublisherNewCommentNotification'
import PublisherNewVideoRequestedNotification from './notifications/PublisherNewVideoRequestedNotification'
import PublisherNewVideoSubmittedNotification from './notifications/PublisherNewVideoSubmittedNotification'
import UploadFailedNotification from './notifications/UploadFailedNotification'
import UploadFinishedNotification from './notifications/UploadFinishedNotification'
import { registerComponent } from '../lib/utils'
import { graphql, compose } from 'react-apollo'
import I18n from 'react-native-i18n'
import Button from './Button' // TODO
import gql from 'graphql-tag'
const NOTIFICATION_COMPONENT_MAP = {
'status_updated': StatusUpdatedNotification,
'new_admin_message': NewAdminMessageNotification,
'a2v_conversion_failed': A2VConversionFailedNotification,
'artist_updated': ArtistUpdatedNotification,
'auth_error': AuthErrorNotification,
'deployment_confirmed': DeploymentConfirmedNotification,
'deployment_scheduled': DeploymentScheduledNotification,
'merge_completed': MergeCompletedNotification,
'merge_conflict': MergeConflictNotification,
'new_merge_request': NewMergeRequestNotification,
'new_message': NewMessageNotification,
'processing_finished': ProcessingFinishedNotification,
'publisher_new_comment': PublisherNewCommentNotification,
'publisher_new_video_requested': PublisherNewVideoRequestedNotification,
'publisher_new_video_submitted': PublisherNewVideoSubmittedNotification,
'upload_failed': UploadFailedNotification,
'upload_finished': UploadFinishedNotification
}
type Props = {
data: Object
}
export class Notifications extends Component {
state: {
dataSource: ListView.DataSource
}
constructor(props: Props) {
super(props)
this.state = {
dataSource: new ListView.DataSource({ rowHasChanged: (r1, r2) => r1 !== r2 })
}
}
componentWillReceiveProps = (nextProps: Props) => {
const { notifications: nextNotifications } = nextProps
const { notifications: prevNotifications } = this.props
if (nextNotifications !== prevNotifications) {
this.updateStateWithProps(nextProps)
}
}
updateStateWithProps = (props: Props) => {
const { notifications, loading } = props
if (loading) {
return
}
this.setState({
dataSource: this.state.dataSource.cloneWithRows(notifications)
})
}
renderNotificationRow = (notification: Object) => {
const NotificationComponent = NOTIFICATION_COMPONENT_MAP[notification.type]
return <NotificationComponent notification={notification} />
}
render = () => {
return (
<View>
<ListView
enableEmptySections={true}
dataSource={this.state.dataSource}
renderRow={rowData => this.renderNotificationRow(rowData)}
/>
<Button
title={I18n.t('Notifications.loadMore')}
onPress={this.props.loadMoreNotifications}
/>
</View>
)
}
static translations = () => ({
en: {
title: 'Notifications',
loadMore: 'Load More'
}
})
}
const query = gql`query ($offset: Int, $limit: Int) {
notifications(offset: $offset, limit: $limit) {
id
type
... on A2VConversionFailed {
song_name
artist_name
}
... on ArtistUpdated {
user_name
artist_name
}
... on AuthError {
broadcast {
unauthorized
failed
name
network {
name
}
release {
file_song_name
id_name
artist {
name
}
}
}
}
... on DeploymentConfirmed {
broadcast {
network {
name
}
release {
file_song_name
artist {
name
}
}
}
}
... on DeploymentScheduled {
broadcast {
pretty_scheduled_time
network {
name
}
release {
file_song_name
artist {
name
}
}
}
}
... on MergeCompleted {
old_account
new_account
}
... on MergeConflict {
old_account
new_account
}
... on NewAdminMessage {
message_author_name
release {
id
file_song_name
artist {
name
}
}
}
... on NewMergeRequest {
old_account
new_account
}
... on NewMessage {
message_author_name
release {
id_name
file_song_name
artist {
name
}
}
}
... on ProcessingFinished {
media {
file_song_name
artist {
name
}
}
}
... on PublisherNewComment {
content
user_name
media {
file_song_name
artist {
name
}
}
}
... on PublisherNewVideoRequested {
user_name
media {
file_song_name
artist {
name
}
}
}
... on PublisherNewVideoSubmitted {
media {
file_song_name
artist {
name
}
}
}
... on StatusUpdated {
release {
id
id_name
file_song_name
artist {
name
}
}
}
... on UploadFailed {
max_file_size
}
... on UploadFinished {
media {
file_song_name
artist {
name
}
}
}
}
}`
const queryOptions = {
options: () => ({
variables: {
offset: 0,
limit: 1
},
// forceFetch: true, // TODO: The examples have this but do we need it?
pollInterval: 20 * 60 * 1000 // Auto-re-fetch every 20 minutes. TODO: Convention?
}),
props({ data: { loading, notifications, fetchMore } }) {
return {
loading,
notifications,
loadMoreNotifications() {
return fetchMore({
// query: ... (you can specify a different query. FEED_QUERY is used by default)
variables: {
// We are able to figure out which offset to use because it matches
// the feed length, but we could also use state, or the previous
// variables to calculate this (see the cursor example below)
offset: notifications.length
},
updateQuery: (previousResult, { fetchMoreResult }) => {
if (!fetchMoreResult.data) {
return previousResult
}
const notifications = [...previousResult.notifications, ...fetchMoreResult.data.notifications]
return Object.assign({}, previousResult, {
// Append the new feed results to the old one
notifications
})
}
})
}
}
}
}
const ConnectedComponent = compose(
graphql(query, queryOptions)
)(Notifications)
registerComponent(Notifications, ConnectedComponent)
export default ConnectedComponent |
@helfer Is there anything else I could include here to make it easier to figure out what's going on? |
@TSMMark Are the variables the same in both cases? If the variables (like offset, etc.) aren't the same, then Apollo Client will refetch. This is because fetchMore actually doesn't modify the variables of the original query, but only modifies the result. So if your params were |
@helfer A good theory, but that is not what's happening. I even tried hardcoding const queryOptions = {
options: () => ({
variables: {
offset: 0,
limit: 1
}
}),
props({ data: { loading, notifications, fetchMore } }) {
return {
loading,
notifications,
loadMoreNotifications() {
return fetchMore({
variables: {
offset: 0,
limit: 1
},
updateQuery: (previousResult, { fetchMoreResult }) => {
if (!fetchMoreResult.data) {
return previousResult
}
const notifications = [...previousResult.notifications, ...fetchMoreResult.data.notifications]
return Object.assign({}, previousResult, {
notifications
})
}
})
}
}
}
}
const ConnectedComponent = compose(
graphql(query, queryOptions)
)(Notifications) Still makes a request to the server every time the component mounts. |
OK on a hunch I tried the same type of pagination on a simpler gql query to see if that would fetch remote date on every component mount, and it does not! Remote data is fetched initially, then subsequent component mounts do not trigger identical fetches. Based on this experiment, to me it looks like that the issue must have something to do with the specific |
Original post updated with new information. |
@TSMMark this is probably due to a bug in fragment matching, which will make the data in the cache never match. Can you test if the behavior differs if the fragment is on a union, interface or an object type? |
@helfer It definitely seems like a bug in fragment matching. I am not familiar with any other type of fragments other than "interface", but it looks like this blog post will help me https://medium.com/the-graphqlhub/graphql-tour-interfaces-and-unions-7dd5be35de0d#.szp76x3ct I will try to get to this sometime this week. |
@helfer there is a possibility that we are seeing the same thing in our application. Is this a known problem? |
@nnance Yes, that's definitely possible. There are a few corner-cases with interfaces and unions that the heuristic fragment matching doesn't quite get right. We've decided to just implement a fragment matcher that actually has the knowledge to conclusively decide whether a fragment matches or not. That should solve this and related issues. |
@helfer that sounds nice. Any idea when that will become available? |
I am facing the same issue when I am using Relay style cursor pagination. Exact same query always ends up fetching results from the server. My code only uses 'after' and 'first' parameters so that the query stays the same but the results are not getting fetched from the cache when the query runs again. I even tried the IntrospectionFragmentMatcher after installing react-apollo@next and apollo-client@next. It still didn't work! |
This should be fixed thanks to #1483. The introspection fragment matcher must be used for union and interface types. |
Thanks @helfer! Works like a charm. |
I have a component which utilizes pagination, but it fetches data every time the component mounts, even if the data from the exact same query is already cached in redux. Is this expected?Edit: It is not related to pagination, as originally thought. See below.
This gql query fetches data on the first component mount, and uses local data on subsequent renders:
This gql query causes re-fetch every component mount:
Seems like it's probably a problem with apollo client caching implementation for graphql interfaces. I checked the issues and I did not see anything opened about this yet.
The text was updated successfully, but these errors were encountered: