Skip to content
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

Add checking for existence of fetchMoreQuery in store #3367

Merged
merged 7 commits into from
May 25, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 6 additions & 5 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,16 +11,14 @@
"benchmark": "cd packages/apollo-client && npm run benchmark",
"prelint": "npm run lint-fix",
"lint": "lerna run -- lint",
"lint-fix":
"prettier --trailing-comma all --single-quote --write \"packages/*/{src,tests,test,benchmark}/**/*.{j,t}s*\"",
"lint-fix": "prettier --trailing-comma all --single-quote --write \"packages/*/{src,tests,test,benchmark}/**/*.{j,t}s*\"",
"lint-staged": "lint-staged",
"filesize": "lerna run -- filesize && bundlesize",
"type-check": "lerna run -- type-check",
"coverage": "lerna run -- coverage",
"coverage:upload": "codecov",
"danger": "danger run --verbose",
"deploy":
"lerna publish -m \"chore: Publish\" --independent && cd packages/apollo-client && npm run deploy"
"deploy": "lerna publish -m \"chore: Publish\" --independent && cd packages/apollo-client && npm run deploy"
},
"bundlesize": [
{
Expand Down Expand Up @@ -64,7 +62,10 @@
"prettier --trailing-comma all --single-quote --write",
"git add"
],
"*.js*": ["prettier --trailing-comma all --single-quote --write", "git add"]
"*.js*": [
"prettier --trailing-comma all --single-quote --write",
"git add"
]
},
"pre-commit": "lint-staged",
"devDependencies": {
Expand Down
5 changes: 5 additions & 0 deletions packages/apollo-client/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,11 @@
(Android), by instead setting the `prototype` of `this` manually.
[Issue #3236](https://github.com/apollographql/apollo-client/issues/3236)
[PR #3306](https://github.com/apollographql/apollo-client/pull/3306)
- Added safeguards to make sure `QueryStore.initQuery` and
`QueryStore.markQueryResult` don't try to set the network status of a
`fetchMoreForQueryId` query, if it does not exist in the store.
[Issue #3345](https://github.com/apollographql/apollo-client/issues/3345)
[PR #3367](https://github.com/apollographql/apollo-client/pull/3367)

### 2.3.0
- fixed edge case bug of changing fetchPolicies right after resetStore with no variables present
Expand Down
75 changes: 75 additions & 0 deletions packages/apollo-client/src/data/__tests__/queries.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
import { QueryStore } from '../queries';
import { NetworkStatus } from '../../core/networkStatus';
import { DocumentNode } from 'graphql';

describe('QueryStore', () => {
const queryId = 'abc123';
let queryStore;

beforeEach(() => {
queryStore = new QueryStore();
queryStore.initQuery({
queryId,
document: {} as DocumentNode,
storePreviousVariables: false,
variables: {},
isPoll: false,
isRefetch: false,
metadata: {},
fetchMoreForQueryId: queryId,
});
});

describe('initQuery', () => {
it(
'should set the network status of a query to `fetchMore` if the ' +
'query has a `fetchMoreForQueryId` property',
() => {
expect(queryStore.get(queryId).networkStatus).toBe(
NetworkStatus.fetchMore,
);
},
);

it(
'should not attempt to set the network status of a ' +
'`fetchMoreForQueryId` query, if it does not exist in the store',
() => {
queryStore.stopQuery(queryId);
expect(() => {
queryStore.initQuery({
queryId: 'new-query-id',
document: {} as DocumentNode,
storePreviousVariables: false,
variables: {},
isPoll: false,
isRefetch: false,
metadata: {},
fetchMoreForQueryId: queryId,
});
}).not.toThrow("Cannot set property 'networkStatus' of undefined");
},
);
});

describe('markQueryResult', () => {
it(
'should set the network status of a `fetchMoreForQueryId` query to ' +
'`ready` in the store, if it exists',
() => {
queryStore.markQueryResult(queryId, {}, queryId);
expect(queryStore.get(queryId).networkStatus).toBe(NetworkStatus.ready);
},
);

it(
'should not attempt to set the network status of a ' +
'`fetchMoreForQueryId` query, if it does not exist in the store',
() => {
expect(() => {
queryStore.markQueryResult(queryId, {}, 'id-does-not-exist');
}).not.toThrow("Cannot set property 'networkStatus' of undefined");
},
);
});
});
10 changes: 8 additions & 2 deletions packages/apollo-client/src/data/queries.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,10 @@ export class QueryStore {
// error action branch, but importantly *not* in the client result branch.
// This is because the implementation of `fetchMore` *always* sets
// `fetchPolicy` to `network-only` so we would never have a client result.
if (typeof query.fetchMoreForQueryId === 'string') {
if (
typeof query.fetchMoreForQueryId === 'string' &&
this.store[query.fetchMoreForQueryId]
) {
this.store[query.fetchMoreForQueryId].networkStatus =
NetworkStatus.fetchMore;
}
Expand All @@ -125,7 +128,10 @@ export class QueryStore {
// If we have a `fetchMoreForQueryId` then we need to update the network
// status for that query. See the branch for query initialization for more
// explanation about this process.
if (typeof fetchMoreForQueryId === 'string') {
if (
typeof fetchMoreForQueryId === 'string' &&
this.store[fetchMoreForQueryId]
) {
this.store[fetchMoreForQueryId].networkStatus = NetworkStatus.ready;
}
}
Expand Down