express icon indicating copy to clipboard operation
express copied to clipboard

Add "view options" settings for rendering engines

Open nvlled opened this issue 6 years ago • 2 comments

Avoids cluttering app.locals, and is consistent with the assignment of the "view engine" settings:

app.set("view options", {
    basedir: "somedir"
});
app.set("view engine", "pug");

The intent is also stated more clearly in comparison to:

app.locals.basedir = "somedir"

nvlled avatar Apr 02 '18 13:04 nvlled

Seems reasonable. Would this only apply to the set default view engine, or to all view engines (and if so, how would conflicting options be resolved)? Please add some tests when you find the time 👍

dougwilson avatar Apr 02 '18 13:04 dougwilson

Yeah, it applies to all view engines, although I've only looked and modified the app.render code, so I may not be entirely sure. ~~It's basically an alias for the app.locals.~~ I'll do a double check.

Edit: Yeah, it applies to all view engines, not just the default one. It merges the options in the following order:

  1. app.locals
  2. app.settings["view options"] (this is what is added)
  3. renderOptions (from app.render(name, renderOptions, callback) { ... })

The latter ones override the former. I'm not sure if the added test suffice or they are in the right place.

nvlled avatar Apr 02 '18 13:04 nvlled