-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Persisted document custom hash function #9996
Persisted document custom hash function #9996
Conversation
🦋 Changeset detectedLatest commit: d4f4468 The changes in this PR will be included in the next version bump. 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 |
@nahn20 Can you please also add a changeset for these changes? |
What would happen if the hash function returns the same hash for multiple operations? Should we assert this? |
I think that considering how advanced of a feature that this is, any developer using a custom function should know how to design an effective hash function. I'd be comfortable leaving in this footgun for the sake of full customizability, to account for potential use-cases that we aren't thinking of. I also think that the cost of a bug here is minimal due to this potential issue being easy to debug. If a developer implements a poor hashing function, then they'll find their GraphQL calls aren't working. They would disable persisted documents, then see they are working again. From there, I have full confidence a developer can debug it down to multiple equal hash outputs by analyzing the mismatched operations. Additionally, I think this would add a false sense of security. Not all approved operations are guaranteed to be present at generation time (for example, a backend which remotely stores operations from previous client versions). This functionality is also easy for a developer to implement if they want the extra safety. I don't think this is necessary to build into the preset itself. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your contribution!
Description
This features allows for customization of the
hashAlgorithm
configuration for persisted documents in the client preset. This mitigates the need to support additional future algorithms beyondsha1
andsha256
and gives developers full flexibility for generating document hashes, unlocking new use-cases.Related #9994
Type of change
How Has This Been Tested?
Added three new test cases to
client-preset.spec.ts
. Ranyarn test client-preset
.custom hash remove whitespace
- This test case demonstrates basic functionality in a human-readable manner. Since the output is just the operation without whitespace, it's easy to tell if the custom hash function is broken.custom hash sha256
- This test case replicates the existing test casehashAlgorithm="sha256"
to ensure identical results.custom hash docs sha512
- This test case demonstrates a realistic, real-world use-case and was chosen as the example for the docs.Test Environment:
@graphql-codegen/...
:Checklist: