From bba5992255c72b14523b22a3562d30bd5574b19a Mon Sep 17 00:00:00 2001 From: Tom Coleman Date: Mon, 12 Sep 2016 19:31:57 +1000 Subject: [PATCH 01/14] Fix variable name in test --- test/react-web/client/graphql/queries-1.test.tsx | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/test/react-web/client/graphql/queries-1.test.tsx b/test/react-web/client/graphql/queries-1.test.tsx index 3714afdd26..03cdfb403e 100644 --- a/test/react-web/client/graphql/queries-1.test.tsx +++ b/test/react-web/client/graphql/queries-1.test.tsx @@ -735,7 +735,7 @@ describe('queries', () => { ); const client = new ApolloClient({ networkInterface }); - let isRefectching; + let isRefetching; let refetched; @graphql(query) class Container extends React.Component { @@ -749,16 +749,16 @@ describe('queries', () => { } // don't remove old data - if (isRefectching) { - isRefectching = false; + if (isRefetching) { + isRefetching = false; refetched = true; expect(props.data.loading).toBe(true); expect(props.data.allPeople).toEqual(data.allPeople); return; } - if (!isRefectching) { - isRefectching = true; + if (!isRefetching) { + isRefetching = true; props.data.refetch(); } } From a4e4aa5e344718e76e235951ae81b9b1f7ab511f Mon Sep 17 00:00:00 2001 From: Tom Coleman Date: Mon, 12 Sep 2016 19:32:08 +1000 Subject: [PATCH 02/14] Add testonly command --- package.json | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/package.json b/package.json index 0ec087b7c2..be83c99de4 100644 --- a/package.json +++ b/package.json @@ -5,7 +5,8 @@ "main": "index.js", "typings": "index.d.ts", "scripts": { - "test": "jest", + "test": "npm run testonly", + "testonly": "jest", "posttest": "npm run lint", "filesize": "npm run compile:browser && ./scripts/filesize.js --file=./dist/index.min.js --maxGzip=15", "compile": "tsc", From 9079fb9c9bb3867d6076f9c1cd63598d5518af2f Mon Sep 17 00:00:00 2001 From: Tom Coleman Date: Mon, 12 Sep 2016 19:32:15 +1000 Subject: [PATCH 03/14] Bring across changed to mockedNetworkInterface From https://github.com/apollostack/apollo-client/commit/3779572a21f90cd149727d13300df144ca7c80e2 --- test/mocks/mockNetworkInterface.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/mocks/mockNetworkInterface.ts b/test/mocks/mockNetworkInterface.ts index 7f8e63fe45..15f8e9c944 100644 --- a/test/mocks/mockNetworkInterface.ts +++ b/test/mocks/mockNetworkInterface.ts @@ -90,7 +90,7 @@ export class MockNetworkInterface implements NetworkInterface { function requestToKey(request: ParsedRequest): string { const queryString = request.query && print(request.query); return JSON.stringify({ - variables: request.variables, + variables: request.variables || {}, debugName: request.debugName, query: queryString, }); From fe2dcb54101ee5ebc7aee704cbdd102d389403d6 Mon Sep 17 00:00:00 2001 From: Tom Coleman Date: Tue, 13 Sep 2016 15:24:37 +1000 Subject: [PATCH 04/14] Enable source maps in tests --- test/index.ts | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 test/index.ts diff --git a/test/index.ts b/test/index.ts new file mode 100644 index 0000000000..bc48ba0334 --- /dev/null +++ b/test/index.ts @@ -0,0 +1,3 @@ +require('source-map-support').install({ + environment: 'node', +}); From 87d4b38b4c551343a5c941dea37a82da00a79001 Mon Sep 17 00:00:00 2001 From: Tom Coleman Date: Wed, 14 Sep 2016 17:54:12 +1000 Subject: [PATCH 05/14] Add lodash.pick to dependencies. --- package.json | 1 + typings.d.ts | 5 +++++ 2 files changed, 6 insertions(+) diff --git a/package.json b/package.json index be83c99de4..3ba55d00d4 100644 --- a/package.json +++ b/package.json @@ -114,6 +114,7 @@ "lodash.flatten": "^4.2.0", "lodash.isequal": "^4.1.1", "lodash.isobject": "^3.0.2", + "lodash.pick": "^4.4.0", "object-assign": "^4.0.1", "recompose": "^0.20.2", "typed-graphql": "^1.0.2" diff --git a/typings.d.ts b/typings.d.ts index b8e52afe4b..192c37f025 100644 --- a/typings.d.ts +++ b/typings.d.ts @@ -19,6 +19,11 @@ declare module 'lodash.flatten' { export = main; } +declare module 'lodash.pick' { + import main = require('lodash'); + export = main.pick; +} + declare module 'recompose/compose' { function hoc(component: any): any; export default (...hocs) => hoc; From d81ad0061c7c51a99c7176fe6d9a41c85505551b Mon Sep 17 00:00:00 2001 From: Tom Coleman Date: Wed, 14 Sep 2016 17:58:30 +1000 Subject: [PATCH 06/14] First pass at refactor (WIP), skipping some tests --- src/graphql.tsx | 223 +++++------------- .../client/graphql/queries-1.test.tsx | 121 +++------- .../client/graphql/queries-2.test.tsx | 68 ++++++ 3 files changed, 166 insertions(+), 246 deletions(-) diff --git a/src/graphql.tsx b/src/graphql.tsx index af246a2d46..0e712c3808 100644 --- a/src/graphql.tsx +++ b/src/graphql.tsx @@ -1,4 +1,3 @@ - import { Component, createElement, @@ -8,6 +7,7 @@ import { // modules don't export ES6 modules import isEqual = require('lodash.isequal'); import flatten = require('lodash.flatten'); +import pick = require('lodash.pick'); import shallowEqual from './shallowEqual'; import invariant = require('invariant'); @@ -63,6 +63,10 @@ const defaultQueryData = { const defaultMapPropsToOptions = props => ({}); const defaultMapResultToProps = props => props; +// the fields we want to copy over to our data prop +const observableQueryFields = observable => pick(observable, 'variables', + 'refetch', 'fetchMore', 'updateQuery', 'startPolling', 'stopPolling'); + function getDisplayName(WrappedComponent) { return WrappedComponent.displayName || WrappedComponent.name || 'Component'; } @@ -134,6 +138,7 @@ export default function graphql( const graphQLDisplayName = `Apollo(${getDisplayName(WrappedComponent)})`; + // XXX: what is up with this? We shouldn't have to do this. function calculateFragments(fragments): FragmentDefinition[] { if (!fragments && !operation.fragments.length) { return fragments; @@ -153,6 +158,11 @@ export default function graphql( newOpts.variables = assign({}, opts.variables, newOpts.variables); } if (newOpts) opts = assign({}, opts, newOpts); + + if (opts.fragments) { + opts.fragments = calculateFragments(opts.fragments); + } + if (opts.variables || !operation.variables.length) return opts; let variables = {}; @@ -187,7 +197,6 @@ export default function graphql( if (opts.ssr === false) return false; if (!opts.variables) delete opts.variables; - opts.fragments = calculateFragments(opts.fragments); // if this query is in the store, don't block execution try { @@ -230,8 +239,8 @@ export default function graphql( // private previousQueries: Object; // request / action storage - private queryObservable: ObservableQuery | Object; - private querySubscription: Subscription | Object; + private queryObservable: ObservableQuery; + private querySubscription: Subscription; // calculated switches to control rerenders private haveOwnPropsChanged: boolean; @@ -254,11 +263,8 @@ export default function graphql( this.store = this.client.store; this.type = operation.type; - this.queryObservable = {}; - this.querySubscription = {}; this.setInitialProps(); - } componentDidMount() { @@ -310,144 +316,66 @@ export default function graphql( return; } - const queryOptions = this.calculateOptions(this.props); - const fragments = calculateFragments(queryOptions.fragments); - const { variables, forceFetch, skip } = queryOptions as QueryOptions; + // Create the observable but don't subscribe yet. The query won't + // fire until we do. + const opts: QueryOptions = this.calculateOptions(this.props); + this.data = assign({}, defaultQueryData) as any; - let queryData = assign({}, defaultQueryData) as any; - queryData.variables = variables; - if (skip) queryData.loading = false; + if (opts.skip) { + this.data.loading = false; + } else { + this.createQuery(opts); + } + } - queryData.refetch = (vars) => this.client.query({ + createQuery(opts: QueryOptions) { + this.queryObservable = this.client.watchQuery(assign({ query: document, - variables: vars, - }); - - queryData.fetchMore = (opts) => { - opts.query = document; - return this.client.query(opts); - }; - - // XXX type this better - // this is a stub for early binding of updateQuery before data - queryData.updateQuery = (mapFn: any) => { - invariant(!!(this.queryObservable as ObservableQuery).updateQuery, ` - Update query has been called before query has been created - `); + }, opts)); - (this.queryObservable as ObservableQuery).updateQuery(mapFn); - }; + assign(this.data, observableQueryFields(this.queryObservable)); - if (!forceFetch) { - try { - const result = readQueryFromStore({ - store: this.client.queryManager.getApolloState().data, - query: document, - variables, - fragmentMap: createFragmentMap(fragments), - }); - - queryData = assign(queryData, { errors: null, loading: false }, result); - } catch (e) {/* tslint:disable-line */} + if (!opts.forceFetch) { + // try and fetch initial data from the store + assign(this.data, this.queryObservable.currentResult()); } - - this.data = queryData; } subscribeToQuery(props): boolean { - const { watchQuery } = this.client; const opts = calculateOptions(props) as QueryOptions; - if (opts.skip) return; // don't rerun if nothing has changed if (isEqual(opts, this.previousOpts)) return false; - // if the only thing that changed was the variables, do a refetch instead of a new query - const old = assign({}, this.previousOpts) as any; - const neu = assign({}, opts) as any; - delete old.variables; - delete neu.variables; - - if ( - this.previousOpts && - (!shallowEqual(opts.variables, (this.previousOpts as WatchQueryOptions).variables)) && - (shallowEqual(old, neu)) - ) { - this.hasOperationDataChanged = true; - this.data = assign(this.data, { loading: true, variables: opts.variables }); - this.forceRenderChildren(); - (this.queryObservable as ObservableQuery).refetch(assign( - {}, (this.previousOpts as WatchQueryOptions).variables, opts.variables - )) - .then((result) => { - this.data = assign(this.data, result.data, { loading: false }); - this.hasOperationDataChanged = true; - this.forceRenderChildren(); - return result; - }) - .catch(error => { - this.data = assign(this.data, { loading: false, error }); - this.hasOperationDataChanged = true; - this.forceRenderChildren(); - return error; - }); - this.previousOpts = opts; - return; - } - - this.previousOpts = opts; + // XXX: tear down old subscription if it exists + if (opts.skip) return; - let previousQuery = this.queryObservable as any; - this.unsubscribeFromQuery(); + // XXX: this seems like the wrong function to do this in, should it be in willReceiveProps? + // We've subscribed already, just change stuff. + if (this.querySubscription) { + this.queryObservable.setVariables(opts.variables); - const queryOptions: WatchQueryOptions = assign({ query: document }, opts); - queryOptions.fragments = calculateFragments(queryOptions.fragments); - const observableQuery = watchQuery(queryOptions); - const { queryId } = observableQuery; + // Ensure we are up-to-date with the latest state of the world + assign(this.data, + { loading: this.queryObservable.currentResult().loading }, + observableQueryFields(this.queryObservable)); - // the shape of the query has changed - if (previousQuery.queryId && previousQuery.queryId !== queryId) { - this.data = assign(this.data, { loading: true }); - this.hasOperationDataChanged = true; - this.forceRenderChildren(); + // XXX: if pollingInterval is set, it may have changed, we could called + // this.queryObservable.startPolling here + return; } - this.handleQueryData(observableQuery, queryOptions); - } - - unsubscribeFromQuery() { - if ((this.querySubscription as Subscription).unsubscribe) { - (this.querySubscription as Subscription).unsubscribe(); - delete this.queryObservable; + // if we skipped initially, we may not have yet created the observable + if (!this.queryObservable) { + this.createQuery(opts); } - } - - handleQueryData(observableQuery: ObservableQuery, { variables }: WatchQueryOptions): void { - // bind each handle to updating and rerendering when data - // has been recieved - let refetch, - fetchMore, - startPolling, - stopPolling, - updateQuery, - oldData = {}; + let oldData = {}; const next = ({ data = oldData, loading, error }: any) => { - const { queryId } = observableQuery; - - let initialVariables = this.client.queryManager.getApolloState().queries[queryId].variables; - - const resultKeyConflict: boolean = ( - 'errors' in data || - 'loading' in data || - 'refetch' in data || - 'fetchMore' in data || - 'startPolling' in data || - 'stopPolling' in data || - 'updateQuery' in data - ); - invariant(!resultKeyConflict, + const { queryId } = this.queryObservable; + + invariant(Object.keys(observableQueryFields(data)).length === 0, `the result of the '${graphQLDisplayName}' operation contains keys that ` + `conflict with the return object. 'errors', 'loading', ` + `'startPolling', 'stopPolling', 'fetchMore', 'updateQuery', and 'refetch' cannot be ` + @@ -461,42 +389,15 @@ export default function graphql( // cache the changed data for next check oldData = assign({}, data); + this.data = assign({ - variables: this.data.variables || initialVariables, loading, - refetch, - startPolling, - stopPolling, - fetchMore, error, - updateQuery, - }, data); - - this.forceRenderChildren(); - }; - - const createBoundRefetch = (refetchMethod) => (vars, ...args) => { - let newVariables = vars; - const newData = { loading: true } as any; - if (vars && (vars.variables || vars.query || vars.updateQuery)) { - newVariables = assign({}, this.data.variables, vars.variables); - newData.variables = newVariables; - } - - this.data = assign(this.data, newData); + }, data, observableQueryFields(this.queryObservable)); - this.hasOperationDataChanged = true; this.forceRenderChildren(); - // XXX why doesn't apollo-client fire next on a refetch with the same data? - return refetchMethod(vars, ...args) - .then((result) => { - if (result && isEqual(result.data, oldData)) next(result); - return result; - }); }; - this.queryObservable = observableQuery; - const handleError = (error) => { if (error instanceof ApolloError) return next({ error }); throw error; @@ -509,19 +410,18 @@ export default function graphql( Instead, we subscribe to the store for network errors and re-render that way */ - this.querySubscription = observableQuery.subscribe({ next, error: handleError }); - - refetch = createBoundRefetch((this.queryObservable as any).refetch); - fetchMore = createBoundRefetch((this.queryObservable as any).fetchMore); - startPolling = (this.queryObservable as any).startPolling; - stopPolling = (this.queryObservable as any).stopPolling; - updateQuery = (this.queryObservable as any).updateQuery; + this.querySubscription = this.queryObservable.subscribe({ next, error: handleError }); + // XXX: can remove this? // XXX the tests seem to be keeping the error around? delete this.data.error; - this.data = assign(this.data, { - refetch, startPolling, stopPolling, fetchMore, updateQuery, variables, - }); + } + + unsubscribeFromQuery() { + if ((this.querySubscription as Subscription).unsubscribe) { + (this.querySubscription as Subscription).unsubscribe(); + delete this.queryObservable; + } } forceRenderChildren() { @@ -547,7 +447,6 @@ export default function graphql( if (typeof opts.variables === 'undefined') delete opts.variables; (opts as any).mutation = document; - opts.fragments = calculateFragments(opts.fragments); return this.client.mutate((opts as any)); }; diff --git a/test/react-web/client/graphql/queries-1.test.tsx b/test/react-web/client/graphql/queries-1.test.tsx index 03cdfb403e..0d8f236cef 100644 --- a/test/react-web/client/graphql/queries-1.test.tsx +++ b/test/react-web/client/graphql/queries-1.test.tsx @@ -16,6 +16,18 @@ import { import graphql from '../../../../src/graphql'; +// XXX: this is also defined in apollo-client +// I'm not sure why mocha doesn't provide something like this, you can't +// always use promises +const wrap = (done: Function, cb: (...args: any[]) => any) => (...args: any[]) => { + try { + return cb(...args); + } catch (e) { + done(e); + } +}; + + describe('queries', () => { it('binds a query to props', () => { @@ -96,73 +108,6 @@ describe('queries', () => { // XXX reinsert `queries-2.test.tsx` after react 15.4 - it('correctly sets loading state on component with changed variables and unchanged result', (done) => { - const query = gql` - query remount($first: Int) { allPeople(first: $first) { people { name } } } - `; - const data = { allPeople: null }; - const variables = { first: 1 }; - const variables2 = { first: 2 }; - const networkInterface = mockNetworkInterface( - { request: { query, variables }, result: { data }, delay: 10 }, - { request: { query, variables: variables2 }, result: { data }, delay: 10 } - ); - const client = new ApolloClient({ networkInterface }); - let count = 0; - - const connect = (component) : any => { - return class Container extends React.Component { - constructor(props) { - super(props); - - this.state = { - first: 1, - }; - this.setFirst = this.setFirst.bind(this); - } - - setFirst(first) { - this.setState({first}); - } - - render() { - return React.createElement(component, { - first: this.state.first, - setFirst: this.setFirst - }); - } - } - } - - @connect - @graphql(query, { options: ({ first }) => ({ variables: { first }})}) - class Container extends React.Component { - componentWillReceiveProps(props) { - if (count === 0) { // has data - setTimeout(() => { - this.props.setFirst(2); - }, 5); - } - - if (count === 1) { - expect(this.props.data.loading).toBe(true); // on variables change - } - - if (count === 2) { - // new data after fetch - expect(props.data.loading).toBe(false); - done(); - } - count++; - } - render() { - return null; - } - }; - - renderer.create(); - }); - it('executes a query with two root fields', (done) => { const query = gql`query people { allPeople(first: 1) { people { name } } @@ -400,7 +345,8 @@ describe('queries', () => { }, 25); }); - it('reruns the query if it changes', (done) => { + // XXX: apollo client isn't firing when data is the same after a setVariable + it.skip('reruns the query if it changes', (done) => { let count = 0; const query = gql` query people($first: Int) { @@ -619,7 +565,9 @@ describe('queries', () => { renderer.create(); }); - it('exposes fetchMore as part of the props api', (done) => { + // Failing because fetchMore is not bound w/ createBoundRefetch either, + // so no loading state + it.skip('exposes fetchMore as part of the props api', (done) => { const query = gql` query people($skip: Int, $first: Int) { allPeople(first: $first, skip: $skip) { people { name } } } `; @@ -641,7 +589,7 @@ describe('queries', () => { expect(this.props.data.fetchMore).toBeTruthy(); expect(this.props.data.fetchMore instanceof Function).toBe(true); } - componentWillReceiveProps(props) { + componentWillReceiveProps = wrap(done, (props) => { if (count === 0) { expect(props.data.fetchMore).toBeTruthy(); expect(props.data.fetchMore instanceof Function).toBe(true); @@ -652,16 +600,17 @@ describe('queries', () => { people: prev.allPeople.people.concat(fetchMoreResult.data.allPeople.people), }, }), - }); - // XXX add a test for the result here when #508 is merged and released + }).then(wrap(done, result => { + expect(result.data.allPeople.people).to.deep.equal(data1.allPeople.people); + })); } else if (count === 1) { - expect(props.data.variables).toEqual(variables2); - expect(props.data.loading).toBe(true); - expect(props.data.allPeople).toEqual(data.allPeople); + expect(props.data.variables).to.deep.equal(variables); + expect(props.data.loading).to.be.true; + expect(props.data.allPeople).to.deep.equal(data.allPeople); } else if (count === 2) { - expect(props.data.variables).toEqual(variables2); - expect(props.data.loading).toBe(false); - expect(props.data.allPeople.people).toEqual( + expect(props.data.variables).to.deep.equal(variables); + expect(props.data.loading).to.be.false; + expect(props.data.allPeople.people).to.deep.equal( data.allPeople.people.concat(data1.allPeople.people) ); done(); @@ -669,7 +618,7 @@ describe('queries', () => { throw new Error('should not reach this point'); } count++; - } + }) render() { return null; } @@ -725,7 +674,9 @@ describe('queries', () => { }); - it('resets the loading state after a refetched query', (done) => { + // XXX: no longer goes to loading state because we don't bind refetch and + // apollo-client doesn't do it for us (so goes straight to done) + it.skip('resets the loading state after a refetched query', (done) => { const query = gql`query people { allPeople(first: 1) { people { name } } }`; const data = { allPeople: { people: [ { name: 'Luke Skywalker' } ] } }; const data2 = { allPeople: { people: [ { name: 'Leia Skywalker' } ] } }; @@ -739,7 +690,7 @@ describe('queries', () => { let refetched; @graphql(query) class Container extends React.Component { - componentWillReceiveProps(props) { + componentWillReceiveProps = wrap(done, (props) => { // get new data with no more loading state if (refetched) { expect(props.data.loading).toBe(false); @@ -761,7 +712,7 @@ describe('queries', () => { isRefetching = true; props.data.refetch(); } - } + }) render() { return null; } @@ -770,7 +721,9 @@ describe('queries', () => { renderer.create(); }); - it('resets the loading state after a refetched query even if the data doesn\'t change', (d) => { + // XXX: as above, but also doesn't go to done, see + // https://github.com/apollostack/apollo-client/pull/635#discussion_r78505278 + it.skip('resets the loading state after a refetched query even if the data doesn\'t change', (d) => { const query = gql`query people { allPeople(first: 1) { people { name } } }`; const data = { allPeople: { people: [ { name: 'Luke Skywalker' } ] } }; const networkInterface = mockNetworkInterface( @@ -1030,7 +983,7 @@ describe('queries', () => { this.props.data.updateQuery(); done(); } catch (e) { - expect(e.message).toMatch(/Update query has been called before query has been created/); + expect(e).to.match(/ObservableQuery with this id doesn't exist:/); done(); } } diff --git a/test/react-web/client/graphql/queries-2.test.tsx b/test/react-web/client/graphql/queries-2.test.tsx index f4b4edb575..9200043ac8 100644 --- a/test/react-web/client/graphql/queries-2.test.tsx +++ b/test/react-web/client/graphql/queries-2.test.tsx @@ -205,4 +205,72 @@ describe('queries', () => { // Prop update: fetch. setTimeout(() => render(variables2), 1000); }); + + // XXX: apollo client isn't firing when data is the same after a setVariable + it.skip('correctly sets loading state on component with changed variables and unchanged result', (done) => { + const query = gql` + query remount($first: Int) { allPeople(first: $first) { people { name } } } + `; + const data = { allPeople: null }; + const variables = { first: 1 }; + const variables2 = { first: 2 }; + const networkInterface = mockNetworkInterface( + { request: { query, variables }, result: { data }, delay: 10 }, + { request: { query, variables: variables2 }, result: { data }, delay: 10 } + ); + const client = new ApolloClient({ networkInterface }); + let count = 0; + + const connect = (component) : any => { + return class Container extends React.Component { + constructor(props) { + super(props); + + this.state = { + first: 1, + }; + this.setFirst = this.setFirst.bind(this); + } + + setFirst(first) { + this.setState({first}); + } + + render() { + return React.createElement(component, { + first: this.state.first, + setFirst: this.setFirst + }); + } + } + } + + @connect + @graphql(query, { options: ({ first }) => ({ variables: { first }})}) + class Container extends React.Component { + componentWillReceiveProps(props) { + if (count === 0) { // has data + setTimeout(() => { + this.props.setFirst(2); + }, 5); + } + + if (count === 1) { + expect(this.props.data.loading).toBe(true); // on variables change + } + + if (count === 2) { + // new data after fetch + expect(props.data.loading).toBe(false); + done(); + } + count++; + } + render() { + return null; + } + }; + + renderer.create(); + }); }); From 49560b01f6979306617095bf30deeb32a29ab1a4 Mon Sep 17 00:00:00 2001 From: Tom Coleman Date: Thu, 15 Sep 2016 17:08:31 +1000 Subject: [PATCH 07/14] Get SSR working with refactored AC --- src/graphql.tsx | 32 ++++++++++++---------------- test/react-web/server/index.test.tsx | 3 +-- 2 files changed, 15 insertions(+), 20 deletions(-) diff --git a/src/graphql.tsx b/src/graphql.tsx index 0e712c3808..e461a0f189 100644 --- a/src/graphql.tsx +++ b/src/graphql.tsx @@ -192,26 +192,19 @@ export default function graphql( function fetchData(props, { client }) { if (operation.type === DocumentType.Mutation) return false; + const opts = calculateOptions(props) as any; - opts.query = document; - - if (opts.ssr === false) return false; - if (!opts.variables) delete opts.variables; - - // if this query is in the store, don't block execution - try { - readQueryFromStore({ - store: client.queryManager.getApolloState().data, - query: opts.query, - variables: opts.variables, - fragmentMap: createFragmentMap(opts.fragments), - }); - return false; - } catch (e) {/* tslint:disable-line */} + if (opts.ssr === false || opts.skip) return false; - return client.query(opts); - } + const observable = client.watchQuery(assign({ query: document }, opts)); + const result = observable.currentResult(); + if (result.loading) { + return observable.result(); + } else { + return false; + } + } class GraphQL extends Component { static displayName = graphQLDisplayName; @@ -336,8 +329,11 @@ export default function graphql( assign(this.data, observableQueryFields(this.queryObservable)); if (!opts.forceFetch) { + const currentResult = this.queryObservable.currentResult({ + returnPartialData: true, + }); // try and fetch initial data from the store - assign(this.data, this.queryObservable.currentResult()); + assign(this.data, currentResult.data, { loading: currentResult.loading }); } } diff --git a/test/react-web/server/index.test.tsx b/test/react-web/server/index.test.tsx index 7cfa44451b..e7db6e1c00 100644 --- a/test/react-web/server/index.test.tsx +++ b/test/react-web/server/index.test.tsx @@ -51,8 +51,7 @@ describe('SSR', () => { .then(() => { const markup = ReactDOM.renderToString(app); expect(markup).toMatch(/James/); - }) - ; + }); }); it('should run return the initial state for hydration', () => { From 8289af1afb5fa8b9180134fab03fa43afba08edb Mon Sep 17 00:00:00 2001 From: Tom Coleman Date: Mon, 26 Sep 2016 20:33:39 +1000 Subject: [PATCH 08/14] Fix a bunch of tests I'd skipped --- src/graphql.tsx | 32 ++-- .../client/graphql/queries-1.test.tsx | 31 +--- .../client/graphql/queries-2.test.tsx | 146 +++++++++--------- 3 files changed, 88 insertions(+), 121 deletions(-) diff --git a/src/graphql.tsx b/src/graphql.tsx index e461a0f189..2d718a6f62 100644 --- a/src/graphql.tsx +++ b/src/graphql.tsx @@ -329,9 +329,7 @@ export default function graphql( assign(this.data, observableQueryFields(this.queryObservable)); if (!opts.forceFetch) { - const currentResult = this.queryObservable.currentResult({ - returnPartialData: true, - }); + const currentResult = this.queryObservable.currentResult(); // try and fetch initial data from the store assign(this.data, currentResult.data, { loading: currentResult.loading }); } @@ -340,16 +338,13 @@ export default function graphql( subscribeToQuery(props): boolean { const opts = calculateOptions(props) as QueryOptions; - // don't rerun if nothing has changed - if (isEqual(opts, this.previousOpts)) return false; - // XXX: tear down old subscription if it exists if (opts.skip) return; // XXX: this seems like the wrong function to do this in, should it be in willReceiveProps? // We've subscribed already, just change stuff. if (this.querySubscription) { - this.queryObservable.setVariables(opts.variables); + this.queryObservable.setOptions(opts); // Ensure we are up-to-date with the latest state of the world assign(this.data, @@ -366,26 +361,17 @@ export default function graphql( this.createQuery(opts); } - let oldData = {}; - const next = ({ data = oldData, loading, error }: any) => { - - const { queryId } = this.queryObservable; + const next = ({ data, loading, error }: any) => { + const { queryId } = this.queryObservable; - invariant(Object.keys(observableQueryFields(data)).length === 0, + const clashingKeys = Object.keys(observableQueryFields(data)); + invariant(clashingKeys.length === 0, `the result of the '${graphQLDisplayName}' operation contains keys that ` + - `conflict with the return object. 'errors', 'loading', ` + - `'startPolling', 'stopPolling', 'fetchMore', 'updateQuery', and 'refetch' cannot be ` + - `returned keys` + `conflict with the return object.` + + clashingKeys.map(k => `'${k}'`).join(', ') + ` not allowed.` ); - // only rerender child component if data has changed - if (!isEqual(oldData, data) || loading !== this.data.loading) { - this.hasOperationDataChanged = true; - } - - // cache the changed data for next check - oldData = assign({}, data); - + this.hasOperationDataChanged = true; this.data = assign({ loading, error, diff --git a/test/react-web/client/graphql/queries-1.test.tsx b/test/react-web/client/graphql/queries-1.test.tsx index 0d8f236cef..57c493b64a 100644 --- a/test/react-web/client/graphql/queries-1.test.tsx +++ b/test/react-web/client/graphql/queries-1.test.tsx @@ -106,8 +106,6 @@ describe('queries', () => { renderer.create(); }); - // XXX reinsert `queries-2.test.tsx` after react 15.4 - it('executes a query with two root fields', (done) => { const query = gql`query people { allPeople(first: 1) { people { name } } @@ -345,8 +343,7 @@ describe('queries', () => { }, 25); }); - // XXX: apollo client isn't firing when data is the same after a setVariable - it.skip('reruns the query if it changes', (done) => { + it('reruns the query if it changes', (done) => { let count = 0; const query = gql` query people($first: Int) { @@ -567,7 +564,7 @@ describe('queries', () => { // Failing because fetchMore is not bound w/ createBoundRefetch either, // so no loading state - it.skip('exposes fetchMore as part of the props api', (done) => { + it('exposes fetchMore as part of the props api', (done) => { const query = gql` query people($skip: Int, $first: Int) { allPeople(first: $first, skip: $skip) { people { name } } } `; @@ -604,10 +601,6 @@ describe('queries', () => { expect(result.data.allPeople.people).to.deep.equal(data1.allPeople.people); })); } else if (count === 1) { - expect(props.data.variables).to.deep.equal(variables); - expect(props.data.loading).to.be.true; - expect(props.data.allPeople).to.deep.equal(data.allPeople); - } else if (count === 2) { expect(props.data.variables).to.deep.equal(variables); expect(props.data.loading).to.be.false; expect(props.data.allPeople.people).to.deep.equal( @@ -674,9 +667,7 @@ describe('queries', () => { }); - // XXX: no longer goes to loading state because we don't bind refetch and - // apollo-client doesn't do it for us (so goes straight to done) - it.skip('resets the loading state after a refetched query', (done) => { + it('resets the loading state after a refetched query', (done) => { const query = gql`query people { allPeople(first: 1) { people { name } } }`; const data = { allPeople: { people: [ { name: 'Luke Skywalker' } ] } }; const data2 = { allPeople: { people: [ { name: 'Leia Skywalker' } ] } }; @@ -687,27 +678,17 @@ describe('queries', () => { const client = new ApolloClient({ networkInterface }); let isRefetching; - let refetched; @graphql(query) class Container extends React.Component { componentWillReceiveProps = wrap(done, (props) => { // get new data with no more loading state - if (refetched) { + if (isRefetching) { expect(props.data.loading).toBe(false); expect(props.data.allPeople).toEqual(data2.allPeople); done(); return; } - // don't remove old data - if (isRefetching) { - isRefetching = false; - refetched = true; - expect(props.data.loading).toBe(true); - expect(props.data.allPeople).toEqual(data.allPeople); - return; - } - if (!isRefetching) { isRefetching = true; props.data.refetch(); @@ -721,8 +702,8 @@ describe('queries', () => { renderer.create(); }); - // XXX: as above, but also doesn't go to done, see - // https://github.com/apollostack/apollo-client/pull/635#discussion_r78505278 + // XXX: this does not occur at the moment. When we add networkStatus, we should + // see a few more states it.skip('resets the loading state after a refetched query even if the data doesn\'t change', (d) => { const query = gql`query people { allPeople(first: 1) { people { name } } }`; const data = { allPeople: { people: [ { name: 'Luke Skywalker' } ] } }; diff --git a/test/react-web/client/graphql/queries-2.test.tsx b/test/react-web/client/graphql/queries-2.test.tsx index 9200043ac8..25f7cc2900 100644 --- a/test/react-web/client/graphql/queries-2.test.tsx +++ b/test/react-web/client/graphql/queries-2.test.tsx @@ -127,7 +127,7 @@ describe('queries', () => { class Container extends React.Component { componentWillMount() { if (count === 1) { - expect(this.props.data.loading).toBe(true); // on remount + expect(this.props.data.loading).toBe.true; // on remount count++; } } @@ -141,7 +141,7 @@ describe('queries', () => { if (count === 2) { // remounted data after fetch - expect(props.data.loading).toBe(false); + expect(props.data.loading).toBe.false; done(); } count++; @@ -176,11 +176,11 @@ describe('queries', () => { class Container extends React.Component { render() { const { loading } = this.props.data; - if (count === 0) expect(loading).toBe(true); - if (count === 1) expect(loading).toBe(false); - if (count === 2) expect(loading).toBe(true); + if (count === 0) expect(loading).toBe.true; + if (count === 1) expect(loading).toBe.false; + if (count === 2) expect(loading).toBe.true; if (count === 3) { - expect(loading).toBe(false); + expect(loading).toBe.false; done(); } count ++; @@ -206,71 +206,71 @@ describe('queries', () => { setTimeout(() => render(variables2), 1000); }); - // XXX: apollo client isn't firing when data is the same after a setVariable - it.skip('correctly sets loading state on component with changed variables and unchanged result', (done) => { - const query = gql` - query remount($first: Int) { allPeople(first: $first) { people { name } } } - `; - const data = { allPeople: null }; - const variables = { first: 1 }; - const variables2 = { first: 2 }; - const networkInterface = mockNetworkInterface( - { request: { query, variables }, result: { data }, delay: 10 }, - { request: { query, variables: variables2 }, result: { data }, delay: 10 } - ); - const client = new ApolloClient({ networkInterface }); - let count = 0; - - const connect = (component) : any => { - return class Container extends React.Component { - constructor(props) { - super(props); - - this.state = { - first: 1, - }; - this.setFirst = this.setFirst.bind(this); - } - - setFirst(first) { - this.setState({first}); - } - - render() { - return React.createElement(component, { - first: this.state.first, - setFirst: this.setFirst - }); - } - } - } - - @connect - @graphql(query, { options: ({ first }) => ({ variables: { first }})}) - class Container extends React.Component { - componentWillReceiveProps(props) { - if (count === 0) { // has data - setTimeout(() => { - this.props.setFirst(2); - }, 5); - } - - if (count === 1) { - expect(this.props.data.loading).toBe(true); // on variables change - } - - if (count === 2) { - // new data after fetch - expect(props.data.loading).toBe(false); - done(); - } - count++; - } - render() { - return null; - } - }; - - renderer.create(); - }); + it('correctly sets loading state on component with changed variables and unchanged result', (done) => { + const query = gql` + query remount($first: Int) { allPeople(first: $first) { people { name } } } + `; + const data = { allPeople: null }; + const variables = { first: 1 }; + const variables2 = { first: 2 }; + const networkInterface = mockNetworkInterface( + { request: { query, variables }, result: { data }, delay: 10 }, + { request: { query, variables: variables2 }, result: { data }, delay: 10 } + ); + const client = new ApolloClient({ networkInterface }); + let count = 0; + + const connect = (component) : any => { + return class Container extends React.Component { + constructor(props) { + super(props); + + this.state = { + first: 1, + }; + this.setFirst = this.setFirst.bind(this); + } + + setFirst(first) { + this.setState({first}); + } + + render() { + return React.createElement(component, { + first: this.state.first, + setFirst: this.setFirst + }); + } + } + } + + @connect + @graphql(query, { options: ({ first }) => ({ variables: { first }})}) + class Container extends React.Component { + componentWillReceiveProps(props) { + if (count === 0) { + expect(props.data.loading).toBe.false; // has initial data + setTimeout(() => { + this.props.setFirst(2); + }, 5); + } + + if (count === 1) { + expect(props.data.loading).toBe.true; // on variables change + } + + if (count === 2) { + // new data after fetch + expect(props.data.loading).toBe.false; + done(); + } + count++; + } + render() { + return null; + } + }; + + mount(); + }); }); From c574196b12983927f3c7b82cf622b8140440a894 Mon Sep 17 00:00:00 2001 From: Tom Coleman Date: Mon, 26 Sep 2016 20:49:31 +1000 Subject: [PATCH 09/14] Removed a bunch of remaining XXX notes --- src/graphql.tsx | 21 +++++----- .../client/graphql/queries-1.test.tsx | 42 +++++++++++++++++++ 2 files changed, 53 insertions(+), 10 deletions(-) diff --git a/src/graphql.tsx b/src/graphql.tsx index 2d718a6f62..d6a0c38e58 100644 --- a/src/graphql.tsx +++ b/src/graphql.tsx @@ -338,11 +338,18 @@ export default function graphql( subscribeToQuery(props): boolean { const opts = calculateOptions(props) as QueryOptions; - // XXX: tear down old subscription if it exists - if (opts.skip) return; + if (opts.skip) { + if (this.querySubscription) { + this.hasOperationDataChanged = true; + this.data = { loading: false }; + this.querySubscription.unsubscribe(); + this.forceRenderChildren(); + } + return; + } - // XXX: this seems like the wrong function to do this in, should it be in willReceiveProps? - // We've subscribed already, just change stuff. + // We've subscribed already, just update with our new options and + // take the latest result if (this.querySubscription) { this.queryObservable.setOptions(opts); @@ -351,8 +358,6 @@ export default function graphql( { loading: this.queryObservable.currentResult().loading }, observableQueryFields(this.queryObservable)); - // XXX: if pollingInterval is set, it may have changed, we could called - // this.queryObservable.startPolling here return; } @@ -393,10 +398,6 @@ export default function graphql( Instead, we subscribe to the store for network errors and re-render that way */ this.querySubscription = this.queryObservable.subscribe({ next, error: handleError }); - - // XXX: can remove this? - // XXX the tests seem to be keeping the error around? - delete this.data.error; } unsubscribeFromQuery() { diff --git a/test/react-web/client/graphql/queries-1.test.tsx b/test/react-web/client/graphql/queries-1.test.tsx index 57c493b64a..e153d7e46f 100644 --- a/test/react-web/client/graphql/queries-1.test.tsx +++ b/test/react-web/client/graphql/queries-1.test.tsx @@ -343,6 +343,48 @@ describe('queries', () => { }, 25); }); + it('stops the query if skip becomes true on prop change', (done) => { + 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 }); + + let renderCount = 0; + @graphql(query, { + options: ({ skip }) => ({ skip }), + }) + class Container extends React.Component { + componentWillReceiveProps(props) { + renderCount += 1; + if (renderCount === 1) { + // first reprop is with real data + expect(props.data.loading).to.be.false; + expect(props.data.allPeople).to.exist; + props.doSkip(); + } else { + // second time we skip + expect(props.data.loading).to.be.false; + expect(props.data.allPeople).to.not.exist; + done(); + } + + } + render() { + return null; + } + }; + + class ChangingProps extends React.Component { + state = { skip: false }; + + render() { + return this.setState({ skip: true })}/>; + } + } + + mount(); + }); + it('reruns the query if it changes', (done) => { let count = 0; const query = gql` From 3c423383ceed9bbac2052355968588b6fb87191f Mon Sep 17 00:00:00 2001 From: Tom Coleman Date: Mon, 26 Sep 2016 20:51:52 +1000 Subject: [PATCH 10/14] Linting --- src/graphql.tsx | 9 --------- test/index.ts | 6 +++--- 2 files changed, 3 insertions(+), 12 deletions(-) diff --git a/src/graphql.tsx b/src/graphql.tsx index d6a0c38e58..32b5c0d286 100644 --- a/src/graphql.tsx +++ b/src/graphql.tsx @@ -5,7 +5,6 @@ import { } from 'react'; // modules don't export ES6 modules -import isEqual = require('lodash.isequal'); import flatten = require('lodash.flatten'); import pick = require('lodash.pick'); import shallowEqual from './shallowEqual'; @@ -16,10 +15,7 @@ import assign = require('object-assign'); import hoistNonReactStatics = require('hoist-non-react-statics'); import ApolloClient, { - readQueryFromStore, - createFragmentMap, ApolloError, - WatchQueryOptions, ObservableQuery, MutationBehavior, MutationQueryReducersMap, @@ -228,9 +224,6 @@ export default function graphql( private data: any = {}; // apollo data private type: DocumentType; - private previousOpts: Object; - // private previousQueries: Object; - // request / action storage private queryObservable: ObservableQuery; private querySubscription: Subscription; @@ -367,8 +360,6 @@ export default function graphql( } const next = ({ data, loading, error }: any) => { - const { queryId } = this.queryObservable; - const clashingKeys = Object.keys(observableQueryFields(data)); invariant(clashingKeys.length === 0, `the result of the '${graphQLDisplayName}' operation contains keys that ` + diff --git a/test/index.ts b/test/index.ts index bc48ba0334..e36c6bbe79 100644 --- a/test/index.ts +++ b/test/index.ts @@ -1,3 +1,3 @@ -require('source-map-support').install({ - environment: 'node', -}); +import { install } from 'source-map-support'; + +install({ environment: 'node' }); From d8f1b11cdfd73a7793268ec478d6b26a1418d591 Mon Sep 17 00:00:00 2001 From: Tom Coleman Date: Fri, 30 Sep 2016 13:58:46 +1000 Subject: [PATCH 11/14] Update apollo version so this works w/ a branch! --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 3ba55d00d4..6c8be7b0f0 100644 --- a/package.json +++ b/package.json @@ -51,7 +51,7 @@ "peerDependencies": { "react": "0.14.x || 15.* || ^15.0.0", "redux": "^2.0.0 || ^3.0.0", - "apollo-client": "^0.4.15" + "apollo-client": "^0.4.20" }, "devDependencies": { "@types/chai": "^3.4.33", From 16dece1dca7bcc7520415321b93bd1ddcfb67a81 Mon Sep 17 00:00:00 2001 From: Tom Coleman Date: Fri, 30 Sep 2016 14:35:40 +1000 Subject: [PATCH 12/14] Fixed some problems with the rebase --- .../client/graphql/queries-1.test.tsx | 100 +++++++++--------- typings.d.ts | 4 +- 2 files changed, 52 insertions(+), 52 deletions(-) diff --git a/test/react-web/client/graphql/queries-1.test.tsx b/test/react-web/client/graphql/queries-1.test.tsx index e153d7e46f..4de044bfb9 100644 --- a/test/react-web/client/graphql/queries-1.test.tsx +++ b/test/react-web/client/graphql/queries-1.test.tsx @@ -358,13 +358,13 @@ describe('queries', () => { renderCount += 1; if (renderCount === 1) { // first reprop is with real data - expect(props.data.loading).to.be.false; - expect(props.data.allPeople).to.exist; + expect(props.data.loading).toBe.false; + expect(props.data.allPeople).toBeDefined; props.doSkip(); } else { // second time we skip - expect(props.data.loading).to.be.false; - expect(props.data.allPeople).to.not.exist; + expect(props.data.loading).toBe.false; + expect(props.data.allPeople).not.toBeDefined; done(); } @@ -382,7 +382,7 @@ describe('queries', () => { } } - mount(); + renderer.create(); }); it('reruns the query if it changes', (done) => { @@ -644,7 +644,7 @@ describe('queries', () => { })); } else if (count === 1) { expect(props.data.variables).to.deep.equal(variables); - expect(props.data.loading).to.be.false; + expect(props.data.loading).toBe.false; expect(props.data.allPeople.people).to.deep.equal( data.allPeople.people.concat(data1.allPeople.people) ); @@ -746,49 +746,49 @@ describe('queries', () => { // XXX: this does not occur at the moment. When we add networkStatus, we should // see a few more states - it.skip('resets the loading state after a refetched query even if the data doesn\'t change', (d) => { - const query = gql`query people { allPeople(first: 1) { people { name } } }`; - const data = { allPeople: { people: [ { name: 'Luke Skywalker' } ] } }; - const networkInterface = mockNetworkInterface( - { request: { query }, result: { data } }, - { request: { query }, result: { data } } - ); - const client = new ApolloClient({ networkInterface }); - - let isRefectching; - let refetched; - @graphql(query) - class Container extends React.Component { - componentWillReceiveProps(props) { - // get new data with no more loading state - if (refetched) { - expect(props.data.loading).toBe(false); - expect(props.data.allPeople).toEqual(data.allPeople); - d(); - return; - } - - // don't remove old data - if (isRefectching) { - isRefectching = false; - refetched = true; - expect(props.data.loading).toBe(true); - expect(props.data.allPeople).toEqual(data.allPeople); - return; - } - - if (!isRefectching) { - isRefectching = true; - props.data.refetch(); - } - } - render() { - return null; - } - }; - - renderer.create(); - }); + // it('resets the loading state after a refetched query even if the data doesn\'t change', (d) => { + // const query = gql`query people { allPeople(first: 1) { people { name } } }`; + // const data = { allPeople: { people: [ { name: 'Luke Skywalker' } ] } }; + // const networkInterface = mockNetworkInterface( + // { request: { query }, result: { data } }, + // { request: { query }, result: { data } } + // ); + // const client = new ApolloClient({ networkInterface }); + // + // let isRefectching; + // let refetched; + // @graphql(query) + // class Container extends React.Component { + // componentWillReceiveProps(props) { + // // get new data with no more loading state + // if (refetched) { + // expect(props.data.loading).toBe(false); + // expect(props.data.allPeople).toEqual(data.allPeople); + // d(); + // return; + // } + // + // // don't remove old data + // if (isRefectching) { + // isRefectching = false; + // refetched = true; + // expect(props.data.loading).toBe(true); + // expect(props.data.allPeople).toEqual(data.allPeople); + // return; + // } + // + // if (!isRefectching) { + // isRefectching = true; + // props.data.refetch(); + // } + // } + // render() { + // return null; + // } + // }; + // + // renderer.create(); + // }); it('allows a polling query to be created', (done) => { const query = gql`query people { allPeople(first: 1) { people { name } } }`; @@ -1006,7 +1006,7 @@ describe('queries', () => { this.props.data.updateQuery(); done(); } catch (e) { - expect(e).to.match(/ObservableQuery with this id doesn't exist:/); + expect(e.toString()).toMatch(/ObservableQuery with this id doesn't exist:/); done(); } } diff --git a/typings.d.ts b/typings.d.ts index 192c37f025..101765feed 100644 --- a/typings.d.ts +++ b/typings.d.ts @@ -20,8 +20,8 @@ declare module 'lodash.flatten' { } declare module 'lodash.pick' { - import main = require('lodash'); - export = main.pick; + import main = require('lodash/pick'); + export = main; } declare module 'recompose/compose' { From eca6cba3534ace1a755d89972e0948f6c0eb3b0e Mon Sep 17 00:00:00 2001 From: James Baxley Date: Fri, 7 Oct 2016 22:30:29 -0400 Subject: [PATCH 13/14] reintroduce skip test --- package.json | 3 +- .../client/graphql/queries-1.test.tsx | 328 ++++++++---------- 2 files changed, 151 insertions(+), 180 deletions(-) diff --git a/package.json b/package.json index ead9e451b1..98c209ac12 100644 --- a/package.json +++ b/package.json @@ -5,8 +5,7 @@ "main": "index.js", "typings": "index.d.ts", "scripts": { - "test": "npm run testonly", - "testonly": "jest", + "test": "jest", "posttest": "npm run lint", "filesize": "npm run compile:browser && ./scripts/filesize.js --file=./dist/index.min.js --maxGzip=15", "compile": "tsc", diff --git a/test/react-web/client/graphql/queries-1.test.tsx b/test/react-web/client/graphql/queries-1.test.tsx index fe3658b1b7..9af0a6bcc6 100644 --- a/test/react-web/client/graphql/queries-1.test.tsx +++ b/test/react-web/client/graphql/queries-1.test.tsx @@ -344,184 +344,156 @@ describe('queries', () => { }, 25); }); -// <<<<<<< HEAD -// it('stops the query if skip becomes true on prop change', (done) => { -// ======= -// it('allows you to skip a query without running it', (done) => { -// >>>>>>> a23883fd226e0352813b4889cf647964c4c0e0fc -// 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 }); - -// <<<<<<< HEAD -// let renderCount = 0; -// @graphql(query, { -// options: ({ skip }) => ({ skip }), -// }) -// class Container extends React.Component { -// componentWillReceiveProps(props) { -// renderCount += 1; -// if (renderCount === 1) { -// // first reprop is with real data -// expect(props.data.loading).toBe.false; -// expect(props.data.allPeople).toBeDefined; -// props.doSkip(); -// } else { -// // second time we skip -// expect(props.data.loading).toBe.false; -// expect(props.data.allPeople).not.toBeDefined; -// done(); -// } - -// ======= -// let queryExecuted; -// @graphql(query, { skip: ({ skip }) => skip }) -// class Container extends React.Component { -// componentWillReceiveProps(props) { -// queryExecuted = true; -// } -// render() { -// expect(this.props.data).toBeFalsy(); -// return null; -// } -// }; - -// renderer.create(); - -// setTimeout(() => { -// if (!queryExecuted) { done(); return; } -// done(new Error('query ran even though skip present')); -// }, 25); -// }); - -// it('doesn\'t run options or props when skipped', (done) => { -// 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 }); - -// let queryExecuted; -// @graphql(query, { -// skip: ({ skip }) => skip, -// options: ({ willThrowIfAccesed }) => ({ pollInterval: willThrowIfAccesed.pollInterval }), -// props: ({ willThrowIfAccesed }) => ({ pollInterval: willThrowIfAccesed.pollInterval }), -// }) -// class Container extends React.Component { -// componentWillReceiveProps(props) { -// queryExecuted = true; -// } -// render() { -// expect(this.props.data).toBeFalsy(); -// return null; -// } -// }; - -// renderer.create(); - -// setTimeout(() => { -// if (!queryExecuted) { done(); return; } -// done(new Error('query ran even though skip present')); -// }, 25); -// }); - -// it('allows you to skip a query without running it (alternate syntax)', (done) => { -// 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 }); - -// let queryExecuted; -// @graphql(query, { skip: true }) -// class Container extends React.Component { -// componentWillReceiveProps(props) { -// queryExecuted = true; -// } -// render() { -// expect(this.props.data).toBeFalsy(); -// return null; -// } -// }; - -// renderer.create(); - -// setTimeout(() => { -// if (!queryExecuted) { done(); return; } -// done(new Error('query ran even though skip present')); -// }, 25); -// }); - -// it('removes the injected props if skip becomes true', (done) => { -// let count = 0; -// const query = gql` -// query people($first: Int) { -// allPeople(first: $first) { people { name } } -// } -// `; - -// const data1 = { allPeople: { people: [ { name: 'Luke Skywalker' } ] } }; -// const variables1 = { first: 1 }; - -// const data2 = { allPeople: { people: [ { name: 'Leia Skywalker' } ] } }; -// const variables2 = { first: 2 }; - -// const networkInterface = mockNetworkInterface( -// { request: { query, variables: variables1 }, result: { data: data1 } }, -// { request: { query, variables: variables2 }, result: { data: data2 } } -// ); - -// const client = new ApolloClient({ networkInterface }); - -// @graphql(query, { -// skip: () => count === 1, -// options: (props) => ({ variables: props }), -// }) -// class Container extends React.Component { -// componentWillReceiveProps({ data }) { -// // loading is true, but data still there -// if (count === 0) expect(data.allPeople).toEqual(data1.allPeople); -// if (count === 1 ) expect(data).toBeFalsy(); -// if (count === 2 && data.loading) expect(data.allPeople).toBeFalsy(); -// if (count === 2 && !data.loading) { -// expect(data.allPeople).toEqual(data2.allPeople); -// done(); -// } -// >>>>>>> a23883fd226e0352813b4889cf647964c4c0e0fc -// } -// render() { -// return null; -// } -// }; - -// class ChangingProps extends React.Component { -// <<<<<<< HEAD -// state = { skip: false }; - -// render() { -// return this.setState({ skip: true })}/>; -// ======= -// state = { first: 1 }; - -// componentDidMount() { -// setTimeout(() => { -// count++; -// this.setState({ first: 2 }); -// }, 50); - -// setTimeout(() => { -// count++; -// this.setState({ first: 3 }); -// }, 100); -// } - -// render() { -// return ; -// >>>>>>> a23883fd226e0352813b4889cf647964c4c0e0fc -// } -// } - -// renderer.create(); -// }); + it('allows you to skip a query without running it', (done) => { + 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 }); + + let queryExecuted; + @graphql(query, { skip: ({ skip }) => skip }) + class Container extends React.Component { + componentWillReceiveProps(props) { + queryExecuted = true; + } + render() { + expect(this.props.data).toBeFalsy(); + return null; + } + }; + + renderer.create(); + + setTimeout(() => { + if (!queryExecuted) { done(); return; } + done(new Error('query ran even though skip present')); + }, 25); + }); + + it('doesn\'t run options or props when skipped', (done) => { + 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 }); + + let queryExecuted; + @graphql(query, { + skip: ({ skip }) => skip, + options: ({ willThrowIfAccesed }) => ({ pollInterval: willThrowIfAccesed.pollInterval }), + props: ({ willThrowIfAccesed }) => ({ pollInterval: willThrowIfAccesed.pollInterval }), + }) + class Container extends React.Component { + componentWillReceiveProps(props) { + queryExecuted = true; + } + render() { + expect(this.props.data).toBeFalsy(); + return null; + } + }; + + renderer.create(); + + setTimeout(() => { + if (!queryExecuted) { done(); return; } + done(new Error('query ran even though skip present')); + }, 25); + }); + + it('allows you to skip a query without running it (alternate syntax)', (done) => { + 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 }); + + let queryExecuted; + @graphql(query, { skip: true }) + class Container extends React.Component { + componentWillReceiveProps(props) { + queryExecuted = true; + } + render() { + expect(this.props.data).toBeFalsy(); + return null; + } + }; + + renderer.create(); + + setTimeout(() => { + if (!queryExecuted) { done(); return; } + done(new Error('query ran even though skip present')); + }, 25); + }); + + it('removes the injected props if skip becomes true', (done) => { + let count = 0; + const query = gql` + query people($first: Int) { + allPeople(first: $first) { people { name } } + } + `; + + const data1 = { allPeople: { people: [ { name: 'Luke Skywalker' } ] } }; + const variables1 = { first: 1 }; + + const data2 = { allPeople: { people: [ { name: 'Leia Skywalker' } ] } }; + const variables2 = { first: 2 }; + + + const data3 = { allPeople: { people: [ { name: 'Anakin Skywalker' } ] } }; + const variables3 = { first: 3 }; + + const networkInterface = mockNetworkInterface( + { request: { query, variables: variables1 }, result: { data: data1 } }, + { request: { query, variables: variables2 }, result: { data: data2 } }, + { request: { query, variables: variables3 }, result: { data: data2 } } + ); + + const client = new ApolloClient({ networkInterface }); + + @graphql(query, { + skip: () => count === 1, + options: (props) => ({ variables: props }), + }) + class Container extends React.Component { + componentWillReceiveProps({ data }) { + // loading is true, but data still there + if (count === 0) expect(data.allPeople).toEqual(data1.allPeople); + if (count === 1 ) expect(data).toBeFalsy(); + if (count === 2 && data.loading) expect(data.allPeople).toBeFalsy(); + if (count === 2 && !data.loading) { + expect(data.allPeople).toEqual(data2.allPeople); + done(); + } + } + render() { + return null; + } + }; + + class ChangingProps extends React.Component { + state = { first: 1 }; + + componentDidMount() { + setTimeout(() => { + count++; + this.setState({ first: 2 }); + }, 50); + + setTimeout(() => { + count++; + this.setState({ first: 3 }); + }, 100); + } + + render() { + return ; + } + } + + renderer.create(); + }); it('reruns the query if it changes', (done) => { let count = 0; From 3473345cb8c3ba3010c2cf641baea8ee2be8da74 Mon Sep 17 00:00:00 2001 From: James Baxley Date: Fri, 7 Oct 2016 22:31:31 -0400 Subject: [PATCH 14/14] update changelog --- Changelog.md | 1 + 1 file changed, 1 insertion(+) diff --git a/Changelog.md b/Changelog.md index 049e31e236..c3eb4c5807 100644 --- a/Changelog.md +++ b/Changelog.md @@ -8,6 +8,7 @@ Expect active development and potentially significant breaking changes in the `0 - Feature: Move types to dev deps [#251](https://github.com/apollostack/react-apollo/pull/251) - Feature: New method for skipping queries which bypasses HOC internals [#253](https://github.com/apollostack/react-apollo/pull/253) - Feature: Integrated subscriptions! [#256](https://github.com/apollostack/react-apollo/pull/256) +- Feature: Refactor loading state managment to use apollo-client fully. Reduces library size by ~50% [#211](https://github.com/apollostack/react-apollo/pull/211) ### v0.5.7