graphql-rust / juniper

GraphQL server library for Rust
Other
5.7k stars 421 forks source link

Custom scalar integration policy / decide what (if anything) to do about smaller crate integrations #235

Open LegNeato opened 6 years ago

LegNeato commented 6 years ago

See https://github.com/graphql-rust/juniper/pull/165#issuecomment-416403522.

There will always be a push and pull of wanting to put integrations in juniper vs keeping them out.

I think this will come up more and more as Juniper gets popular, especially once we go full async and show some benchmarks compared to other languages 😜 . We should probably have some "official" policy of what gets added or not.

Thinking out loud, perhaps we should have an unstable-integrations flag that we can put less popular crates behind. We would have no contract for keeping them release-to-release and deleting them would not impact semver but people can opt into using them at their own risk. Users would be acutely aware that if we delete the integration they need to copy the code into their own project and/or create their own integration crate.

sporto commented 6 years ago

I would like to see most types supported in Diesel also supported in Juniper. E.g. BigDecimal. As Diesel is a very likely crate that people will use with Juniper, I think maintaining a close parity make sense.

weiznich commented 6 years ago

Also see #232.

theduke commented 6 years ago

The biggest challenge is the maintenance burden of version upgrades of dependencies. We already had that situation with uuid.

We need a solid strategy for dealing with this, especially when Juniper approaches a 1.0.

With Uuid and chrono we have been able to get a way with having a version range, since the API surface we need is usually very shallow and rarely changes. If it were to change though, we would need features like uuid-06, chrono-07, etc. Which are also a bit awkward long term because we can never get rid of them until a a new major version.

I'd propose to remain very conservative with adding integrations. We should only add dependencies if they are very stable, widely used or really essential for the domain (as in, most of projects will need it). Everything else will be a big headache.

I currently don't see (m)any crates that would fit this definition.

Making newtypes easier

Using the newtype pattern and defining wrapper types is admittedly pretty annoying, no doubt about that. But it's also not that bad. Especially since, at least in my experience, you end up needing custom types just for the API anyway, since they usually look subtly to not so subtly different from the types used in DB or application logic layers. I think it's also usually a good pattern to do this, to enforce separation between your db, business logic and API code base.

We can make using wrapper types quite a bit easier though! We could have a macro like newtype_scalar! ( or a custom derive) that makes implementing a wrapper type very effortless, with automatic From / Into implementations etc.

If we then have a good documentation page in the book for this pattern, it should be enough for most use cases where we feel uncomfortable with adding the dependency.