Support codegen from query string

[dphm]

Context

Our crate generates the GraphQL query that uses the macro to generate Rust types. We want to avoid writing the generated query to a temporary file.

Proposal

• Refactor graphql_client_codegen and expose a function to generate module token stream from a query string (instead of a query path)
• Maintain backwards-compatibility with the existing generate_module_token_stream

[tomhoule]

I don't have context on the change but that made me think, now that #[graphql(query = include_str!("./my_query.graphql")] is possible (pseudo-code, and I haven't checked, but pretty sure it works now), couldn't we make this the one and only way to inject the query? It would be a breaking change, of course, but we can have a deprecation cycle.

[dphm]

@tomhoule Oh! I didn't know that existed. I agree that would be a better way to include the query from a file. Do you know if that would still work with all the file path logic (CARGO_MANIFEST_DIR)?

Seems like I could still make the changes to accept a query string instead of the file, right?

[dphm]

For some early 👀 and any thoughts! @surma @nickwesselman @andrewhassan @adampetro

[tomhoule]

Do you know if that would still work with all the file path logic (CARGO_MANIFEST_DIR)?

The logic is different: the paths in the proc macro are relative to CARGO_MANIFEST_DIR, but include_str!() takes a path relative to the file it is located in.

To address the questions in the PR description

Should backwards-compatibility be maintained, or is a breaking change acceptable?

It should be backwards compatible at first, unless we decide to make the new attribute argument the only way, but progressively, deprecating the old argument.

Should the schema use the same mechanism? Is the size concerning?

The requirement is new to me, so I don't know. I haven''t looked too deep into your use case, but since this is done at compile time, maybe depending on the graphql_client_codegen crate directly would be easier, since it wouldn't involve the derive macro at all, and you can pass the string directly in there, so you probably wouldn't need to make any change to graphql_cilent.

[surma]

Great stuff 🎉 Left some comments! Hope that’s helpful.

Would be great to also see a test for query_string.

[dphm]

Ah, I removed the changes to graphql_query_derive so I'll move the test out of graphql_client to graphql_client_codegen

[dphm]

depending on the graphql_client_codegen crate directly

Is this something we want to do?

[tomhoule]

Is this something we want to do?

I can't speak for shopify, but I think that if I had more time to work on this crate, I'd like it to become more modular. The core value proposition is the codegen, and that's just a fn(graphql_query: &str, graphql_schema: &str, config: Config) -> String function — we shouldn't force people to use a derive macro to take advantage of it. If it's hard to depend on graphql-client-codegen directly, we could make it easier.

[dphm]

graphql-client/graphql_client_codegen/src/lib.rs

Lines 5 to 7 in a7283cb

	//! Crate for internal use by other graphql-client crates, for code generation.
	//!
	//! It is not meant to be used directly by users of the library.

Sounds like we should align on a public interface for using the codegen crate directly, and then I can update this comment 😆

[dphm]

🤔 We are using the derive though, and it would be nice to have the query string literal work. I could make that change in a separate PR.

Oh, I guess we would replace the entire quoted derive with the call to generate module tokens from graphql_client_codegen.

[tomhoule]

Oh, I guess we would replace the entire quoted derive with the call to generate module tokens from graphql_client_codegen.

Yes, that's what I had in mind.

[andrewhassan]

Hey @tomhoule @surma! 👋 Would it be possible to get your 👀 on this PR again? Thank you!! 🙏

[surma]

LGTM!