Two unrelated small cache improvements #7241
Conversation
- Explicitly allow people to pass `cache: 'bounded'`. Non-TS users upgrading from the recommended AS3.9+ configuration could do this by accident. Fixes #7240. - Upgrade `@apollo/utils.keyvaluecache` so that the new `PrefixingKeyValueCache.cacheDangerouslyDoesNotNeedPrefixesForIsolation` feature lets you disable prefixing for the APQ and full response caches. Throw if you try to pass such a cache to `ApolloServer` itself because that cache is designed to be shared across features. Migrate off of PrefixingKeyValueCache for `documentStore` so that its prefixing can't be disabled. Fixes #6742.
✅ Deploy Preview for apollo-server-docs ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
|
I put this in one PR because they touched similar code. A test would be nice but eh, we don't have any tests of passing |
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit fa9fdd4:
|
| @@ -214,6 +220,19 @@ export class ApolloServer<in out TContext extends BaseContext = BaseContext> { | |||
|
|
|||
| const isDev = nodeEnv !== 'production'; | |||
|
|
|||
| if ( | |||
| config.cache && | |||
| config.cache !== 'bounded' && | |||
There was a problem hiding this comment.
Can we log / warn when this is set? It'd be nice to re-remove this for good in v5
There was a problem hiding this comment.
You think so? I feel like it's not a huge deal and not worth bugging folks about (and if we have a very short migration guide for v5 then it's fine to remove it then without an explicit deprecation period).
There was a problem hiding this comment.
Yeah I don't feel too strongly about it, I guess the nice thing about the warning is it's simple and actionable: "you have a line of code that you can delete". I'm ok with leaving it though.
|
For the cache plugin we definitely want the upgrade. And integration test doesn't really matter either way. I guess the only one where not upgrading makes sense is gateway-interface... but I do kinda like trying to keep deps consistent to avoid accidental double installs? Would like your thoughts on both these questions before I merge/release @trevor-scheer. |
|
Nbd to upgrade them, go ahead and keep as is 👍 |
This PR was opened by the [Changesets release](https://github.com/changesets/action) GitHub action. When you're ready to do a release, you can merge this and the packages will be published to npm automatically. If you're not ready to do a release yet, that's fine, whenever you add more changesets to main, this PR will be updated. # Releases ## @apollo/server-plugin-response-cache@4.1.0 ### Minor Changes - [#7241](#7241) [`d7e9b9759`](d7e9b97) Thanks [@glasser](https://github.com/glasser)! - If the cache you provide to the `cache` option is created with `PrefixingKeyValueCache.cacheDangerouslyDoesNotNeedPrefixesForIsolation` (new in `@apollo/utils.keyvaluecache@2.1.0`), the `fqc:` prefix will not be added to cache keys. ## @apollo/server@4.3.0 ### Minor Changes - [#7241](#7241) [`d7e9b9759`](d7e9b97) Thanks [@glasser](https://github.com/glasser)! - If the cache you provide to the `persistedQueries.cache` option is created with `PrefixingKeyValueCache.cacheDangerouslyDoesNotNeedPrefixesForIsolation` (new in `@apollo/utils.keyvaluecache@2.1.0`), the `apq:` prefix will not be added to cache keys. Providing such a cache to `new ApolloServer()` throws an error. ### Patch Changes - [#7232](#7232) [`3a4823e0d`](3a4823e) Thanks [@glasser](https://github.com/glasser)! - Refactor the implementation of `ApolloServerPluginDrainHttpServer`'s grace period. This is intended to be a no-op. - [#7229](#7229) [`d057e2ffc`](d057e2f) Thanks [@dnalborczyk](https://github.com/dnalborczyk)! - Improve compatibility with Cloudflare workers by avoiding the use of the Node `util` package. This change is intended to be a no-op. - [#7228](#7228) [`f97e55304`](f97e553) Thanks [@dnalborczyk](https://github.com/dnalborczyk)! - Improve compatibility with Cloudflare workers by avoiding the use of the Node `url` package. This change is intended to be a no-op. - [#7241](#7241) [`d7e9b9759`](d7e9b97) Thanks [@glasser](https://github.com/glasser)! - For ease of upgrade from the recommended configuration of Apollo Server v3.9+, you can now pass `new ApolloServer({ cache: 'bounded' })`, which is equivalent to not providing the `cache` option (as a bounded cache is now the default in AS4). ## @apollo/server-integration-testsuite@4.3.0 ### Patch Changes - [#7228](#7228) [`f97e55304`](f97e553) Thanks [@dnalborczyk](https://github.com/dnalborczyk)! - Improve compatibility with Cloudflare workers by avoiding the use of the Node `url` package. This change is intended to be a no-op. - Updated dependencies \[[`3a4823e0d`](3a4823e), [`d057e2ffc`](d057e2f), [`f97e55304`](f97e553), [`d7e9b9759`](d7e9b97), [`d7e9b9759`](d7e9b97)]: - @apollo/server@4.3.0 Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Explicitly allow people to pass
cache: 'bounded'. Non-TS users upgrading from the recommended AS3.9+ configuration could do this by accident. Fixes Allowcache: "bounded"or fail quickly if it is passed #7240.Upgrade
@apollo/utils.keyvaluecacheso that the newPrefixingKeyValueCache.cacheDangerouslyDoesNotNeedPrefixesForIsolationfeature lets you disable prefixing for the APQ and full response caches. Throw if you try to pass such a cache toApolloServeritself because that cache is designed to be shared across features. Migrate off of PrefixingKeyValueCache fordocumentStoreso that its prefixing can't be disabled. Fixes fqc prefix is added and there's no way to disable it #6742.