Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Allow cache: "bounded" or fail quickly if it is passed #7240

Closed
glasser opened this issue Dec 10, 2022 · 0 comments · Fixed by #7241
Closed

Allow cache: "bounded" or fail quickly if it is passed #7240

glasser opened this issue Dec 10, 2022 · 0 comments · Fixed by #7241

Comments

@glasser
Copy link
Member

glasser commented Dec 10, 2022

Issue Description

In AS 3.9 we introduced cache: "bounded" as an easy way to shift to the newly-recommended-for-DOS-prevention behavior of not having an unbounded in-memory cache. In AS4 we made this behavior the default and stopped recognizing cache: "bounded". Non-TypeScript users upgrading to AS4 may miss the migration guide section stating that this is the default and in fact the migration guide does not state that the old value no longer works. While this is not a big deal for TS users (or @ts-check users) and it would be unreasonable for us to try to reproduce our API's entire set of compile-time checks at runtime, this particular change penalizes users who followed our recommended practices in 3.9 on, so it would be nice to either explicitly allow cache: "bounded" or to make it an immediate runtime failure (and update the migration guide).

Link to Reproduction

caused problems here: apollographql/apollo-utils#236

Reproduction Steps

No response
glasser added a commit that referenced this issue Dec 10, 2022
@glasser
Two unrelated small cache improvements
fa9fdd4 
- 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.
This was referenced Dec 10, 2022
Merged
Merged
@smyrick smyrick mentioned this issue Dec 12, 2022
Merged
glasser added a commit that referenced this issue Dec 12, 2022
@glasser
Two unrelated small cache improvements (#7241)
d7e9b97 
- 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.
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 19, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Two unrelated small cache improvements apollographql/apollo-server
1 participant
@glasser