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

(feat) - allow passing operationContext to useQuery #351

Merged
merged 10 commits into from
Aug 1, 2019

Conversation

JoviDeCroock
Copy link
Collaborator

Fixes: #346

@JoviDeCroock JoviDeCroock requested review from kitten, andyrichardson and parkerziegler and removed request for andyrichardson July 23, 2019 09:38
src/hooks/useQuery.ts Outdated Show resolved Hide resolved
src/hooks/useQuery.ts Outdated Show resolved Hide resolved
@kitten
Copy link
Member

kitten commented Jul 30, 2019

If there are no objections, I'd just like to merge this as is for now 😅 Potentially this could obviously be nicer, but since for most users context won't be passed, the simple solution may be best here. In other cases for "advanced" use-cases with context, I'd assume that people would spot the required memoisation rather quickly.

On this note though, we may want to add the same modification to useMutation and useSubscription as well:

Edit: Actually let's merge this for now so that we don't increase the scope of this PR :)

@JoviDeCroock
Copy link
Collaborator Author

Hmm, weird issue in the useSubscription test. When logging it calls with url but when checking the mock it doesn't seem to be present. Will look further after a break

@JoviDeCroock JoviDeCroock requested a review from kitten July 31, 2019 10:11
@JoviDeCroock
Copy link
Collaborator Author

@kitten just updating docs atm and then I think it's ready to merge, is there a spot that should be safe to note about the context and memoization of it?

@kitten
Copy link
Member

kitten commented Jul 31, 2019

@JoviDeCroock I suppose we should add an entire section about the context somewhere, where we're also talking about customising urql. So maybe in "Extending & Experimenting". I'm thinking that doc can be extended with "FAQ-style" guides.

E.g.: "How to write custom exchanges", "How to extend the context", etc

@JoviDeCroock
Copy link
Collaborator Author

All right, added it thanks for the heads up. It's rather small now but we can iterate on this.

@kitten
Copy link
Member

kitten commented Jul 31, 2019

@JoviDeCroock let's discuss this tomorrow! There's a lot of todos for the docs anyway, so we can put down some specific todos for what sections need to be written 🙌

@kitten kitten merged commit 90c3c26 into master Aug 1, 2019
@kitten kitten deleted the chore/addOperationContext branch August 1, 2019 09:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add OperationContext to useQuery arguments
4 participants