Added the ability to specify the JSON encoder#25
Conversation
rwe
left a comment
There was a problem hiding this comment.
I'm just a bystander here, but this change looks good to me. My comments aren't actual issues, just suggestions.
| pretty = self.pretty or show_graphiql or request.args.get('pretty') | ||
| if not pretty: | ||
| return json.dumps(d, separators=(',', ':')) | ||
| return json.dumps(d, separators=(',', ':'), cls=self.json_encoder) |
There was a problem hiding this comment.
Any reason to continue using json.dumps here rather than the encoder directly?
return self.json_encoder(separators=(',' ':')).encode(d)I believe they're functionally equivalent but the .encode() route reduces the dependency on the builtin json module, since there are several json libraries that could be reasonably used: http://artem.krylysov.com/blog/2015/09/29/benchmark-python-json-libraries/
There was a problem hiding this comment.
I'm not an expert on json encoding/decoding in Python. What you've said sounds great but I'd rather leave this to someone in a better position to comment.
| graphiql_template = None | ||
| middleware = None | ||
| batch = False | ||
| json_encoder = None |
There was a problem hiding this comment.
Should this also include a json_decoder member? Roughly equivalent change with the json.loads -> self.json_decoder(…).decode(s) a couple places below.
There was a problem hiding this comment.
Completely agree, I've just updated the branch with json_decoder support.
3 similar comments
|
@syrusakbary This PR looks good, is anything preventing it from merging? |
|
Hi @erydo @Mleonard87, Thanks for taking time for making this PR. I think the serialization/deserialization is better to be assumed by the scalar type itself rather than the transport formatting language ( However I might be missing some of your context here. Would be great if you could provide an example of a Scalar type that could not be created without the "customize json serialization class" approach. |
|
@syrusakbary I think you're mostly right, but a couple considerations:
Error serialization doesn't go through the Scalar system, though I think pretty much everything else does. |
|
For error serialization, you can provide a custom error function But then the solution for that is not a custom decoder for |

json.dumps()allows for a JSON encoder to be specified by theclsargument. This PR allows for a JSONEncoder to be passed through when constructing the GraphQLView object.Supporting this PR allows for serialization of custom types which can be particular useful when defining custom graphene scalar types.