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.

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

Update docs to clarify that id is not a required field for cache.evict(options) #8146

Merged
merged 5 commits into from
Aug 5, 2021

Conversation

aayush-k
Copy link
Contributor

@aayush-k aayush-k commented May 5, 2021

Tried this without looking at which options were required/not required and realized that just providing a fieldName is sufficient when i want to evict all cached entries for a specific gql query. I also found that @benjamn mentioned the same solution/implied that id is optional in #6795 (comment)

image

The sourcecode also indicates that the default default value is ROOT_QUERY https://github.com/apollographql/apollo-client/blob/main/src/cache/inmemory/inMemoryCache.ts#L266-L272

Checklist:

  • If this PR is a new feature, please reference an issue where a consensus about the design was reached (not necessary for small changes) (N/A)
  • Make sure all of the significant new logic is covered by tests (N/A)

…or cache.evict(options)

Tried this without looking at which options were required/not required and realized that just providing a fieldName is sufficient when i want to evict all cached entries for a specific gql query. I also found that @benjamn mentioned the same solution/implied that `id` is optional in apollographql#6795 (comment)
@apollo-cla
Copy link

@aayush-k: Thank you for submitting a pull request! Before we can merge it, you'll need to sign the Apollo Contributor License Agreement here: https://contribute.apollographql.com/

@aayush-k aayush-k marked this pull request as ready for review May 5, 2021 23:08
@aayush-k aayush-k requested a review from StephenBarlow as a code owner May 5, 2021 23:08
@aayush-k
Copy link
Contributor Author

aayush-k commented May 5, 2021

Also I stumbled across this comment:

// To my knowledge, TypeScript does not currently provide a way to
// enforce that an optional property?:type must *not* be undefined
// when present. That ability would be useful here, because we want
// options.id to default to ROOT_QUERY only when no options.id was
// provided. If the caller attempts to pass options.id with a
// falsy/undefined value (perhaps because cache.identify failed), we
// should not assume the goal was to modify the ROOT_QUERY object.
// We could throw, but it seems natural to return false to indicate
// that nothing was modified.

I'm not sure if this was the reason why id was originally labeled as "Required" in the docs but maybe this would help:

const modify = (x: ({ id: string; optimistic?: boolean } | {optimistic?: boolean})) => x;
modify({ id: 'string' }); // <- ok
modify({ id: undefined }); // <- errors
modify({}); // <- ok

@StephenBarlow
Copy link
Contributor

Thanks @aayush-k! I believe this was incorrectly marked as required purely because I incorrectly thought it was required 😄

@StephenBarlow StephenBarlow merged commit c1b01f1 into apollographql:main Aug 5, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 15, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants