Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create basic example follow http://graphql.org/docs/getting-started/ #21

Merged
merged 5 commits into from
Oct 30, 2015

Conversation

thanhliem89dn
Copy link

Just port a simple example from JS into GoLang follow http://graphql.org/docs/getting-started/
I hope it will make everyone easy to start to learn about Graphql by using graphql-go and understand how to port js schema sources into Golang

@sogko
Copy link
Member

sogko commented Oct 16, 2015

Hi @thanhliem89dn

Thanks for sharing your time to contribute this PR 👍🏻

I think thats a great idea having more examples that others can use to draw parallels with the official docs.

/cc: @chris-ramon: What do you think?

@chris-ramon
Copy link
Member

Thanks a lot @thanhliem89dn! 🌟

I've tried locally and get seems working as expected but not post test.

Test with Get: http://localhost:8080/graphql?query={user(id:%221%22){name}}

screen shot 2015-10-18 at 18 39 07


Test with Post: curl -XPOST http://localhost:8080/graphql -H 'Content-Type: application/graphql'  -d 'query Root{ user(id:'1'){name}  }'
{
  "data": {
    "user": null
  }
}

Also it think we might want to pluralize example -> examples, so we can include other examples in this directory as well.

* Test with Post command
* Change [example] -> [examples]
@thanhliem89dn
Copy link
Author

Thanks for your time to take a look on this pull request.

