-
Notifications
You must be signed in to change notification settings - Fork 788
Conversation
@@ -21,7 +21,7 @@ describe('App', () => { | |||
const query = gql`query people { allPeople(first: 1) { people { name } } }`; | |||
const data = { allPeople: { people: { name: 'Luke Skywalker' } } }; | |||
const networkInterface = mockNetworkInterface({ request: { query }, result: { data } }); | |||
const client = new ApolloClient({ networkInterface }); | |||
const client = new ApolloClient({ networkInterface, addTypename: false }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do this all over the place and default the network interface to supporting it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the alternative?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought we discussed making mockNetworkInterface
support adding __typename
to results automatically so we didn't have to worry about it in tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, right. I guess this seemed easier for now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I guess it just seems like the sort of change that is hard to back out later. But i'll merge it and do that later today.
@@ -66,8 +65,18 @@ const defaultMapResultToProps = props => props; | |||
const defaultMapPropsToSkip = props => false; | |||
|
|||
// the fields we want to copy over to our data prop | |||
const observableQueryFields = observable => pick(observable, 'variables', | |||
'refetch', 'fetchMore', 'updateQuery', 'startPolling', 'stopPolling'); | |||
function observableQueryFields(observable) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
patch release?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or do we just wait?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can patch. I don't think there's a big cost to more releases right.
Blockers:
Tests passing:
TODO: