search

alexsteinerde / graphql-kit

Easy setup of a GraphQL server with Vapor. It uses the GraphQL implementation of Graphiti.
MIT License
99 stars 18 forks source link

add register overload for variadic PathComponent #19

Closed jaredh159 closed 1 year ago

jaredh159 commented 2 years ago

In one of my apps, I'm wanting to mount several GraphQL resolvers for different API consumers, and to implement some path-based API versioning (like /graphql/macosapp-05-2022 and /graphql/dashboard), but the current register(...) function doesn't allow passing through variadic path components. Because you can't have default arguments for variadic parameters in Swift, I couldn't just changed the type to PathComponent..., so I added an overload.

The diff is noisier than it should be, because I ran swift-format on it, and it broken things up a little. I didn't touch the first function at all, it's just re-formatted.

If you'd prefer me to undo the formatting changes, I'm happy to do that, just let me know.

Or, if you don't think this is a change you want in the main library, or have a better alternative on how to implement it, I'm all ears.

alexsteinerde commented 2 years ago

Thanks for the PR. I would like to understand why you would want to have the versioning/grouping at the end instead of e.g. /dashboard/graphql and /macosapp-05-2022/graphql. This could already be achieved with app.grouped("dashboard").register(...). The official GraphQL documentation uses /graphqlat the end as well and I would like to keep this aligned with that. If this doesn't meet your requirements I would prefer to keep reformats and functional changes as much as possible separated from each other. So I would like to ask you to undo that reformatting for this PR, please.

jaredh159 commented 2 years ago

@alexsteinerde you know, I hadn't even thought of the fact that I could achieve the same thing by nesting my routes with app.grouped, thanks for pointing that out.

with that workaround, I'm comfortable closing this PR, based on the reasons you outlined in your comments.

that said, if you think the overload is still generally useful, i'm happy to remove the formatting and reduce the duplication and resubmit. whatever seems good to you.

thanks again for the great library, i'm using it in production for two different apps, and it's been working great for me. 🙏