Skip to content

Commit 2315e73

Browse files
jaydensericjaydenseric
committed
Refactor the GraphQL instance method operate.
1 parent 7c5d9a1 commit 2315e73

File tree

2 files changed

+125
-112
lines changed

2 files changed

+125
-112
lines changed

‎changelog.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
- Improved the test utility `promisifyEvent` function.
1919
- Test the the `GraphQL` instance method `operate` option `reloadOnLoad` in isolation.
2020
- Test better the order of the `GraphQL` instance method `operate` triggered events.
21+
- Refactored the `GraphQL` instance method `operate` to eliminate the `GraphQL` private instance method `fetch` and reduce the chance of race conditions in consumer code.
2122
- Reduced the number of promises created by the `GraphQL` instance method `operate` when the `reloadOnLoad` and `reloadOnLoad` options are `false`.
2223
- Added a code example for how to await all loading GraphQL operations.
2324
- Used consistent JSDoc types for promises that resolve `void`.

‎src/universal/GraphQL.js‎

Lines changed: 124 additions & 112 deletions
Original file line numberDiff line numberDiff line change
@@ -154,117 +154,6 @@ module.exports = class GraphQL {
154154
this.emit('reset', { exceptCacheKey });
155155
};
156156

157-
/**
158-
* Fetches a GraphQL operation.
159-
* @param {GraphQLFetchOptions} fetchOptions URL and options for [`fetch`](https://developer.mozilla.org/docs/Web/API/Fetch_API).
160-
* @param {GraphQLCacheKey} cacheKey [GraphQL cache]{@link GraphQL#cache} [key]{@link GraphQLCacheKey}.
161-
* @returns {Promise<GraphQLCacheValue>} A promise that resolves the [GraphQL cache]{@link GraphQL#cache} [value]{@link GraphQLCacheValue}.
162-
* @fires GraphQL#event:fetch
163-
* @fires GraphQL#event:cache
164-
* @ignore
165-
*/
166-
fetch = ({ url, ...options }, cacheKey) => {
167-
let fetchResponse;
168-
169-
const fetcher =
170-
typeof fetch === 'function'
171-
? fetch
172-
: () =>
173-
Promise.reject(
174-
new TypeError('Global fetch API or polyfill unavailable.')
175-
);
176-
const cacheValue = {};
177-
const cacheValuePromise = fetcher(url, options)
178-
.then(
179-
(response) => {
180-
fetchResponse = response;
181-
182-
if (!response.ok)
183-
cacheValue.httpError = {
184-
status: response.status,
185-
statusText: response.statusText,
186-
};
187-
188-
return response.json().then(
189-
({ errors, data }) => {
190-
// JSON parse ok.
191-
if (!errors && !data)
192-
cacheValue.parseError = 'Malformed payload.';
193-
if (errors) cacheValue.graphQLErrors = errors;
194-
if (data) cacheValue.data = data;
195-
},
196-
({ message }) => {
197-
// JSON parse error.
198-
cacheValue.parseError = message;
199-
}
200-
);
201-
},
202-
({ message }) => {
203-
cacheValue.fetchError = message;
204-
}
205-
)
206-
.then(() => {
207-
// If there’s earlier GraphQL operation(s) loading for the same cache
208-
// key, wait for them to complete so the cache is updated and the
209-
// `cache` events are emitted in the order the operations were
210-
// initiated. This prevents a slow earlier operation from overwriting
211-
// the cache from a faster later operation.
212-
if (this.operations[cacheKey].length > 1) {
213-
const operationIndex = this.operations[cacheKey].indexOf(
214-
cacheValuePromise
215-
);
216-
217-
if (operationIndex)
218-
// There are earlier GraphQL operations.
219-
return Promise.all(
220-
// The earlier GraphQL operations.
221-
this.operations[cacheKey].slice(0, operationIndex)
222-
);
223-
}
224-
})
225-
.then(() => {
226-
// The cache value promise should resolve after the cache has been
227-
// updated, it’s cleared from the map of loading GraphQL operations,
228-
// and the `cache` event has been emitted (in that order).
229-
230-
// Update the cache.
231-
this.cache[cacheKey] = cacheValue;
232-
233-
// Clear this operation from the map of loading GraphQL operations.
234-
this.operations[cacheKey].splice(
235-
// The `>>> 0` is a clever way to defend against `indexOf` returning
236-
// `-1` if the cache value promise is not in the array for some
237-
// unexpected reason. It leaves non-negative integers alone, but
238-
// converts `-1` into the largest possible unsigned 32-bit integer,
239-
// which happens to be the ECMAScript spec max length of an array;
240-
// resulting in a harmless splice.
241-
this.operations[cacheKey].indexOf(cacheValuePromise) >>> 0,
242-
1
243-
);
244-
if (!this.operations[cacheKey].length) delete this.operations[cacheKey];
245-
246-
// Emit the `cache` event.
247-
this.emit('cache', {
248-
cacheKey,
249-
cacheValue,
250-
251-
// May be undefined if there was a fetch error.
252-
response: fetchResponse,
253-
});
254-
255-
return cacheValue;
256-
});
257-
258-
if (!this.operations[cacheKey]) this.operations[cacheKey] = [];
259-
this.operations[cacheKey].push(cacheValuePromise);
260-
261-
// Emit the `fetch` event after the cache value promise has been added to
262-
// the map of loading GraphQL operations.
263-
this.emit('fetch', { cacheKey, cacheValuePromise });
264-
265-
return cacheValuePromise;
266-
};
267-
268157
/**
269158
* Loads a GraphQL operation, visible in
270159
* [GraphQL operations]{@link GraphQL#operations}. Emits a
@@ -300,10 +189,133 @@ module.exports = class GraphQL {
300189
'operate() options “reloadOnLoad” and “resetOnLoad” can’t both be true.'
301190
);
302191

192+
// The `fetch` global not being defined correctly results in a `fetchError`
193+
// within the cache value; not the `operate` method throwing an error.
194+
const fetcher =
195+
typeof fetch === 'function'
196+
? fetch
197+
: () =>
198+
Promise.reject(
199+
new TypeError('Global fetch API or polyfill unavailable.')
200+
);
201+
202+
// Create the fetch options.
303203
const fetchOptions = graphqlFetchOptions(operation);
304204
if (fetchOptionsOverride) fetchOptionsOverride(fetchOptions);
205+
const { url, ...options } = fetchOptions;
206+
207+
// Create the cache key.
305208
const cacheKey = cacheKeyCreator(fetchOptions);
306-
const cacheValuePromise = this.fetch(fetchOptions, cacheKey);
209+
210+
// The `operations` property must be updated for sync code following this
211+
// `operate` method call, as well as async code using `cacheValuePromise`.
212+
let resolveOperationsUpdated;
213+
const operationsUpdatedPromise = new Promise((resolve) => {
214+
resolveOperationsUpdated = resolve;
215+
});
216+
217+
// Start the fetch sync within the `operate` method rather than within the
218+
// `cacheValuePromise` chain to ensure fetches are sent in the order of
219+
// multiple `operate` method calls within sync code.
220+
const responsePromise = fetcher(url, options);
221+
222+
let fetchResponse;
223+
224+
const cacheValue = {};
225+
const cacheValuePromise = operationsUpdatedPromise.then(() =>
226+
responsePromise
227+
.then(
228+
(response) => {
229+
fetchResponse = response;
230+
231+
if (!response.ok)
232+
cacheValue.httpError = {
233+
status: response.status,
234+
statusText: response.statusText,
235+
};
236+
237+
return response.json().then(
238+
({ errors, data }) => {
239+
// JSON parse ok.
240+
if (!errors && !data)
241+
cacheValue.parseError = 'Malformed payload.';
242+
if (errors) cacheValue.graphQLErrors = errors;
243+
if (data) cacheValue.data = data;
244+
},
245+
({ message }) => {
246+
// JSON parse error.
247+
cacheValue.parseError = message;
248+
}
249+
);
250+
},
251+
({ message }) => {
252+
cacheValue.fetchError = message;
253+
}
254+
)
255+
.then(() => {
256+
// If there’s earlier GraphQL operation(s) loading for the same cache
257+
// key, wait for them to complete so the cache is updated and the
258+
// `cache` events are emitted in the order the operations were
259+
// initiated. This prevents a slow earlier operation from overwriting
260+
// the cache from a faster later operation.
261+
if (this.operations[cacheKey].length > 1) {
262+
const operationIndex = this.operations[cacheKey].indexOf(
263+
cacheValuePromise
264+
);
265+
266+
if (operationIndex)
267+
// There are earlier GraphQL operations.
268+
return Promise.all(
269+
// The earlier GraphQL operations.
270+
this.operations[cacheKey].slice(0, operationIndex)
271+
);
272+
}
273+
})
274+
.then(() => {
275+
// The cache value promise should resolve after the cache has been
276+
// updated, it’s cleared from the map of loading GraphQL operations,
277+
// and the `cache` event has been emitted (in that order).
278+
279+
// Update the cache.
280+
this.cache[cacheKey] = cacheValue;
281+
282+
// Clear this operation from the map of loading GraphQL operations.
283+
this.operations[cacheKey].splice(
284+
// The `>>> 0` is a clever way to defend against `indexOf` returning
285+
// `-1` if the cache value promise is not in the array for some
286+
// unexpected reason. It leaves non-negative integers alone, but
287+
// converts `-1` into the largest possible unsigned 32-bit integer,
288+
// which happens to be the ECMAScript spec max length of an array;
289+
// resulting in a harmless splice.
290+
this.operations[cacheKey].indexOf(cacheValuePromise) >>> 0,
291+
1
292+
);
293+
294+
// If there are no more operations loading for this cache key, delete
295+
// the empty array from the `operations` property.
296+
if (!this.operations[cacheKey].length)
297+
delete this.operations[cacheKey];
298+
299+
// Emit the `cache` event.
300+
this.emit('cache', {
301+
cacheKey,
302+
cacheValue,
303+
304+
// May be undefined if there was a fetch error.
305+
response: fetchResponse,
306+
});
307+
308+
return cacheValue;
309+
})
310+
);
311+
312+
// Add this operation to the map of loading GraphQL operations.
313+
if (!this.operations[cacheKey]) this.operations[cacheKey] = [];
314+
this.operations[cacheKey].push(cacheValuePromise);
315+
resolveOperationsUpdated();
316+
317+
// Emit the `fetch` event after the `operations` property has been updated.
318+
this.emit('fetch', { cacheKey, cacheValuePromise });
307319

308320
// A reload or reset happens after the cache is updated as a side effect.
309321
if (reloadOnLoad)

0 commit comments

Comments
 (0)