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

(docs) - custom exchanges #361

Merged
merged 15 commits into from
Aug 5, 2019
Merged

(docs) - custom exchanges #361

merged 15 commits into from
Aug 5, 2019

Conversation

JoviDeCroock
Copy link
Collaborator

@JoviDeCroock JoviDeCroock commented Aug 1, 2019

In this first commit I've made an exchange that looks at the token and refreshes when needed, we build iteratively up until this point.

This covers the concepts:

  • HOC exchange
  • Basic exchange
  • operations + forward
  • link to wonka docs
  • usage of mergeMap to decide a direction

Sandboxes:

Server: https://codesandbox.io/s/urql-issue-template-server-0ufyz
Client: https://codesandbox.io/s/recursing-shadow-t8b6g

@JoviDeCroock JoviDeCroock requested a review from kitten August 2, 2019 08:26
@andyrichardson
Copy link
Contributor

This looks good! I think we've been needing something like this for a while and the use case is a valid one (although I'm not sure if blocking operations from being forwarded while waiting for async actions is something we want to encourage?).

There's some work here that might be salvageable - https://github.com/FormidableLabs/urql/pull/186/files

Would love to see the docs that you've done get merged 🙌

@JoviDeCroock
Copy link
Collaborator Author

JoviDeCroock commented Aug 2, 2019

Thanks for the feedback @andyrichardson I do think it's needed since ideally when using a
refreshToken mechanism you don't want to request unless your token is refreshed. When it's still valid it will just complete with the fromValue anyway. I understand your concern though, I'll add a comment why this isn't good.

I'll add one of your examples in there as well thanks for the link!

@kitten
Copy link
Member

kitten commented Aug 2, 2019

@andyrichardson @JoviDeCroock I don't think we need to add a comment on this. In a lot of cases there'll be an existing access token mechanism that people will have to work with and refreshing on the fly isn't always an option but trivial to add to this example.

So the main thing I'd like to highlight is that these guides aren't about best practices but just examples so people can write any exchange they need and not just the ones we're writing about :)

It's just not realistic to add a super complex auth example on a guide here

@JoviDeCroock JoviDeCroock marked this pull request as ready for review August 2, 2019 09:29
@JoviDeCroock
Copy link
Collaborator Author

JoviDeCroock commented Aug 4, 2019

I made some small adjustments to the language, removed a link since it was redundant and formatted the long lines.

Copy link
Contributor

@andyrichardson andyrichardson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we could use a different example (such as re-creating one of our existing exchanges) but this is huge progress regardless of the example use case 👍

…this needs to change to the formidable website)
@JoviDeCroock
Copy link
Collaborator Author

Moved the sandbox with the completed refreshToken exchange to the urql codesandbox team

@kitten kitten merged commit c34725b into master Aug 5, 2019
@kitten kitten deleted the docs/customExchanges branch August 5, 2019 14:27
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.

4 participants