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

(core) - Fix stringifyVariables (and thus operation keys) for Files #650

Merged
merged 4 commits into from
Mar 22, 2020

Conversation

kitten
Copy link
Member

@kitten kitten commented Mar 20, 2020

Resolve #649

Summary

When two operations with a File in the variables are sent the operation key would likely be the same since we never take any File attributes into account.

We'd hence like to have special handling for objects without any enumerable keys that also aren't plain objects.

Set of changes

  • Add random (but stable) keying for objects without enumerable keys that aren't plain objects

@kitten kitten requested a review from JoviDeCroock March 20, 2020 16:56
@changeset-bot
Copy link

changeset-bot bot commented Mar 20, 2020

🦋 Changeset is good to go

Latest commit: 6ec3f62

We got this.

This PR includes changesets to release 1 package
Name Type
@urql/core 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
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.

We could also add file.size to avoid the very unlikely scenario of these being equal but the solution in itself is awesome. Great work!

@gabor
Copy link

gabor commented Mar 20, 2020

thanks for looking into this problem. i don't want to complicate things too much, but you can assign Blob objects too,probably, and those do not have names or modification-dates :-(

(as i explained in #649, i would prefer to just drop deduplication for mutations, but still, with this pull-requests, things will be more consistent, and that is a good thing)

@JoviDeCroock
Copy link
Collaborator

thanks for looking into this problem. i don't want to complicate things too much, but you can assign Blob objects too,probably, and those do not have names or modification-dates :-(

(as i explained in #649, i would prefer to just drop deduplication for mutations, but still, with this pull-requests, things will be more consistent, and that is a good thing)

Mutations aren't deduplicated it's a single stream of events, so the response is associated with an operationKey. We need a way to distinguish these things efficiently

@kitten
Copy link
Member Author

kitten commented Mar 20, 2020

We could likely just use ”[Blob ${size}]” for now?

@kitten
Copy link
Member Author

kitten commented Mar 22, 2020

Ok, we've found an alternative. Instead we now generate a random key for objects with non-enumerable keys that aren't "plain objects". So this should support File, Blob, and more

kitten added 3 commits March 22, 2020 20:33
Any object without any enumerable keys that isn't a plain
object will now receive a random key.
@kitten kitten force-pushed the fix/file-hashing branch from d438195 to dfe119f Compare March 22, 2020 20:33
@kitten kitten merged commit e3754da into master Mar 22, 2020
@kitten kitten deleted the fix/file-hashing branch March 22, 2020 20:45
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.

mutations with different files in parallel, results lost
3 participants