Skip to content

Commit

Permalink
Merge pull request #2869 from gdi2290/fix-unstable-stringify-key
Browse files Browse the repository at this point in the history
Store key names now leverage a more deterministic approach to handling JSON based strings.
  • Loading branch information
hwillson committed Jun 1, 2018
2 parents aee073e + d9d4016 commit 74e5b3c
Show file tree
Hide file tree
Showing 9 changed files with 56 additions and 14 deletions.
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@
{
"name": "apollo-utilities",
"path": "./packages/apollo-utilities/lib/bundle.min.js",
"maxSize": "5 kB"
"maxSize": "5.2 kB"
},
{
"name": "graphql-anywhere",
Expand Down
4 changes: 2 additions & 2 deletions packages/apollo-cache-inmemory/src/__tests__/readFromStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ describe('reading from the store', () => {
ROOT_QUERY: {
id: 'abcd',
nullField: null,
'numberField({"intArg":5,"floatArg":3.14})': 5,
'numberField({"floatArg":3.14,"intArg":5})': 5,
'stringField({"arg":"This is a string!"})': 'Heyo',
},
});
Expand Down Expand Up @@ -220,7 +220,7 @@ describe('reading from the store', () => {
ROOT_QUERY: {
id: 'abcd',
nullField: null,
'numberField({"intArg":0,"floatArg":3.14})': 5,
'numberField({"floatArg":3.14,"intArg":0})': 5,
'stringField({"arg":"This is a default string!"})': 'Heyo',
},
});
Expand Down
18 changes: 9 additions & 9 deletions packages/apollo-cache-inmemory/src/__tests__/writeToStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ describe('writing to the store', () => {
ROOT_QUERY: {
id: 'abcd',
nullField: null,
'numberField({"intArg":5,"floatArg":3.14})': 5,
'numberField({"floatArg":3.14,"intArg":5})': 5,
'stringField({"arg":"This is a string!"})': 'Heyo',
},
});
Expand Down Expand Up @@ -215,7 +215,7 @@ describe('writing to the store', () => {
ROOT_QUERY: {
id: 'abcd',
nullField: null,
'numberField({"intArg":5,"floatArg":3.14})': 5,
'numberField({"floatArg":3.14,"intArg":5})': 5,
'stringField({"arg":"This is a default string!"})': 'Heyo',
},
});
Expand Down Expand Up @@ -1188,17 +1188,17 @@ describe('writing to the store', () => {
}).toObject(),
).toEqual({
'5': {
'some_mutation({"input":{"id":"5","arr":[1,{"a":"b"}],"obj":{"a":"b"},"num":5.5,"nil":null,"bo":true}})': {
type: 'id',
id: '5',
id: 'id',
'some_mutation({"input":{"arr":[1,{"a":"b"}],"bo":true,"id":"5","nil":null,"num":5.5,"obj":{"a":"b"}}})': {
generated: false,
},
'some_mutation_with_variables({"input":{"id":"5","arr":[1,{"a":"b"}],"obj":{"a":"b"},"num":5.5,"nil":null,"bo":true}})': {
type: 'id',
id: '5',
type: 'id',
},
'some_mutation_with_variables({"input":{"arr":[1,{"a":"b"}],"bo":true,"id":"5","nil":null,"num":5.5,"obj":{"a":"b"}}})': {
generated: false,
id: '5',
type: 'id',
},
id: 'id',
},
});
} else {
Expand Down
9 changes: 9 additions & 0 deletions packages/apollo-utilities/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,14 @@
# CHANGELOG

### vNext

- Store key names generated by `getStoreKeyName` now leverage a more
deterministic approach to handling JSON based strings. This prevents store
key names from differing when using `args` like
`{ prop1: 'value1', prop2: 'value2' }` and
`{ prop2: 'value2', prop1: 'value1' }`.
[PR #2869](https://github.com/apollographql/apollo-client/pull/2869)

### 1.0.13

- Make `maybeDeepFreeze` a little more defensive, by always using
Expand Down
3 changes: 3 additions & 0 deletions packages/apollo-utilities/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,9 @@
"minify:browser": "uglifyjs -c -m -o ./lib/bundle.min.js -- ./lib/bundle.js",
"filesize": "npm run build && npm run build:browser"
},
"dependencies": {
"fast-json-stable-stringify": "^2.0.0"
},
"devDependencies": {
"@types/graphql": "0.12.7",
"@types/jest": "22.2.3",
Expand Down
23 changes: 23 additions & 0 deletions packages/apollo-utilities/src/__tests__/storeUtils.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import { getStoreKeyName } from '../storeUtils';

describe('getStoreKeyName', () => {
it(
'should return a deterministic version of the store key name no matter ' +
'which order the args object properties are in',
() => {
const validStoreKeyName =
'someField({"prop1":"value1","prop2":"value2"})';
let generatedStoreKeyName = getStoreKeyName('someField', {
prop1: 'value1',
prop2: 'value2',
});
expect(generatedStoreKeyName).toEqual(validStoreKeyName);

generatedStoreKeyName = getStoreKeyName('someField', {
prop2: 'value2',
prop1: 'value1',
});
expect(generatedStoreKeyName).toEqual(validStoreKeyName);
},
);
});
1 change: 1 addition & 0 deletions packages/apollo-utilities/src/declarations.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
declare module 'fast-json-stable-stringify';
7 changes: 6 additions & 1 deletion packages/apollo-utilities/src/storeUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ import {
NameNode,
} from 'graphql';

import stringify from 'fast-json-stable-stringify';

export interface IdValue {
type: 'id';
id: string;
Expand Down Expand Up @@ -217,7 +219,10 @@ export function getStoreKeyName(
let completeFieldName: string = fieldName;

if (args) {
const stringifiedArgs: string = JSON.stringify(args);
// We can't use `JSON.stringify` here since it's non-deterministic,
// and can lead to different store key names being created even though
// the `args` object used during creation has the same properties/values.
const stringifiedArgs: string = stringify(args);
completeFieldName += `(${stringifiedArgs})`;
}

Expand Down
3 changes: 2 additions & 1 deletion packages/apollo-utilities/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@
"compilerOptions": {
"rootDir": "./src",
"outDir": "lib",
"lib": ["es6", "dom", "es2017.object"]
"lib": ["es6", "dom", "es2017.object"],
"allowSyntheticDefaultImports": true
},
"include": ["src/**/*.ts"],
"exclude": ["src/**/__tests__/*.ts"]
Expand Down

0 comments on commit 74e5b3c

Please sign in to comment.