krakenjs / swaggerize-express

Design-driven apis with swagger 2.0 and express.
Other
355 stars 81 forks source link

overriding express options #68

Closed ksmithut closed 8 years ago

ksmithut commented 9 years ago

It seems that swaggerize express is overriding some of my options that I set for my express app. I have set json spaces to 2, but it gets overridden here.

Here is my example code:

'use strict';

var http = require('http');
var express = require('express');
var swaggerize = require('swaggerize-express');
var app = express();
var server = http.createServer(app);

app.disable('x-powered-by');
app.set('json spaces', 2);

var compression = require('compression');
var bodyParser = require('body-parser');

app.use(compression());
app.use(bodyParser.json());
app.use(bodyParser.urlencoded({extended: true}));

app.use(swaggerize({
  api: require('./resources'),
  docspath: '/api-docs',
  handlers: './routes'
});

server.listen(process.env.PORT || 8000)
  .on('listening', console.log.bind(null, 'Server started'))
  .on('error', console.log.bind(null));

Expected Behavior:

Api endpoints should have the correctly set json spaces option (for pretty printed json)

Actual Behavior:

Api endpoints are not pretty printed because the setting is getting overridden.

Proposed Solution:

I know that json spaces isn't the only option that is being overriden, but I agree that the ones being overridden are sensible defaults for a majority of people. My only problem with it is that it is unexpected behavior based on my settings. I don't think that you can detect whether or not a setting was set on the user's end, but perhaps just leaving it up to the user to choose the settings would be best.

tlivings commented 9 years ago

This is a good point. While we need to provide sensible defaults, we should provide capabilities to override. This will likely be config options passed to swaggerize().

jdassonvil commented 9 years ago

The view configuration are also overrided to null since merge request #60, what's the motivation for that ? Swaggerize is a REST API tool but it's not uncommon for small services to manage APIs and UI in the same express app. In my opinion this behavior leads to confusion regarding to express documentation.

ksmithut commented 9 years ago

For a workaround, you can put app.use(...); statements after you initialize the swaggerize-express middleware.

tlivings commented 9 years ago

This will be addresses in https://github.com/krakenjs/swaggerize-express/pull/79.

jsdevel commented 8 years ago

See #80. Middleware should never configure express.

tlivings commented 8 years ago

This is addressed in 4.0.5.