Skip to content

Commit

Permalink
fix(apollo-cache-inmemory): track typename in IdValue and replace und…
Browse files Browse the repository at this point in the history
…erlying objects when it changes (#3159)

* fix(apollo-cache-inmemory): track typename in field id values and allow missing id when it changes

There is an edge case bug with types that are of a Union type of:
`{ id, __typename: 'First' }` and `{ __typename: 'Second' }` (without the `id` field).

In such a case, if we first retrieve a result with `First` and then re-query when the response contains the other type (`Second`), Apollo would throw an error that the field is now missing an `id`, which was previously present. This would be correct if the object was of the same type, however we cannot know whether another type (`Second`) even contains an `id` field in the schema in the first place (here, it didn't).

This commit adds support for tracking typenames to the `IdValue` and should such a case occur, it fully replaces fields with autogenerated names in case their underlying types have changed, after being re-queried.

* chore(tests): add typenames to IdValues created in tests

* chore(tests): use snapshots to simplify testing changes to the Cache implementation

* doc(apollo-cache-inmemory): add CHANGELOG entry

* refactor(apollo-cache-inmemory): try to address benchmark drop

* doc(apollo-cache-inmemory): add contributor

* chore(apollo-utilities): add a warning about toIdValue signature change

* chore(apollo-utilities): update flow typings

* chore(apollo-utilities): update flow typings

* Update package.json

* refactor(apollo-utilities): change toIdValue helper signature to remove breaking change
  • Loading branch information
niieani authored and James Baxley committed Mar 23, 2018
1 parent bdfcc93 commit 6903c0e
Show file tree
Hide file tree
Showing 18 changed files with 1,063 additions and 597 deletions.
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@
{
"name": "apollo-utilities",
"path": "./packages/apollo-utilities/lib/bundle.min.js",
"maxSize": "4.75 kB"
"maxSize": "5 kB"
},
{
"name": "graphql-anywhere",
Expand Down
2 changes: 2 additions & 0 deletions packages/apollo-cache-inmemory/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
# Change log

### vNext
- Fix an edge case where fields that were unions of two types, one with an `id`,
one without an `id`, would cause the cache to throw while saving the result [#3159](https://github.com/apollographql/apollo-client/pull/3159)
- Map coverage to original source
- Fixed bug with cacheRedirects not getting attached [#3016](https://github.com/apollographql/apollo-client/pull/3016)

Expand Down
16 changes: 7 additions & 9 deletions packages/apollo-cache-inmemory/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@
"James Baxley <james@meteor.com>",
"Jonas Helfer <jonas@helfer.email>",
"Sashko Stubailo <sashko@stubailo.com>",
"James Burgess <jamesmillerburgess@gmail.com>"
"James Burgess <jamesmillerburgess@gmail.com>",
"Bazyli Brzóska <bazyli.brzoska@gmail.com>"
],
"license": "MIT",
"main": "./lib/bundle.umd.js",
Expand All @@ -33,8 +34,10 @@
"watch": "tsc -w -p .",
"clean": "rimraf coverage/* && rimraf lib/*",
"prepublishOnly": "npm run build",
"build:browser": "browserify ./lib/bundle.umd.js --i apollo-cache --i apollo-utilities --i graphql -o=./lib/bundle.js && npm run minify:browser",
"minify:browser": "uglifyjs -c -m -o ./lib/bundle.min.js -- ./lib/bundle.js",
"build:browser":
"browserify ./lib/bundle.umd.js --i apollo-cache --i apollo-utilities --i graphql -o=./lib/bundle.js && npm run minify:browser",
"minify:browser":
"uglifyjs -c -m -o ./lib/bundle.min.js -- ./lib/bundle.js",
"filesize": "npm run build:browser"
},
"dependencies": {
Expand Down Expand Up @@ -66,12 +69,7 @@
".(ts|tsx)": "<rootDir>/node_modules/ts-jest/preprocessor.js"
},
"testRegex": "(/__tests__/.*|\\.(test|spec))\\.(ts|tsx|js)$",
"moduleFileExtensions": [
"ts",
"tsx",
"js",
"json"
],
"moduleFileExtensions": ["ts", "tsx", "js", "json"],
"mapCoverage": true
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,135 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[
`Cache writeFragment will write some deeply nested data into the store at any id 1`
] = `
Object {
"bar": Object {
"i": 7,
},
"foo": Object {
"e": 4,
"h": Object {
"generated": false,
"id": "bar",
"type": "id",
"typename": undefined,
},
},
}
`;

exports[
`Cache writeFragment will write some deeply nested data into the store at any id 2`
] = `
Object {
"bar": Object {
"i": 7,
"j": 8,
"k": 9,
},
"foo": Object {
"e": 4,
"f": 5,
"g": 6,
"h": Object {
"generated": false,
"id": "bar",
"type": "id",
"typename": undefined,
},
},
}
`;

exports[
`Cache writeFragment will write some deeply nested data into the store at any id 3`
] = `
Object {
"bar": Object {
"i": 10,
"j": 8,
"k": 9,
},
"foo": Object {
"e": 4,
"f": 5,
"g": 6,
"h": Object {
"generated": false,
"id": "bar",
"type": "id",
"typename": undefined,
},
},
}
`;

exports[
`Cache writeFragment will write some deeply nested data into the store at any id 4`
] = `
Object {
"bar": Object {
"i": 10,
"j": 11,
"k": 12,
},
"foo": Object {
"e": 4,
"f": 5,
"g": 6,
"h": Object {
"generated": false,
"id": "bar",
"type": "id",
"typename": undefined,
},
},
}
`;

exports[
`Cache writeFragment will write some deeply nested data into the store at any id 5`
] = `
Object {
"bar": Object {
"i": 7,
"j": 8,
"k": 9,
},
"foo": Object {
"e": 4,
"f": 5,
"g": 6,
"h": Object {
"generated": false,
"id": "bar",
"type": "id",
"typename": "Bar",
},
},
}
`;

exports[
`Cache writeFragment will write some deeply nested data into the store at any id 6`
] = `
Object {
"bar": Object {
"i": 10,
"j": 11,
"k": 12,
},
"foo": Object {
"e": 4,
"f": 5,
"g": 6,
"h": Object {
"generated": false,
"id": "bar",
"type": "id",
"typename": "Bar",
},
},
}
`;
Original file line number Diff line number Diff line change
@@ -1,5 +1,139 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[
`MapCache Cache writeFragment will write some deeply nested data into the store at any id 1`
] = `
Object {
"bar": Object {
"i": 7,
},
"foo": Object {
"e": 4,
"h": Object {
"generated": false,
"id": "bar",
"type": "id",
"typename": undefined,
},
},
}
`;

exports[
`MapCache Cache writeFragment will write some deeply nested data into the store at any id 2`
] = `
Object {
"bar": Object {
"i": 7,
"j": 8,
"k": 9,
},
"foo": Object {
"e": 4,
"f": 5,
"g": 6,
"h": Object {
"generated": false,
"id": "bar",
"type": "id",
"typename": undefined,
},
},
}
`;

exports[
`MapCache Cache writeFragment will write some deeply nested data into the store at any id 3`
] = `
Object {
"bar": Object {
"i": 10,
"j": 8,
"k": 9,
},
"foo": Object {
"e": 4,
"f": 5,
"g": 6,
"h": Object {
"generated": false,
"id": "bar",
"type": "id",
"typename": undefined,
},
},
}
`;

exports[
`MapCache Cache writeFragment will write some deeply nested data into the store at any id 4`
] = `
Object {
"bar": Object {
"i": 10,
"j": 11,
"k": 12,
},
"foo": Object {
"e": 4,
"f": 5,
"g": 6,
"h": Object {
"generated": false,
"id": "bar",
"type": "id",
"typename": undefined,
},
},
}
`;

exports[
`MapCache Cache writeFragment will write some deeply nested data into the store at any id 5`
] = `
Object {
"bar": Object {
"i": 7,
"j": 8,
"k": 9,
},
"foo": Object {
"e": 4,
"f": 5,
"g": 6,
"h": Object {
"generated": false,
"id": "bar",
"type": "id",
"typename": "Bar",
},
},
}
`;

exports[
`MapCache Cache writeFragment will write some deeply nested data into the store at any id 6`
] = `
Object {
"bar": Object {
"i": 10,
"j": 11,
"k": 12,
},
"foo": Object {
"e": 4,
"f": 5,
"g": 6,
"h": Object {
"generated": false,
"id": "bar",
"type": "id",
"typename": "Bar",
},
},
}
`;

exports[
`MapCache writing to the store throws when trying to write an object without id that was previously queried with id 1`
] = `
Expand Down
Loading

0 comments on commit 6903c0e

Please sign in to comment.