Apollo-server: Assume query if neither 'query' or 'mutation' present at top level?

[lorensr]

I'm not sure whether this is standard, but might be nice:

Current behavior

With query

Works:

$ curl -XPOST -H "Content-Type:application/json" -d '{ "query": "{ posts { title } }" }' https://runkit.io/lorensr/apollo-endpoint-trial/2.0.0/graphql
{"data":{"posts":[{"title":"First"},{"title":"Second"}]}}

Without query

Doesn't work.

application/json:

$ curl -XPOST -H "Content-Type:application/json" -d '{ posts { title } }' https://runkit.io/lorensr/apollo-endpoint-trial/2.0.0/graphql
Bad Request

application/graphql:

$ curl -XPOST -H "Content-Type:application/graphql" -d '{ posts { title } }' https://runkit.io/lorensr/apollo-endpoint-trial/2.0.0/graphql
{"errors":[{"message":"Cannot read property 'definitions' of undefined"}]}

Code:

https://runkit.com/lorensr/apollo-endpoint-trial/2.0.0

Perhaps related: #112 #109

[stubailo]

We intentionally don't support application/graphql because it doesn't seem right to create a new MIME type and it's impossible to send variables, which are a critical feature of GraphQL.

However, the following query:

{
  posts {
    title
  }
}

Is definitely supported, perhaps there is something invalid about that CURL command? If you open GraphiQL and send that query it definitely works correctly.

[lorensr]

CURL looks good. Perhaps it's the fact that { posts { title } } isn't valid JSON? Maybe GraphiQL is autowrapping in { "query": "X" }?

[helfer]

I think the "query" part here is a bit misleading. Maybe it should have been called "document" instead, but people have used the terms interchangeably. We intentionally support only JSON requests, and the "query" field can be a query or a mutation.

[papandreou]

We intentionally don't support application/graphql because it doesn't seem right to create a new MIME type and it's impossible to send variables, which are a critical feature of GraphQL.

Hmm, I'm not sure I follow that. The application/graphql Content-Type is already "out there" and used in tutorials, and express-graphql supports it. It'll probably be brought up here again and again.

I found that using application/graphql is really neat when writing tests is nice because:

• I don't need really need variables in tests, it's easier to read my test cases when the values are directly in the query
• It allowed me to implement syntax highlighting of the requests based on the Content-Type when I have a failing test (via unexpected-express)

To get it working anyway I whipped up this quick-and-dirty middleware and mounted it before graphql-express-server:

function rewriteApplicationGraphQlToJson(req, res, next) {
    if (req.is('application/graphql')) {
        const chunks = [];
        req
            .on('data', chunk => chunks.push(chunk))
            .on('error', next)
            .on('end', () => {
                req.headers['content-type'] = 'application/json';
                req.body = {
                    query: Buffer.concat(chunks).toString('utf-8'),
                    variables: {},
                    handler: null
                };
                next();
            });
    } else {
        next();
    }
}

[papandreou]

Here's what my tests look like:

it('should support querying the session TTL of the logged in user', function () {
    return expect(`
        {
            getSession {
                error { type }
                session { ttl, username }
            }
        }
    `, 'to yield response', {
        data: {
            getSession: {
                error: null,
                session: {
                    username: '[email protected]',
                    ttl: 321
                }
            }
        }
    });
});

And here's the output I get when a test fails:

[stubailo]

Sounds good, we still don't think it's a good idea for a GraphQL server to accept more content types than it needs to though. I'm happy that there is a middleware workaround!

[helfer]

@papandreou I'm pretty sure there's also an option you can pass to bodyParser to let you do this.

[willxy]

Fwiw, when I switched the other week from express-graphql and missed the support for application/graphql (for me, it was easier for generating test queries from bash/curl), what seemed to do the trick for me was essentially:

app.use(bodyParser.text({ type: 'application/graphql' }));
app.use('/graphql', (req, res, next) => {
        req.body = {query: req.body};
        next();
});

[helfer]

@papandreou what @willxy did is exactly what I was referring to. A much simpler solution than writing a custom middleware! 🙂

[papandreou]

@willxy @helfer Thanks, guys! I had no idea that body-parser could do that. That's indeed much simpler. You should add it to the README so users won't keep asking for direct application/graphql support :)

[helfer]

@papandreou that's a great idea about adding it to the docs! You wouldn't mind making a PR, would you!? :pray:

[papandreou]

@helfer Sure thing: https://github.com/apollographql/graphql-server/pull/318