Skip to content

(persisted-fetch) - Don't send persisted queries if the hash operation fails#934

Merged
kitten merged 5 commits intourql-graphql:mainfrom
lorenries:main
Aug 7, 2020
Merged

(persisted-fetch) - Don't send persisted queries if the hash operation fails#934
kitten merged 5 commits intourql-graphql:mainfrom
lorenries:main

Conversation

@lorenries
Copy link
Copy Markdown
Contributor

Summary

In insecure browser environments, the window.crypto.subtle api isn't available. The default hashing function in the persisted query exchange will resolve an empty string in this case, and send that empty string as the persisted query, which leads to a PersistedQueryNotFound exception. This is fine, because the exchange will follow up with non-persisted query, but we can improve the default behavior by not sending a persisted query at all if the hash fails.

Set of changes

Updates the persisted query exchange to only send a persisted query if there's a non-falsey hash value. Let me know if there's a better way of implementing this with Wonka!

@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented Aug 6, 2020

🦋 Changeset is good to go

Latest commit: a4e2bbc

We got this.

This PR includes changesets to release 1 package
Name Type
@urql/exchange-persisted-fetch Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link
Copy Markdown
Collaborator

@JoviDeCroock JoviDeCroock left a comment

Choose a reason for hiding this comment

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

This just needs a changeset and this looks good to go. Thank you so much for this contribution

operation,
body,
dispatchDebug,
false
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We could move up the if-block and let this condition be !!((options as PersistedFetchExchangeOptions) .preferGetForPersistedQueries && sha256Hash) 🤔 may not be worth it though

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good idea, that's definitely cleaner!

@kitten kitten merged commit 1651fd5 into urql-graphql:main Aug 7, 2020
@kitten
Copy link
Copy Markdown
Member

kitten commented Sep 8, 2020

This has now been released 🙌 Sorry for the delay and thanks for the contribution! ❤️ https://github.com/FormidableLabs/urql/releases/tag/%40urql%2Fexchange-persisted-fetch%401.2.1

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.

3 participants