feat: optimize concurrent queries via shared network requests
Types of changes
- [ ] Chore (a non-breaking change which is related to package maintenance)
- [ ] Bug fix (a non-breaking change which fixes an issue)
- [x] New feature (a non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to change)
To do before merging
- [ ] Add tests for all methods using
Client.prototype.fetch() - [ ] Refactor code to reduce size increase
Description
This PR optimizes concurrent queries by sharing network requests. If two or more concurrent network requests look identical (same URL and, if given, signal), only one network request is made.
The primary purpose of this optimization is to share calls to /api/v2 (i.e. getCachedRepository()), which all queries perform before calling the /api/v2/documents/search endpoint. The implementation, however, allows any concurrent equivalent network requests to be shared.
The following contrived example demonstrates identical concurrent queries:
import * as prismic from "@prismicio/client";
const client = prismic.createClient("qwerty");
// Two network requests are made:
//
// 1x request to `/api/v2`
// 1x request to `/api/v2/documents/search`
await Promise.all([
client.get(),
client.get(),
client.get(),
client.get(),
client.get(),
]);
The following more realistic example demonstrates practical concurrent queries:
// Four network requests are made:
//
// 1x request to `/api/v2`
// 3x requests to `/api/v2/documents/search`
await Promise.all([
client.getByUID("page", "about"),
client.getSingle("navigation"),
client.getSingle("settings"),
]);
Note how only one call to /api/v2 is made. Without this PR, multiple concurrent calls to /api/v2 are necessary since each query starts immediately without a cached /api/v2 response.
// Without this PR, six network requests are made:
//
// 3x request to `/api/v2`
// 3x requests to `/api/v2/documents/search`
await Promise.all([
client.getByUID("page", "about"),
client.getSingle("navigation"),
client.getSingle("settings"),
]);
Abortable queries via AbortSignal
All query methods are abortable (i.e. cancelable) via the signal parameter. An AbortSignal, usually controlled by an AbortController, can be aborted, causing the network request to be stopped and throw an error.
A query with a signal and an equivalent query without a signal are not equivalent. If the AbortSignal is cancelled, we don't want the signal-less query to abort as well. Thus, network requests cannot be shared.
const controller = new AbortController();
// Four network requests are made:
//
// 2x request to `/api/v2`
// 2x requests to `/api/v2/documents/search`
await Promise.all([
// Shared
client.get(),
client.get(),
// Not shared
client.get({ signal: controller.signal }),
]);
Things to consider
This approach to implementing #260 provides optimizations that are general. All network requests, not just calls to getCachedRepository(), are optimized.
Using a general implementation may not be necessary or useful in real-world scenarios. Calling the same query within a single Promise.all() should almost never happen.
It is possible for a single client to be shared across multiple contexts which makes identical queries in quick succession, such as a static site generator that calls client.getSingle("settings") on each page.
The general implementation in this PR takes up more bytes than something specific to getCachedRepository(). We need to decide if the bytes are worth optimizing every, albeit infrequent, edge case.
Closes #260
Checklist:
- [ ] My change requires an update to the official documentation.
- [x] All TSDoc comments are up-to-date and new ones have been added where necessary.
- [x] All new and existing tests are passing.
size-limit report 📦
| Path | Size |
|---|---|
| dist/index.js | 4.26 KB (+2.74% 🔺) |
| dist/index.cjs | 7.17 KB (+1.53% 🔺) |
Codecov Report
Merging #261 (3fcdb42) into v7 (a520aa4) will increase coverage by
0.00%. The diff coverage is100.00%.
:mega: This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more
@@ Coverage Diff @@
## v7 #261 +/- ##
=======================================
Coverage 99.96% 99.96%
=======================================
Files 48 48
Lines 5281 5326 +45
Branches 303 311 +8
=======================================
+ Hits 5279 5324 +45
Misses 2 2
| Impacted Files | Coverage Δ | |
|---|---|---|
| src/createClient.ts | 99.94% <100.00%> (+<0.01%) |
:arrow_up: |
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.
Do we want to still work on that or no? I feel like it'd still be beneficial
Yes, let's try to get this into v7. Sorry, it's my fault that this didn't proceed; I totally forgot about it! I'll finish this PR before we merge v7.
No pressure, but any update on this @angeloashmore?
All tests have been updated with a new testConcurrentMethod() test helper.
I messed up and made all the changes as part of the v7 merge commit. :<
The implementation has not been refactored to reduce the size increase, but I think we can leave it as is for now. As we get closer to a v7 release with its full feature set, we can refactor and optimize for size.