Replace '1' in [curl -XPOST http://localhost:8080/graphql -H 'Content-Type: application/graphql' -d 'query Root{ user(id:'1'){name} }'] with "1"
===> curl -XPOST http://localhost:8080/graphql -H 'Content-Type: application/graphql' -d 'query Root{ user(id:"1"){name} }'

It'll be ok :). And I also fixed curl command in example in new commit beside changed example to examples.

@chris-ramon
Copy link
Member

Great! 🌟 - thanks for updates @thanhliem89dn

Just tried and both test are working as expected:

Test with Get: http://localhost:8080/graphql?query={user(id:%221%22){name}}

screen shot 2015-10-19 at 01 52 16


curl -XPOST http://localhost:8080/graphql -H 'Content-Type: application/graphql'  -d 'query Root{ user(id:"1"){name}  }'
{
  "data": {
    "user": {
      "name": "Dan"
    }
  }
}

Also just for consistency could we use curl for get as well, like:

curl -g "http://localhost:8080/graphql?query={user(id:%221%22){name}}"
{
  "data": {
    "user": {
      "name": "Dan"
    }
  }
}

Finally what do you think if we go for snake case naming so SimpleUserGraph -> simple_user_graph and rename simpleUserGraph.go -> main.go

cc/ @sogko

@thanhliem89dn
Copy link
Author

In my opinion, I think camelCase is better snake_case. About rename simpleUserGraph.go -> main.go, It's a good idea :)

@chris-ramon
Copy link
Member

Alright, thanks @thanhliem89dn, I think we are close to merging this one, we could do the following:

  • Rename example -> examples - to place other examples within this directory.
  • Rename simpleUserGraph.go -> main.go - to make it clear where program begins.
  • gofmt
  • goimports
$GOPATH/bin/goimports -d simpleUserGraph.go
diff simpleUserGraph.go gofmt/simpleUserGraph.go
--- /var/folders/v7/ssjjclsd4ng3l03hjpk19fs80000gn/T/gofmt350536176 2015-10-19 15:27:13.000000000 -0300
+++ /var/folders/v7/ssjjclsd4ng3l03hjpk19fs80000gn/T/gofmt848666255 2015-10-19 15:27:13.000000000 -0300
@@ -3,10 +3,11 @@
 import (
    "encoding/json"
    "fmt"
-   "github.com/chris-ramon/graphql-go/types"
-   "github.com/sogko/graphql-go-handler"
    "io/ioutil"
    "net/http"
+
+   "github.com/chris-ramon/graphql-go/types"
+   "github.com/sogko/graphql-go-handler"
 )

 /*

Finally I think we could rename SimpleUserGraph to http, so this one becomes the http example.

cc/ @sogko

-Rename SimpleUserGraph to http
-Goimports
@thanhliem89dn
Copy link
Author

Can't agree more with you. I have changed all of them. Please take a look @chris-ramon @sogko

@chris-ramon
Copy link
Member

Great! thanks @thanhliem89dn 🌟 - to keep consistency could we use curl for get test as well?.
So we can have something like:

go run main.go 
Now server is running on port 8080
Test with Post: curl -XPOST http://localhost:8080/graphql -H 'Content-Type: application/graphql'  -d 'query Root{ user(id:"1"){name}  }'
Test with Get: curl -g "http://localhost:8080/graphql?query={user(id:%221%22){name}}"

@sogko
Copy link
Member

sogko commented Oct 21, 2015

Thanks for spending time to contribute this @thanhliem89dn, this looks great!

@thanhliem89dn
Copy link
Author

thank for your time @chris-ramon, I have just edited curl command for Get example. I think it's nice. One more thing, Why don't We open a git room for communicate :)

@chris-ramon
Copy link
Member

Alright, thanks again @thanhliem89dn! - yes! good idea to have a chat room, I think we could start by using gitter, here the PR.

I would like to share my final thoughts about this, merging this PR will make graphql-go depend on sogko/graphql-go-handler, when should be the opposite?.

Perhaps this example should be on sogko/graphql-go-handler and we can create a link reference within graphql-go's README?.

Other solution could be merge both projects graphql-go and graphql-go-handler, which for me seems ideal since we will always want use a http handler for our graphql schemas, and we could have a constructor like:

handler := gql.NewHandler()

As @sogko pointed out here.

Please do share your thoughts about this one, thanks! 👍

@sogko sogko mentioned this pull request Oct 27, 2015
@thanhliem89dn
Copy link
Author

Thanks for your time to reply,

But I think We shouldn't merge both projects [graphql-go] and [graphql-go-handler]. In my point of view, We almost use http handler but use my own, so keep [graphql-go] separate to [graphql-go-handler] or [relay] is better. So We can use it in any purpose also implement in any old project.

About my example I think It's a very first look for everyone to interact with [graphql-go] as GraphQL official document. But As you told It used [graphql-go-handler] to handle request so I think it's better if I take [graphql-go-handler] away.

@chris-ramon
Copy link
Member

Alright, yeah I agree @thanhliem89dn, perhaps we should preserve graphql-go and graphql-go-handler as separated libs by now, in that case we could to the following:

  1. Have a reference on graphql-go README to a new repository that you would need to create, something like: thanhliem89dn/graphql-go-examples/http.
  2. Remove graphql-go-handler dependency and use plain net/http.

@sogko sogko mentioned this pull request Oct 28, 2015
@thanhliem89dn
Copy link
Author

Hi @chris-ramon, I have just taken [graphql-go-handler] out of example and just use net/http. I think it's clear for anyone to start with this RP.

@chris-ramon
Copy link
Member

Thanks for the updates @thanhliem89dn ! - I've tested locally and is working great, I think is ready to go.
👍

/cc: @sogko

@sogko
Copy link
Member

sogko commented Oct 29, 2015

I've just tested it on my local machine as well, looks great!

Thanks @thanhliem89dn! Looking forward to more contributions from you 👍🏻

chris-ramon added a commit that referenced this pull request Oct 30, 2015
@chris-ramon chris-ramon merged commit 3b4b43f into graphql-go:master Oct 30, 2015
sogko added a commit that referenced this pull request Jun 11, 2016
Alternative concurrent evaluation of fields
sogko added a commit that referenced this pull request Jun 11, 2016
… from here

for perf-related PRs.
- simple hello world equivalent
- with a list
- deeper query with list
- query with many root fields and list in each of them
- deeper query with many root fields and lists

To really bring out probable issues with query perf, a latency is
introduced to the data retrieval (previously it was just in-memory data fetch)

When testing against branch before PR #20 and #21, this really highlighted the problem with evaluating list fields.

In the future, more benchmarks for queries with fragments probably would be useful.
@base698
Copy link

base698 commented Dec 16, 2016

How do you parse a schema? In the node version you pass a string and get a schema object that you can then initialize as a middleware?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants