Use classes to access context #690

Open
wants to merge 1 commit into
from

Projects

None yet

3 participants

@phadej
Member
phadej commented Jan 30, 2017 edited

As an example what I mean in #689: We don't need to force which construct is used as context. Users should be free to pick their own "extensible-records" library (if they want to use one).

@phadej phadej added the rfc label Jan 30, 2017
@jkarni
Member
jkarni commented Jan 30, 2017

I don't mind the proposal, but I don't also see any huge gain. This is hardly the sort of access pattern that really requires sophisticated record libs (you just construct the datatype once, and never again modify it)

(Presumably we don't want to actually depend on lens just for this?)

@phadej
Member
phadej commented Jan 30, 2017 edited

lens: well we don't need to. There wasn't setters at all previously. The only lost feature is Inject, but I have hard time to come up with generally useful combinator requiring it.

The gain: it's much more approachable (especially if we don't use lens, but only getter functions) to people who aren't comfortable with HLists. And for them who are, we don't force them to use our implementation.

At work I construct Ctx with connections pools etc, which I partially apply to handlers (maybe I should use enter runReaderTNat, but anyway). So to get e.g auth working, i'd just write an instance HasAuthCheck Ctx and pass it to serveWithContext...

The enter runReaderTNat and serverWithContext context solve about the same problem, but quite differently. I'm not sure if MonadReader context (Handler context) (with context :: * as here) is a good idea, but maybe it is.

TL;DR it's hard to balance between "simplicity of implementation" and "not much boilerplate", but for that reason lens has makeLenses for example. We could have too deriveDefaultServantHasClasses.

@jkarni
Member
jkarni commented Jan 30, 2017

Hmm, interesting. On the one hand I think configuring handlers and combinators are distinct enough to be kept as separate notions, but on the other I do see how it'd be useful if the same datatype could be used for both so that one ends up with a single configuration datatype.

I'm overall coming over to this idea, but still have a question. Let's say we have a default config - the one one serve uses. We have a datatype for it, and instances for all the classes our base combinator instances need. If there's one more combinator that needs something else in the context, it seems like doing that all of a sudden gets a lot harder. Previously you'd just prepend the new data. Now you need to declare a new datatype, and a whole lot of new instances. This is solved decently well by makeClassy, but now we're back to using lens (or microlens).

@phadej
Member
phadej commented Jan 31, 2017 edited

@jkarni: we could still provide hlist-Context, at least for some time. And/or write own TH method for generating boilerplate: if it's as simple "derive servant built-in classes, except ones in this list", it's very trivial.

So it would be better to have only getters, as then () could be the default.

@phadej
Member
phadej commented Feb 1, 2017

I'll improve this after 0.10 is out. Showing code will probably help.

@alpmestan
Contributor

I have a few ideas in that space I'd like us to experiment with, so I agree that we should postpone this to after 0.10.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment