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

Fixes Issue 914 #915

Merged
merged 3 commits into from Apr 17, 2021
Merged

Fixes Issue 914 #915

merged 3 commits into from Apr 17, 2021

Conversation

dyedgreen
Copy link
Contributor

Closes #914

This fixes the issue raised and adds a test to prevent future regressions.

Note that this slightly changes to public api of juniper::Object's: add_field will now check if a new field exists and is an object, and in that case merge the objects keys instead of replacing the old object.

It might also make sense to not return anything from that function (which is a breaking change of the public interface, but seems to compile without modification to the existing uses in the juniper crate, I think the behavior was originally copied from std::collections::HashMap).

@dyedgreen
Copy link
Contributor Author

dyedgreen commented Apr 15, 2021

(The CI fail seems to be related to rocket; but I'm not sure if that was caused by my change 😬)

(If I had to guess, there might be a breaking update of some dependencies with rocket:)

    |
616 |         rocket::ignite().manage(Database::new()).manage(Schema::new(
    |                 ^^^^^^ not found in `rocket`

@LegNeato LegNeato merged commit 5ae930b into graphql-rust:master Apr 17, 2021
@LegNeato
Copy link
Member

Thank you for the fix! 🍻 The breakage is indeed some Rocket changes that have broken us.

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.

Multiple fragments on sub types override each other
2 participants