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.

Sign up for GitHub

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

Jump to bottom

Fields Queried are not returned in the same order #1

Closed
e-lobo opened this issue Sep 27, 2022 · 3 comments · Fixed by #2
Closed

Fields Queried are not returned in the same order #1

e-lobo opened this issue Sep 27, 2022 · 3 comments · Fixed by #2

Comments

@e-lobo
Copy link

e-lobo commented Sep 27, 2022

In the below image the products field is a Dataloader hence execution is deferred until the end. Do notice in the query at line 4 where the products is supposed to be resolved.

image
@jkimbo
Copy link
Owner

jkimbo commented Sep 27, 2022

Interesting I'll take a look @e-lobo . Is field order important to you though? I'm trying to understand when this might be problem.

jkimbo added a commit that referenced this issue Sep 28, 2022
@jkimbo
Preserve field ordering by adding placeholder values for deferred fields
3555cc5 
During execution if a field returns a future the value doesn't get
assigned to the results dictionary until after that future is resolved.
So if there is a mix of deferred and non-deferred fields in a query the
non-deferred fields will always get returned first regardless of the
request ordering.

This PR fixes this issue by adding a placeholder to the result object
that gets overridden when the future gets resolved. That way field
ordering is preserved.

Fixes #1

Diff-Id: 94549
@jkimbo jkimbo mentioned this issue Sep 28, 2022
Merged
@jkimbo jkimbo closed this as completed in #2 Sep 28, 2022
@e-lobo
Copy link
Author

e-lobo commented Sep 28, 2022

e the products is supposed t

not a issue although according to the specs i guess it should resolved as per the query..

@jkimbo
Copy link
Owner

jkimbo commented Sep 28, 2022

@e-lobo it should be fixed in 0.1.1

tony pushed a commit to tony/graphql-sync-dataloaders that referenced this issue Oct 21, 2023
@Arno500
Merge pull request jkimbo#1 from bellini666/main
689762e 
feat: use contextvars to separate batch callbacks execution between multiple threads/contexts
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
2 participants
@jkimbo @e-lobo