Mirror: The highly customizable and versatile GraphQL client with which you add on features like normalized caching as you grow.
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

Fix commutative squashing failing when lowest layer resolved ear… (#587)

* Add writeFragment dependencies test

* Update test cases

* Fix commutative squashing only triggering for blocking key

The commutative squashing would previously only start when
the lowest commutative layer (the first one) was written.
This ignored the case however of when this layer has already
been written. When this was the case the stopping condition
was unreachable and commutative layers were never squashed.

The change is to continuously squash the commutative layers
with the least priorities as they're written.

This change also gets rid of the temporary `squash` queue,
which ends up making it a rather nice implementation.

* Add changeset

* Prevent dependencies from being written for squashLayer

* Simplify squash iteration

We can actually get rid of the double iteration

authored by

Phil Plückthun and committed by
GitHub
687e375f cc372aed

+191 -18
+5
.changeset/good-swans-cough.md
··· 1 + --- 2 + '@urql/exchange-graphcache': patch 3 + --- 4 + 5 + Fix commutative layer edge case when lowest-priority layer comes back earlier than others
+159
exchanges/graphcache/src/cacheExchange.test.ts
··· 136 136 expect(result).toHaveBeenCalledTimes(3); 137 137 }); 138 138 139 + it('updates related queries when a mutation update touches query data', () => { 140 + jest.useFakeTimers(); 141 + 142 + const balanceFragment = gql` 143 + fragment BalanceFragment on Author { 144 + id 145 + balance { 146 + amount 147 + } 148 + } 149 + `; 150 + 151 + const queryById = gql` 152 + query($id: ID!) { 153 + author(id: $id) { 154 + id 155 + name 156 + ...BalanceFragment 157 + } 158 + } 159 + 160 + ${balanceFragment} 161 + `; 162 + 163 + const queryByIdDataA = { 164 + __typename: 'Query', 165 + author: { 166 + __typename: 'Author', 167 + id: '1', 168 + name: 'Author 1', 169 + balance: { 170 + __typename: 'Balance', 171 + amount: 100, 172 + }, 173 + }, 174 + }; 175 + 176 + const queryByIdDataB = { 177 + __typename: 'Query', 178 + author: { 179 + __typename: 'Author', 180 + id: '2', 181 + name: 'Author 2', 182 + balance: { 183 + __typename: 'Balance', 184 + amount: 200, 185 + }, 186 + }, 187 + }; 188 + 189 + const mutation = gql` 190 + mutation($userId: ID!, $amount: Int!) { 191 + updateBalance(userId: $userId, amount: $amount) { 192 + userId 193 + balance { 194 + amount 195 + } 196 + } 197 + } 198 + `; 199 + 200 + const mutationData = { 201 + __typename: 'Mutation', 202 + updateBalance: { 203 + __typename: 'UpdateBalanceResult', 204 + userId: '1', 205 + balance: { 206 + __typename: 'Balance', 207 + amount: 1000, 208 + }, 209 + }, 210 + }; 211 + 212 + const client = createClient({ url: 'http://0.0.0.0' }); 213 + const { source: ops$, next } = makeSubject<Operation>(); 214 + 215 + const reexec = jest 216 + .spyOn(client, 'reexecuteOperation') 217 + .mockImplementation(next); 218 + 219 + const opOne = client.createRequestOperation('query', { 220 + key: 1, 221 + query: queryById, 222 + variables: { id: 1 }, 223 + }); 224 + 225 + const opTwo = client.createRequestOperation('query', { 226 + key: 2, 227 + query: queryById, 228 + variables: { id: 2 }, 229 + }); 230 + 231 + const opMutation = client.createRequestOperation('mutation', { 232 + key: 3, 233 + query: mutation, 234 + variables: { userId: '1', amount: 1000 }, 235 + }); 236 + 237 + const response = jest.fn( 238 + (forwardOp: Operation): OperationResult => { 239 + if (forwardOp.key === 1) { 240 + return { operation: opOne, data: queryByIdDataA }; 241 + } else if (forwardOp.key === 2) { 242 + return { operation: opTwo, data: queryByIdDataB }; 243 + } else if (forwardOp.key === 3) { 244 + return { operation: opMutation, data: mutationData }; 245 + } 246 + 247 + return undefined as any; 248 + } 249 + ); 250 + 251 + const result = jest.fn(); 252 + const forward: ExchangeIO = ops$ => pipe(ops$, delay(1), map(response)); 253 + 254 + const updates = { 255 + Mutation: { 256 + updateBalance: jest.fn((result, _args, cache) => { 257 + const { 258 + updateBalance: { userId, balance }, 259 + } = result; 260 + cache.writeFragment(balanceFragment, { id: userId, balance }); 261 + }), 262 + }, 263 + }; 264 + 265 + const keys = { 266 + Balance: () => null, 267 + }; 268 + 269 + pipe( 270 + cacheExchange({ updates, keys })({ forward, client })(ops$), 271 + tap(result), 272 + publish 273 + ); 274 + 275 + next(opTwo); 276 + jest.runAllTimers(); 277 + expect(response).toHaveBeenCalledTimes(1); 278 + 279 + next(opOne); 280 + jest.runAllTimers(); 281 + expect(response).toHaveBeenCalledTimes(2); 282 + 283 + next(opMutation); 284 + jest.runAllTimers(); 285 + 286 + expect(response).toHaveBeenCalledTimes(3); 287 + expect(updates.Mutation.updateBalance).toHaveBeenCalledTimes(1); 288 + 289 + expect(reexec).toHaveBeenCalledTimes(1); 290 + expect(reexec.mock.calls[0][0].key).toBe(1); 291 + 292 + expect(result.mock.calls[2][0]).toHaveProperty( 293 + 'data.author.balance.amount', 294 + 1000 295 + ); 296 + }); 297 + 139 298 it('does nothing when no related queries have changed', () => { 140 299 const queryUnrelated = gql` 141 300 {
+11 -1
exchanges/graphcache/src/store/data.test.ts
··· 214 214 215 215 InMemoryData.initDataState(data, null); 216 216 expect(InMemoryData.readRecord('Query', 'index')).toBe(2); 217 + 218 + expect(data.optimisticOrder).toEqual([]); 217 219 }); 218 220 219 221 it('creates optimistic layers that may be removed using clearLayer', () => { ··· 239 241 240 242 InMemoryData.initDataState(data, null); 241 243 expect(InMemoryData.readRecord('Query', 'index')).toBe(1); 244 + 245 + expect(data.optimisticOrder).toEqual([]); 242 246 }); 243 247 244 248 it('overrides data using optimistic layers', () => { ··· 261 265 262 266 InMemoryData.initDataState(data, null); 263 267 expect(InMemoryData.readRecord('Query', 'index')).toBe(3); 268 + 269 + expect(data.optimisticOrder).toEqual([3, 2, 1]); 264 270 }); 265 271 266 272 it('avoids optimistic layers when only one layer is pending', () => { ··· 277 283 278 284 InMemoryData.initDataState(data, null); 279 285 expect(InMemoryData.readRecord('Query', 'index')).toBe(1); 286 + 287 + expect(data.optimisticOrder).toEqual([]); 280 288 }); 281 289 282 - it('continues applying optimistic layers even if the first one completes', () => { 290 + it.only('continues applying optimistic layers even if the first one completes', () => { 283 291 InMemoryData.reserveLayer(data, 1); 284 292 InMemoryData.reserveLayer(data, 2); 285 293 InMemoryData.reserveLayer(data, 3); ··· 312 320 313 321 InMemoryData.initDataState(data, null); 314 322 expect(InMemoryData.readRecord('Query', 'index')).toBe(4); 323 + 324 + expect(data.optimisticOrder).toEqual([]); 315 325 }); 316 326 });
+16 -17
exchanges/graphcache/src/store/data.ts
··· 93 93 const optimisticKey = currentOptimisticKey; 94 94 currentOptimisticKey = null; 95 95 96 + // Determine whether the current operation has been a commutative layer 96 97 if (optimisticKey && data.commutativeKeys.has(optimisticKey)) { 98 + // Find the lowest index of the commutative layers 99 + // The first part of `optimisticOrder` are the non-commutative layers 97 100 const commutativeIndex = 98 101 data.optimisticOrder.length - data.commutativeKeys.size; 99 - const blockingKey = data.optimisticOrder[data.optimisticOrder.length - 1]; 100 - // If this is a Query operation that is in the list of commutative keys 101 - // and is the "first" one and hence blocking all others, we squash all 102 - // results and empty the list of commutative keys 103 - if (blockingKey === optimisticKey) { 104 - const squash: number[] = []; 105 - const orderSize = data.optimisticOrder.length; 106 - // Collect all completed, commutative layers until and excluding the first 107 - // pending one that overrides the others 108 - for (let i = commutativeIndex; i < orderSize; i++) { 109 - const layerKey = data.optimisticOrder[i]; 110 - if (!data.refLock[layerKey]) break; 111 - squash.unshift(layerKey); 112 - } 113 102 114 - // Apply all completed, commutative layers 115 - for (let i = 0, l = squash.length; i < l; i++) { 116 - squashLayer(squash[i]); 103 + // Squash all layers in reverse order (low priority upwards) that have 104 + // been written already 105 + let i = data.optimisticOrder.length; 106 + while (--i >= commutativeIndex) { 107 + if (data.refLock[data.optimisticOrder[i]]) { 108 + squashLayer(data.optimisticOrder[i]); 109 + } else { 110 + break; 117 111 } 118 112 } 119 113 } ··· 494 488 495 489 /** Merges an optimistic layer of links and records into the base data */ 496 490 const squashLayer = (layerKey: number) => { 491 + // Hide current dependencies from squashing operations 492 + const prevDependencies = currentDependencies; 493 + currentDependencies = new Set(); 494 + 497 495 const links = currentData!.links.optimistic[layerKey]; 498 496 if (links) { 499 497 links.forEach((keyMap, entityKey) => { ··· 510 508 }); 511 509 } 512 510 511 + currentDependencies = prevDependencies; 513 512 clearLayer(currentData!, layerKey); 514 513 }; 515 514