Something better than req.accepts('json') && !req.accepts('html') ??

[binarykitchen]

Hello again :)

In lots of places in my app I have this line

if (req.accepts('json') && !req.accepts('html')) {
    // process the GET request from superagent (client)
    ...
    res.json(200, data);
} else {
    // middleware further down will load the basic page layout
    next();
}

The reason why I need !req.accepts('html') is because of the single page app structure. In other words, when the URL is called for the first time, the whole page layout has to be loaded first.

And when loaded, then the controller on the client side will fire the GET request (XHR) where a JSON is returned and data is finally presented on the page.

I might be correct there and the above code is fine. But I think there is a bad code smell around req.accepts('json') && !req.accepts('html'). Do you think so?

[dougwilson]

You probably want this:

switch (req.accepts('html', 'json')) {
  case 'json':
    // process the GET request from superagent (client)
    // ..
    res.json(200, {});
    break;
  default:
    // middleware further down will load the basic page layout
    next();
    break;
}

[jonathanong]

yeah i dunno why you would ever want to do that... do a switch

[dougwilson]

...and the bonus of using the switch method above means you are interpreting the Accept header correctly, unlike the original if statement (the if statement will execute the else statement for Accept: application/json, */*;q=0.1, for example, which is probably not what you wanted to happen).

[binarykitchen]

Hmmm... @dougwilson I do not know. I am not a fan of switch blocks because they are not OO friendly and sometimes complicate things.

How about something like req.acceptsOnly('json')? That would be neat!

[jonathanong]

that's not how content negotiation works though

[dougwilson]

@binarykitchen a req.acceptsOnly('json') would still return false for Accept: application/json, */*;q=0.1, which is unlikely what you want. Also, there is nothing "not OO friendly" about switch statements. They are basically fancier if statements. You can always write it as an if if you like...

if (req.accepts('html', 'json') === 'json') {
  // process the GET request from superagent (client)
  // ..
  res.json(200, {});
} else {
  // middleware further down will load the basic page layout
  next();
}

but that if pattern will not scale very well if you want to add more.

[binarykitchen]

Ah, I see now ... thanks so much!

Can I somehow plug in a new method to the request instance through the whole express app which looks like this:

req.wantsJson = function() {
    return this.accepts('html', 'json') === 'json')
}

Where could I do that? I am keen to avoid code duplication in all my controllers hence the need for such a common function.

[dougwilson]

function wantsJson() {
  return this.accepts('html', 'json') === 'json';
}
app.use(function (req, res, next) {
  req.wantsJson = wantsJson;
  next();
});

is the best way to do it. Note that having the wantsJson outside the middleware is how it will perform well since it doesn't create a new function for every request.

[binarykitchen]

Thanks so much @dougwilson - after all your help I owe you one beer!

[dougwilson]

:beer: