graphql-client
graphql-client copied to clipboard
graphql-rust
Always sort scalar types by name when returning them
Previously these were returned in an essentially random order. This would lead to flapping in the generated code when running the client CLI against the same schema multiple times.
In the application I'm writing, I check in the generated schema, so this sort of flapping in the generated code creates unwanted noise in the commit history.
I tested this by hand by running the fixed client against my schema a number of times. This stopped the flapping in the generated code.
I'm not sure how you feel about the itertools dep. I think the only alternative is to have the scalars method call collect into a Vec, then sort the vec and return sorted_vec.into_iter(). I can implement it that way if you prefer. This is exactly what itertools does under the hood.
Itertools avoids allocating a whole vec, but it's very easy to reimplement (see https://github.com/prisma/prisma-engines/blob/fe68ee16ec7694924b82bf075382fa68e69989da/migration-engine/connectors/sql-migration-connector/src/sql_renderer/common.rs#L92 for example) — I'd rather avoid taking on a new dependency so let's add this trait to graphql-client.
The PR looks good!
I updated the PR to do the sorting "by hand". It still has to allocate the Vec. I'm not sure how the example you linked would help avoid that, since it's generating a string rather than returning a sorted Vec or iterator.
Actually, looking more closely at the code I think it'd be possible to have this problem with inputs and enums too, so I'll do the same thing for them.
The CI failures look to be unrelated to this PR. There is some code in main that isn't properly rustfmt'd. If it got to clippy that would also fail because of some unused struct fields. The prettier failure is also because of files in main that aren't properly formatted.
Is this obsoleted by https://github.com/graphql-rust/graphql-client/pull/430 ?
I don't understand the code base well enough to say if #430 obsoletes this one, but I'm fine with closing this if it does.
Ah, I see. #430 probably does resolve this. Tested on main by fetching the Github schema three times and getting the same result each time.
