add hooks to remove view configuration settings made on parent express application

[tongfa]

I had to prevent swaggerize-express from configuring views, view cache, view engine on the express app in order to get the API to work in the same express app as our webserver. We only plan on running this way for development. What was happening is our template engine settings would get overwritten and then express couldn't serve our handlebars templates.

What I didn't know is is why those settings were placed there in the first place. I assumed it was to deliberately disable any view stuff based on the assumption the API runs all by itself within express. And that these are important settings. I relied on those assumptions when adding to the README. If I'm off base, I'd be willing to take a second pass at this.

[tlivings]

Why not just express options? This object can contain any overrides you like to the default behavior.

[tongfa]

I took your suggestion to heart and interpreted it this way. I renamed the object to expressOptions. And then I eliminated the function that deleted the properties, and moved that code into the readme as an example to show how to manipulate the expressOptions object.

[tlivings]

Sorry, not sure I was clear. What I am saying is, there is already a documented option for swaggerize for providing express option overrides.

Can you not simply use that?

[tongfa]

In short, no, using the express option is not sufficient because the damage is already done before the value passed into this option is utilized. The default express options built into swaggerize-express/lib/index.js overwrites the view, "view cache", and "view engine" express settings before the object passed in as express is even considered.

I'll admit I could examine how those express settings were set prior and then restore them. But this doesn't seem like an ideal approach IMO. It would be better to "just work" out of the box with a template engine such as handlebars if that is possible.

Backing up a little bit... Why are the view, "view cache", and "view engine" values set on the express app in the first place? If they are not very important, perhaps the best thing to do is simply remove them from the set of default settings.

If they are very important then I would recommend redesigning such that the set of default options passed into express are exposed for manipulation outside swaggerize-express. This will give outer layers full control over how these express are configured, including even whether they are configured in the first place, and yet allow a reasonable set of defaults for those who have no need to change them.

[tlivings]

You can pass them to this module as options as documented in the API.

If you do this, these will be as overrides immediately following the default settings such that you can undo whatever settings you didn't like.

Your PR seems like duplication of this behavior.

Example:

        Object.keys(settings = {
            'x-powered-by': false,
            'trust proxy': false,
            'jsonp callback name': null,
            'json replacer': null,
            'json spaces': 0,
            'case sensitive routing': false,
            'strict routing': false,
            'views': null,
            'view cache': false,
            'view engine': false
        }).forEach(function (option) {
            parent.set(option, settings[option]);
        });

        Object.keys(options.express).forEach(function (option) {
            parent.set(option, options.express[option]);
        });

[tlivings]

Wait - I see your concern, sorry.

I don't think this should be a big deal though. The express defaults should already be known no?

http://expressjs.com/api.html#app-settings

[tongfa]

thanks for seeing my concern :-)

To be clear I don't need to go back to the express defaults, I need to go to the values set by another framework component (handlebars). IMO, the best way would be to be able to choose handlebars, and to choose swaggerize express, I wire them both up in my server.js, and it just works. But instead, the two framework components disagree about the three express settings, and the one who sets them last wins, leaving the other one in a very broken state.

I'm going to send you another PR that is very simple, it will just delete the three properties that seem to be the root of the conflict from swaggerize-express. I think it's a better approach than what I've done on this one, as long as it's reasonable to sacrifice these options. I feel my experience level is not sufficient to make this call, so I will be curious to hear your opinion on it.

[tongfa]

I just tried it, the suggestion to run in production mode doesn't seem to change anything besides masking the error the browser. Same call stack is showing up from node.

On Sun, Nov 6, 2016 at 12:34 PM, Shaun Warman notifications@github.com wrote:

It looks like you want to run this in development mode. But could you take advantage of production settings for express overrides.

Note that sub-apps will:

• Not inherit the value of settings that have a default value. You must set the value in the sub-app.
• Inherit the value of settings with no default value; these are explicitly noted in the table below.

Exceptions: Sub-apps will inherit the value of trust proxy even though it has a default value (for backward-compatibility); Sub-apps will not inherit the value of view cache in production (when NODE_ENV is “production”).

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/krakenjs/swaggerize-express/pull/115#issuecomment-258704349, or mute the thread https://github.com/notifications/unsubscribe-auth/AGchPsf-v_RYTh1vOFjCTkKTgQoixa4Fks5q7ivSgaJpZM4Kn_JB .