workspace dependency breaks default-features = false

[Follpvosten]

Expected Behavior

default-features = false on async-graphql should turn off the email-validator, tempfile, playground and graphiql features on async-graphql.

Actual Behavior

it does – as long as you don't depend on any other projects from the async-graphql workspace.

Steps to Reproduce the Problem

1. create an empty cargo project and add async-graphql with default-features = false to the dependencies.
2. also add a dependency on, for example, async-graphql-axum (which has default-features = false on the workspace dependency, but this does not appear to work)
3. cargo check once, then check the Cargo.lock file to find unexpected packages in there, like tempfile or handlebars.

Specifications

• Version: 5.0.6 – confirmed against git master as well
• Platform: any
• Subsystem: any

Possible solutions

I have verified that locally, I can fix this by changing one line in the workspace Cargo.toml, namely

[workspace.dependencies]
-async-graphql = { path = ".", version = "5.0.6" }
+async-graphql = { path = ".", version = "5.0.6", default-features = false }

however, this is obviously a major breaking change because it will mean that default-features = false works again, so I'm not sure how to proceed here. I also don't know if this change will break anything else.

[frederikhors]

I think this can be closed?

[Follpvosten]

oh great it was released as a minor point release despite being a major breaking change 😓

[Follpvosten]

to make more clear what I mean: there is a public struct field in UploadValue (content) which changes type based on a feature: https://github.com/async-graphql/async-graphql/blob/master/src/types/upload.rs#L11

this feature is on by default, so someone accessing that field directly may have their code randomly break if they have default-features = false suddenly work correctly again

[Follpvosten]

this was also not the case in 4.0 (as the tempfile feature didn't exist yet)

[Follpvosten]

I suggest the latest release be yoinked immediately as to not break people's code when they run cargo update, and then either re-release as 6.0 or make a branch without this change and release that as 5.0.7 @sunli829

[frederikhors]

I think adding a feature in Cargo.toml is not a big issue. Even if this can be called breaking change. Is not something to fix in code (which is definitely more challenging). Am I wrong?

[Follpvosten]

Yeah if you explicitly don't care about following semver, putting this out in a minor release is fine of course.

I'm gonna try to explain better with an example.

Imagine a project with this in the dependencies:

async-graphql = { version = "5.0", default-features = false }

with a graphql handler like this:

async fn some_field(&self, ctx: &Context<'_>, file: Upload) -> Result<Uuid> {
    let file = file.value(ctx)?.content;
    let content = tokio::task::spawn_blocking(move || -> Result<_> {
        let mut buf = Vec::with_capacity(size);
        file.read_to_end(&mut buf)?;
        Ok(buf)
    })
    .await??;
    do_something_else(&content).await
}

when 5.0.6 is in the lockfile, this will compile fine (UploadValue.content is of type File).

once the user runs cargo update, which is not expected to break your code, the lockfile will depend on 5.0.7 it will no longer compile (because UploadValue.content is of type Bytes now and that doesn't have a read_to_end method).

[Follpvosten]

closing in favour of #1263 I guess