feat: add integration of serde_json

[chirino]

Resolves #957
Fixes #280, #504, #509
Superseeds #325

Implements the GraphQLValue trait for serde_json::Value. It's type info is given using an SDL definition.

Checklist

• Implemented:
  • GraphQLType and GraphQLValue traits
  • GraphQLAsyncValue trait
  • IsOutputType marker and IsInputType marker
• Tested as:
  • input
  • field of input type
  • output
  • field of output type
  • subsctiption return type (Strem::Item)
  • in async context
• Described in Book
• CHANGELOG entry added

[tyranron]

@chirino thanks for bringing this up! 🍻

Seems to be a nice implementation idea for a long-waited feature! But more polishing and work should be done to cover all the nececities.

I've corrected the codestyle a bit, linked the related issue/PRs to it and described the checklist of what is awaited to be done for this.

Also, it would be better to mark this PR as draft until it's not ready for review, so I don't review it accidentally!

[chirino]

Also what do you mean by tested as field of input/output type? Does that mean as field of a struct using the #[derive(GraphQLObject)] macro?

[tyranron]

@chirino yes

#[derive(GraphQLInputObject)]
struct InputObj {
    val: serde_json::Value,
}

struct Human;

#[graphql_object]
impl Human {
    fn output() -> serde_json::Value {
    }

    fn input(arg: serde_json::Value) -> bool {
    }

    fn input_object(obj: InputObj) -> bool {
    }
}

[chirino]

Sadly we can't do something as simple as:

#[derive(GraphQLInputObject)]
struct InputObj {
    val: serde_json::Value,
}

Since we need a way to associate type info with the val field. I did included a test that shows how you can create a wrapper type around serde_json::Value that provides that type info.. but it's a bit verbose. Not sure if there is a way to reduce that.

Do you have an example of how you test the subsctiption return type case?

[tyranron]

@chirino

I did included a test that shows how you can create a wrapper type around serde_json::Value that provides that type info.

Does it worth to have such a wrapper predefined?

Do you have an example of how you test the subsctiption return type case?

See integration_tests/juniper_tests/src/codegen/subscription_attr.rs.

[chirino]

@tyranron

I think the PR should have about the right stuff covered in the tests. It introduces a couple of new public types... and naming things is hard so let me know if you can think of better names for:

• TypeInfo
• TypedJsonInfo
• TypedJson

Also not sure where to document this in the book.

[tyranron]

@chirino go with the names you think are best. I'll bikeshed them in the final review if a better naming will come up in my mind.

Also not sure where to document this in the book.

I think it would be nice to create a separate page in advanced topics.

[LegNeato]

Minor nit, but we probably want to match the integration crate name with the integration file name with the flag name. That is, serde_json everywhere instead of json.

[chirino]

renamed and added a section to the book. Is anything else needed?

[tyranron]

@chirino thanks! I'll look at it in few days.

[LegNeato]

This is great work, thank you! ❤️

[tyranron]

Just to keep everybody up-to-date:
This PR is not abbandoned. I'm working on it right now to extend the work dony by @chirino. It will take quite a while, because the rabbit hole appeared to be deeper than expected.

[chirino]

@tyranron thanks for the update. Do you have a branch of your work that I can peek at? Maybe I can find some time to help.

[tyranron]

@chirino unfortunately no, but thank you. You've already done so much 🙇‍♂️

I just need to somewhat extend that, fix some edge cases and polish. It will be harder to explain than do and finish.

[tyranron]

Wow, this PR has turned to be such a huge rabbit hole 😅
It has spotted so many juniper design weak points. And some of them I intend to fix in separate PRs before merging this.

Sorry for moving it slowly, but I'd like to do it the proper way from the start, rather than having it half-baked or redoing in future.

[chirino]

No rush. Glad my "first ever rust PR" is nudging this project even to become even better.

[chirino]

@tyranron hey.. I know this is a complex PR and I'm a noob, but feel free to reach out to me if you need help with it.

[juancampa]

@tyranron are you still planning on working on this PR? Super excited to get serde_json integration

[tyranron]

@juancampa yes, of course. It's scheduled for 0.16.0 release. But it requires first the #1072 to be completed, as at the moment there is no way to combine dynamic GraphQL schema with a static one, due to TypeInfo being an associative type, so allowing no polymorphism.

[chirino]

@tyranron should I close this PR out?

[chirino]

@tyranron should I close this PR out?