Use impl Future instead of async-trait in most traits.

[fiadliel]

This updates most traits to return impl Future + Send instead of using the async-trait crate.
It retains async-trait for extensions and custom directives, both of which use dynamic dispatch.
Also keeps async-trait for the web server integrations, as none of them have switched yet.

As it stands, this would bump the MSRV to 1.75. Is this acceptable in an upcoming release, or do you want this kind of feature gated behind a feature flag?
It adds a bit of complexity for the user that two traits still use async-trait. I guess one might be able to capture the extension/directive types in a heterogeneous list to fix this, but I haven't thought too much about it yet.

The static schema benchmark reports almost 20% performance improvement (MacOS on M1 Pro):

Running benches/static_schema.rs (target/release/deps/static_schema-9cbe34cce2287750)
Static Schema           time:   [129.63 µs 130.20 µs 130.99 µs]
                        change: [-19.631% -19.268% -18.905%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 5 outliers among 100 measurements (5.00%)
  2 (2.00%) high mild
  3 (3.00%) high severe

[Miaxos]

Nice!! It seems there are some issues to fix with the CI.

[fiadliel]

Updated, this should 🤞 fix all the issues other than the examples (which need #[async_trait] removed from the dataloader examples, but that change would break them for the current code). I'll do a PR for this if/when you decide you want to merge this.

[fiadliel]

Just to test with some different profile settings, with lto = "thin" and codegen-units = 1, the mean time decreases from ~139.95 µs (without this PR) to ~114.27 µs (with), fairly similar improvement:

Static Schema           time:   [114.19 µs 114.27 µs 114.35 µs]
                        change: [-18.443% -18.226% -17.993%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 10 outliers among 100 measurements (10.00%)
  6 (6.00%) high mild
  4 (4.00%) high severe

[sunli829]

After I rebase to the master branch, the test test_impl_dyn_trait cannot pass, can you help me check it? I feel related to #1470. 🙂

[fiadliel]

@sunli829 It's a good test - it fails because it's new, and shows a real behavioral change. The test doesn't pass because impl Future are not object-safe, so one can't use dyn any more (this is why extensions/directives are still using async_trait in this PR).

Right now, unfortunately, it's a choice between this PR and trait objects.

I think because a user will have a closed set of types, one could replace dyn with an enum:

struct MyObj(String);
struct MyObj2(String);

enum Objs {
    MyObj(MyObj),
    MyObj2(MyObj2),
}

#[Object]
impl Objs {
    #[graphql(name = "name")]
    async fn gql_name(&self) -> &str {
          match self {
            Objs::MyObj(o) => &o.0,
            Objs::MyObj2(o) => &o.0,
        }
    }
}

struct Query;

#[Object]
impl Query {
    async fn objs(&self) -> Vec<Objs> {
        vec![
            Objs::MyObj(MyObj("a".to_string())),
            Objs::MyObj2(MyObj2("b".to_string())),
        ]
    }
}

But it's up to you whether the change is worth the cost here, 🥲

[sunli829]

impl for any lifetime

[drager]

Shouldn't this have been a breaking change? My application that was running used version 1.72 of Rust and was unable to run after this change. async-graphql 7.0.2 was brought in via auto dependencies update. I had to update Rust to 1.76 (which is good though). But it was not expected from a patch change.

[xamgore]

@sunli829 I double drager, an accident cargo update bumped the version from 7.0.0 to 7.0.2 and failed the code compilation.

[noehoro]

We are having this issue and it is stopping us from being able to compile any of our code, I attempted to pin the version to 7.0.0, with this:
async-graphql = { version = "=7.0.0", features = ["dataloader"] }

But some of the sub dependancies like async-graphql-derive, async-graphql-parser, or async-graphql-value are still being automatically updated to 7.0.2.

Any solutions out there?

[eopb]

@noehoro you'll need to make all of those transitive dependencies direct dependencies of your project. Then you can pin them to 7.0.1.

You can find all the ones you need by running cargo tree -p async-graphql and looking for crates with version 7.0.2

[noehoro]

@eopb Thank you 🙏

[fenhl]

I agree that this should have been a breaking change as it broke my build even though I'm on the latest release of Rust. Fortunately, in my case, it was easy to fix by removing the #[async_trait] attributes from the Guard implementations.

[fiadliel]

As the original submitter, apologies for the problems. I wonder if I should have highlighted better that this was a breaking change. (and I agree this shouldn't have been in a point release)

[Oliboy50]

for those who implemented the Guard trait and want to fix the breaking change from 7.0.1 to 7.0.2:

error[E0195]: lifetime parameters or bounds on method `check` do not match the trait declaration
   --> my-file.rs:178:14
    |
178 |     async fn check(&self, ctx: &Context<'_>) -> Result<()> {
    |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ lifetimes do not match method in trait

here you go:

- #[async_trait::async_trait]
  impl Guard for MyStruct {
      async fn check(&self, ctx: &Context<'_>) -> Result<()> {

EDIT: just saw that someone already mentioned that... but I didn't see it before fixing my code, so I hope my comment will be easier to see 👀 🤞

[Sytten]

I think there should be a feature flag to keep the async_trait since the lack of boxing clearly is problematic for me some people (including me). Code doesn't compile at all now because of #1516