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

Allow silencing broadcast for cache update methods. #6288

Merged
merged 3 commits into from
May 18, 2020

Conversation

benjamn
Copy link
Member

@benjamn benjamn commented May 15, 2020

As discussed in #6262 (comment), it would be convenient to be able to silence the broadcast of cache updates for methods like cache.writeQuery, cache.writeFragment, and cache.evict.

While this was an easy change for writeQuery and writeFragment, which already accept various named options (see Cache.Write{Query,Fragment}Options), it was a bit trickier in the case of cache.evict, which instead has positional parameters: id: string, fieldName?: string, and args?: Record<string, any> (see #6141 if args is new to you).

Compared to named options, the ergonomics of positional parameters deteriorate rapidly as you add more parameters. Instead of continuing to fight that battle, I decided it was time to make cache.evict accept named options: Cache.EvictOptions. Passing id, fieldName, and args arguments still works as a backwards compatible alternative to Cache.EvictOptions, but we won't be adding any new positional parameters to cache.evict from this point forward. Any/all new cache.evict inputs (such as options.broadcast) will be added via Cache.EvictOptions.

⚠️ This PR is still very much in need of tests! ⚠️

@benjamn
Copy link
Member Author

benjamn commented May 16, 2020

@hwillson Happy to split the options.readField changes out into a separate PR, if you like.

@hwillson
Copy link
Member

@benjamn Keeping everything here together works for me, thanks!

@darkbasic
Copy link

I'm all in for named options. Also, since it's a new major release anyway, I'm also for deprecating the old syntax with positional parameters.

@benjamn benjamn force-pushed the make-readField-and-evict-accept-options branch 2 times, most recently from da198a9 to 2d1c120 Compare May 18, 2020 14:52
@benjamn benjamn changed the base branch from set-context.keyObject-in-defaultDataIdFromObject to master May 18, 2020 14:52
benjamn added a commit that referenced this pull request May 18, 2020
I decided to split these changes out from PR #6288 after all, per my
comment #6288 (comment).
benjamn added a commit that referenced this pull request May 18, 2020
I decided to split these changes out from PR #6288 after all, per my
comment #6288 (comment).
@benjamn benjamn force-pushed the make-readField-and-evict-accept-options branch from 2d1c120 to ef6df37 Compare May 18, 2020 21:39
@benjamn benjamn force-pushed the make-readField-and-evict-accept-options branch from ef6df37 to 4699f5c Compare May 18, 2020 22:09
@benjamn benjamn marked this pull request as ready for review May 18, 2020 22:09
@benjamn benjamn requested a review from hwillson May 18, 2020 22:09
@benjamn benjamn merged commit eb0461f into master May 18, 2020
@benjamn benjamn deleted the make-readField-and-evict-accept-options branch May 19, 2020 16:35
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants