Module naming #19

teh · 2016-12-16T11:39:48Z

What do you think about this naming? In any case, we have a branch to discuss :)

jml · 2016-12-16T11:58:13Z

I've been thinking about this a bit.

We need to start from the perspective of the end user. What modules will they need to import? Are there pairs of modules that they'll always need to import together? What modules will they never need to import? What will they probably almost never need to import?

For example, I'm not sure I believe in the separation of Schema and Value. I'm not sure I don't either. I certainly don't know why a user would want them.

This is why I think starting work on something like the servant tutorial (and/or worked examples in the docs) would be a good thing to do now. It will teach us what our structure should be like.

Haddock has a Description field for modules. This shows up in the link to the module on the module index page. I think we should come up with such a description for all of our modules at the same time we come up with names. The description should say why someone should care about that module.

What I want isn't just better module names, but rather a good, clear, and coherent terminology for the whole project. Module names are simply the first terms that our users are exposed to.

Shorter ideas:

Let's move everything that's internal into an Internal package
I think API is more common Haskell module name than Api, and is to be preferred
Perhaps the TypedAPI & TypedSchema modules should be in a sub-package. Thus, GraphQL.Typed.API and GraphQL.Typed.Schema

I'm sure I had other thoughts, but I cannot recall them now. No doubt there'll be an opportunity 😏

teh · 2016-12-16T14:03:47Z

If you have some time: I added a skeleton tutorial can you test whether you can build it? I don't know how people use pip/python these days so you might have to install some tools.

jml · 2016-12-16T15:22:12Z

Will do as soon as I may. Got distracted cleaning up imports and really do need to get back to work. Further thoughts on naming: * 'Value' should be 'Literal' (or 'Literals' or 'LiteralValues'), and all the "value" names in it renamed to "literal" (so we can drop this "GValue" nonsense elsewhere in the code)

…

On Fri, 16 Dec 2016 at 14:03 teh ***@***.***> wrote: If you have some time: I added a skeleton tutorial can you test whether you can build it? I don't know how people use pip/python these days so you might have to install some tools. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#19 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAHq6mUdju25TxLZofq-jNsr2IesFopoks5rIppEgaJpZM4LPGuv> .

jml

This is a really great start, thanks!

I don't know exactly what state we need it to be in before merging. My comments range in scope quite a great deal.

jml · 2016-12-17T10:30:39Z

docs/source/tutorial/Introduction.lhs

+type HelloWorld = Object "HelloWorld" '[]
+  '[ Argument "greeting" Text :> Field "me" Text
+   ]
+```


The lexer chokes on this for me. not sure there's much we can do about it.

Same, nothting we can do other than upstream fix and wait a year

https://bitbucket.org/birkenfeld/pygments-main/issues/820/haskell-highlighting-promoted-types - bug from 2012. Will try to pick that up

https://bitbucket.org/birkenfeld/pygments-main/pull-requests/685/default/diff

jml · 2016-12-17T10:32:51Z

docs/source/tutorial/Introduction.lhs

+actions** when the field hasn't been requested. We only run the action
+if the query specifies the field. Note that lazy evaluation would not
+work because of the monadic nature of handlers.
+* Finally, we have to terminate each handler with `:<> ()`. This is an


We talked about providing a helper function that did:

fieldList xs = xs :<> ()

Which would make it look less weird.

Yea we did but I'm not sure we can write one. We probably need the representation to be right-side nested tupels (that's why :<> is infixr), but fieldList re-orders the tuple nesting. I.e.

fieldList (a :<> b) == (a :<> b) :<> ()

but we need

a :<> (b :<> ())

I had figured out why we need infixr but I forgot why. I might also be wrong.

Oh yea, I remember: We want the left-most item to be a "normal" Field or Object, and the right hand side to be a composite so we can unpack handlers recursively.

OK. This is a big deal to implement and maybe not worth it for our users, so leave it here for this branch.

Continuing in #31 for further ergonomics discussion.

jml · 2016-12-17T10:35:00Z

docs/source/tutorial/Introduction.lhs

+
+## The handler
+
+We defined a corresponding handler via the `HandlerType m a` which takes


Looking at this, we should probably rename this to just Handler.

jml · 2016-12-17T10:39:53Z

graphql-api.cabal

      GraphQL.Input
      GraphQL.Output
      GraphQL.Schema
-      GraphQL.TypeApi
+      GraphQL.TypedApi
+      GraphQL.TypedSchema


How's this?

GraphQL.API -- How to define a GraphQL API (currently TypedSchema)

GraphQL.Server -- How to implement handlers for a GraphQL API (currently TypedApi)

GraphQL.Literals -- Literal values in GraphQL (currently Value)

GraphQL.Internal

Input

Output

Schema

Validation

+1 except for Literals because it has quite a different connotation (e.g. [1]). Not sure what else to call it though. Aeson has value in the Types module but that doesn't seem great either. Brainstorm:

Raw

Direct

Encoding

EncodedValue

Values

Representation

ValueTree

It's the in-memory intermediate representation before/after serialization. Not sure what the usual name is.

[1]
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Grammar_and_types

Glad you like most of these names. Do you want to do that rename in this PR? (I think you should, but am also happy to see a different PR).

Re Value, I think they sort of are literals, because they are derived from http://hackage.haskell.org/package/graphql-0.3/docs/Data-GraphQL-AST.html#g:4 but with Variable taken away. To put another way, a constructed Value cannot be resolved any further. It is terminal. I guess we could call it Terminal or Terms.

Leave Value as-is for now until we figure out something we both like.

jml · 2016-12-17T10:40:05Z

src/GraphQL/TypedApi.hs

@@ -70,6 +71,11 @@ queryError = throwM . QueryError
 data a :<> b = a :<> b
 infixr 8 :<>

+-- | Union type separation operator.
+data a :<|> b = a :<|> b
+infixr 8 :<|>


jml · 2016-12-17T10:47:31Z

docs/source/tutorial/Introduction.lhs

+* The `pure` in the function call allows us to **avoid running
+actions** when the field hasn't been requested. We only run the action
+if the query specifies the field. Note that lazy evaluation would not
+work because of the monadic nature of handlers.


I don't know what that means. Can you construct an example (for my benefit, if not the docs), that demonstrates why laziness isn't enough?

{-# LANGUAGE NoImplicitPrelude #-} {-# LANGUAGE OverloadedStrings #-} {-# LANGUAGE TypeApplications #-} module Main (main) where import Protolude foo :: IO () foo = putStrLn @Text "foo" bar :: IO () bar = putStrLn @Text "bar" main :: IO () main = do let actions = [foo, bar] let (Just action) = head actions action

Produces:

$ ./lazyactions foo

Your example is what we are doing right now. We only sequence (i.e. execute the field) like you do with action when the field name matches. The (fieldname, action) tuple is constructed in the lazy GHC execution model.

What I am trying to express is that we need the handlers to be monadic so we can do IO. Not sure how to express.

I'm still not following. The type of actions is [IO ()], whereas your example says that it has to be IO [IO (), IO ()] in order to not evaluate everything.

I still would like to see a minimal example demonstrating why you think laziness isn't enough (in fact--we should have integration tests demonstrating this behaviour--not hard to do!), but not necessary for this branch. We can leave the prose as is & refine later.

The outer IO:

IO [IO (), IO ()] ^^

is a once-per-object IO action, e.g. we could open a DB connection, or do a single query to satisfy all fields.

The inner IO actions are only executed if the field name matches. We should chat in person, I'm not sure where we talk past each other :)

I see now. I produced the following minimal example to demonstrate:

{-# LANGUAGE NoImplicitPrelude #-} {-# LANGUAGE OverloadedStrings #-} {-# LANGUAGE TypeApplications #-} module Main (main) where import Protolude foo :: IO Text foo = do putStrLn @Text "foo" pure "foo" bar :: IO Text bar = do putStrLn @Text "bar" pure "bar" allActions :: IO (Text, Text) allActions = do putStrLn @Text "all actions" (,) <$> foo <*> bar main :: IO () main = do result <- fst <$> allActions putStrLn $ "result = " <> result

I'd suggest something like:

Each handler is a separate monadic action so we only perform the side effects for fields present in the query.

Nice, updated.

jml · 2016-12-17T10:49:01Z

docs/source/tutorial/Introduction.lhs

+
+```haskell
+handler :: HandlerType IO HelloWorld
+handler = pure $ (\greeting -> pure (greeting <> " to me")) :<> ()


Does this mean if a user wanted to implement a totally pure GraphQL handler they could say Handler Identity HelloWorld?

Sadly no, because I added a MonadIO constraint everywhere. It might be possible to remove that constraint, but I'm a bit unclear ATM what guarantees we need from the underlying modad.

Not for this PR.

There are two things we can do that I can think of right now:

if we never use the IO nature of the monad, then we can reduce the constraint to Monad (after we drop MonadThrow)

we can have a natural transformation thing like servant.

Both too big for this PR.

jml · 2016-12-17T10:55:48Z

Can confirm that I can build sphinx docs, but cannot build the tutorial docs.

I've sent an email to the servant mailing list asking how they do it: https://groups.google.com/forum/#!topic/haskell-servant/qxNBq11adBI

As a way forward, I think we should:

get the sphinx machinery in asap (ideally integrating with CI but I can do that as follow up)
do whatever non-controversial renames there are, but don't sweat the rest (google doc perhaps?)
not worry too much about getting the tutorial right, as it's something we can iterate on (although I'm so glad you started on it, because it's already easier to think through some of this stuff). Main thing is that what's checked in is true & readable.

jml · 2016-12-17T11:01:26Z

Also docs/build/ needs to be added to .gitignore

teh · 2016-12-18T18:08:25Z

rebased, docs/build/ should be in a gitignore already - not working for you?

jml · 2016-12-19T10:33:30Z

rebased, docs/build/ should be in a gitignore already - not working for you?

Nope.

jml@wit:~/src/graphql-api (module-naming) (0)
$ git rev-parse HEAD
b80ebaa75ddfd1988632cb5bcd880ab3f8eb5372
jml@wit:~/src/graphql-api (module-naming) (0)
$ cat .gitignore 
/.stack-work
jml@wit:~/src/graphql-api (module-naming) (0)
$ git status
On branch module-naming
Your branch is up-to-date with 'origin/module-naming'.
nothing to commit, working tree clean

jml

Would like a quick look again before merging.

teh · 2016-12-19T10:52:23Z

re .gitignore, see: https://github.com/jml/graphql-api/pull/19/files#diff-14b597b3f3e040934bdb59aa959344a7 - it's not the top-level .gitignore

jml · 2016-12-19T10:54:00Z

re .gitignore, see: https://github.com/jml/graphql-api/pull/19/files#diff-14b597b3f3e040934bdb59aa959344a7 - it's not the top-level .gitignore

Oops, my bad. Works for me now.

teh force-pushed the module-naming branch from 0b7f725 to 20d22f5 Compare December 16, 2016 11:40

jml added this to the Initial release milestone Dec 16, 2016

teh force-pushed the module-naming branch from 1a6f3e6 to 699610e Compare December 16, 2016 16:53

jml reviewed Dec 17, 2016

View reviewed changes

teh added 9 commits December 18, 2016 17:44

Proposed module naming changes.

4991507

Add servant-style sphinx tutorial.

df1b84d

Start writing some basic docs.

e289946

Document handler.

9628dce

Rebase.

6d881a1

Make sense.

77f0327

wording.

c4aea70

Move the :<|> operator to TypedApi, as we did with :<>

c233954

Rename HandlerType to Handler.

b80ebaa

teh force-pushed the module-naming branch from 3d6f2e5 to b80ebaa Compare December 18, 2016 18:07

jml reviewed Dec 19, 2016

View reviewed changes

Rename to agreed schema.

5c0ed2d

jml approved these changes Dec 19, 2016

View reviewed changes

Update tutorial with jml suggestion.

04ae7cf

teh merged commit dfdabeb into master Dec 19, 2016

teh deleted the module-naming branch December 19, 2016 12:22

jml mentioned this pull request Dec 19, 2016

Sphinx docs #15

Closed

jml mentioned this pull request Jan 2, 2017

Rename 'Values' to 'Const' #50

Closed


		## The handler

		We defined a corresponding handler via the `HandlerType m a` which takes

Module naming #19

Module naming #19

Conversation

teh commented Dec 16, 2016

jml commented Dec 16, 2016

teh commented Dec 16, 2016

jml commented Dec 16, 2016 via email

jml left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jml commented Dec 17, 2016

jml commented Dec 17, 2016

teh commented Dec 18, 2016

jml commented Dec 19, 2016

jml left a comment

Choose a reason for hiding this comment

teh commented Dec 19, 2016

jml commented Dec 19, 2016