Skip to content

Add JSON or Map type #298

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

Closed
januszm opened this issue Mar 18, 2018 · 6 comments
Closed

Add JSON or Map type #298

januszm opened this issue Mar 18, 2018 · 6 comments

Comments

@januszm
Copy link

januszm commented Mar 18, 2018

There's a (still) open issue at graphql/graphql-spec#101

The point is that there are many situations where having the Map type in GraphQL would save a lot of time and effort required to return the jsonb records or other no-sql like data. It's not only about preparing the response on the server side, but it'd also facilitate querying (no need to add 'sub selections').

If I understand correctly, it is required that we have to either return a scalar (boolean, number or string), list or enums or we have to define custom types but after all they have to return simple scalars (like in the StarWars example, character's fields are all strings or numbers of episodes). What's more, we have to specify all the fields in the query.

I can imagine that we could introduce a "convenience" type, JSON or Map, without breaking the rules. I'm wondering if it would be possible to have a graphql.Json or graphql.Map type that would facilitate returning arbitrary JSON objects to clients. There are plenty of valid use cases for this, eg.

type Document struct {
    ID       uuid.UUID      `gorm:"column:id" json:"id"`
    Title    string         `gorm:"column:title" json:"title"`
    Details  postgres.Jsonb `gorm:"column:details" json:"details"`
}
// or just string instead of Jsonb

... // code omitted
    "details": &graphql.Field{
        Type: graphql.Map,
        Resolve: ...

It should produce something that json.NewEncoder(w).Encode(result) would properly Marshal into {"foo":"bar"} without escaping quotes (like it does if you simply return the jsonb column as a string

@januszm
Copy link
Author

januszm commented Mar 18, 2018

In Ruby (https://github.com/rmosolgo/graphql-ruby) we can 'workaround' this issue by doing:

  field :details, types.String do
    resolve ->(obj, args, ctx) { obj.details.to_json }
  end

and luckily the Rails server is NOT escaping the resulting json string. It seems like a side effect but after all it saves the day. The json string created with to_json appears in the response as is, without escaping quotation marks.

For JavaScript we have graphql-type-json which adds a new scalar type.

In our case it'd be a new type var and coerce function defined in https://github.com/graphql-go/graphql/blob/master/scalars.go ?
eg.

func coerceMap(value interface{}) interface{} { // or coerceJson
...
var Map = NewScalar(ScalarConfig{ // or var Json
  Name: "Map", // or Json
  Serialize: coerceMap,
  ParseValue: ...,
  ParseLiteral: ...,

@bbuck
Copy link
Collaborator

bbuck commented Mar 19, 2018

My 2 cents here:

If you want to add a new type to handle this, then more power to you, you do you. As a native type to GraphQL though, I don't agree. And that discussion isn't really appropriate here. This project's goal is to maintain the current official GraphQL spec so if you can get a Map/JSON type added to the spec you'll see it crop here.

As a general note, if you want to return arbitrary data from an endpoint, we already have great methods for doing so, just don't use GraphQL for that data. The point of GraphQL is structure, and I don't consider having part of the return object being a JSON object in itself handles "structure." I could fetch a JSON field from a type and then have no idea what is possibly being returned from in each record, what is the point there? You're storing internal meta-data that is either repeated enough to define an actual object for it, or not where you need a separate endpoint to fetch metadata. In your example your GraphQL field for details could be a URL to hit with the JSON dump, perfectly fine. Sure you have to make two requests, but it's better than polluting your GraphQL schema with unknown data.

Since GraphQL's purpose is to provide a clear and structured means to interact with website data, I don't think I'll ever agree that breaking those points for convenience is ever a good idea. Frankly it would become a leaky abstraction. You can use GraphQL regardless of the data source (flat files, structured database, graph database, memory, etc...) and having a JSON/Map field is exposing your implementation details to the consumer which is a pretty big code smell IMO.

Again, if this is what you want -- I'd say find some way to do it without modifying the actual GraphQL library (via scalar, strings, etc...) but my recommendation is either a secondary route that just returns the JSON blob or structuring it with your Schema.

P.S. It looks like the Ruby example is the result of a bug and in no way intentional. I wouldn't write any code that depended on that or used that as a reference for desired behavior. The Javascript example being an outside type is exactly my point, if you want it, add it yourself. This shouldn't be something that becomes an officially supported part of GraphQL since it goes against the grain of what GraphQL is intended to do.

@januszm
Copy link
Author

januszm commented Mar 19, 2018

@bbuck thank you very much for the detailed response.

This project's goal is to maintain the current official GraphQL spec
so if you can get a Map/JSON type added to the spec you'll see it crop here.

The above actually answers my questions. I wanted to know what's graphql-go guys take on this. It feels like we can close this issue and wait for the resolution (or not) of graphql/graphql-spec#101

Nonetheless, there's a reason why that facebook/graphql issue is still open.

I agree with some of your arguments about leaky abstraction, etc. I mean, there are also good reasons why this type should be available. It would save time and help in rapid prototyping. Used with caution, it will not pose a threat. E.g. would never us this approach to serve user generated content.

I will stick to the current solution, which is to return the valid JSON and GeoJSON (shapes) objects as strings and let clients parse them. The project I'm currently working on is still in the prototype phase and that is why some information is still stored/returned as JSON objects. Making separate round-trips to the server to fetch additional JSON data, which would require an additional API endpoint (which would require additional maintenance) would be unreasonable in terms of time, budget and project status.

Thanks again for fruitful discussion!

@januszm januszm closed this as completed Mar 19, 2018
@bbuck
Copy link
Collaborator

bbuck commented Mar 20, 2018

Heh, if I had a nickel.

It would save time and help in rapid prototyping. Used with caution, it will not pose a threat.

@januszm
Copy link
Author

januszm commented Mar 20, 2018

Remember what they say, a dull knife is more dangerous than a sharp knife.

@bhoriuchi
Copy link
Contributor

bhoriuchi commented Sep 27, 2018

very lightly tested

import (
	"fmt"

	"github.com/graphql-go/graphql"
	"github.com/graphql-go/graphql/language/ast"
	"github.com/graphql-go/graphql/language/kinds"
)

func parseLiteral(astValue ast.Value) interface{} {
	kind := astValue.GetKind()

	switch kind {
	case kinds.StringValue:
		return astValue.GetValue()
	case kinds.BooleanValue:
		return astValue.GetValue()
	case kinds.IntValue:
		return astValue.GetValue()
	case kinds.FloatValue:
		return astValue.GetValue()
	case kinds.ObjectValue:
		obj := make(map[string]interface{})
		for _, v := range astValue.GetValue().([]*ast.ObjectField) {
			obj[v.Name.Value] = parseLiteral(v.Value)
		}
		return obj
	case kinds.ListValue:
		list := make([]interface{}, 0)
		for _, v := range astValue.GetValue().([]ast.Value) {
			list = append(list, parseLiteral(v))
		}
		return list
	default:
		return nil
	}
}

// JSON json type
var JSON = graphql.NewScalar(
	graphql.ScalarConfig{
		Name:        "JSON",
		Description: "The `JSON` scalar type represents JSON values as specified by [ECMA-404](http://www.ecma-international.org/publications/files/ECMA-ST/ECMA-404.pdf)",
		Serialize: func(value interface{}) interface{} {
			return value
		},
		ParseValue: func(value interface{}) interface{} {
			return value
		},
		ParseLiteral: parseLiteral,
	},
)

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

No branches or pull requests

3 participants