Set Consolidate options.settings.views property by default

[etherealite]

The search path for the directory containing template files for most of the engines supported by Consolidate is configured by setting the options.settings.views for the options parameter in:

consolidate.engineName.render(string, options)

If you take a look at the source for consolidate.js in the Consolidate project, you will see numerous references to options.settings.views

In the case of invoking koa-views with the typical options of:

app.use(views(__dirname, { map: {html: 'nunjucks' }}))

If you inspect index.js line #46 you will see that the Nunjucks FileSystemLoader constructor is called with undefined parameters nunjucks.FileSystemLoader(undefined, undefined) by Consolidate consolidate.js line #1167 for every call to koa-views' ctx#render in down stream middleware.

When the Nunjucks FileSystemLoader constructor is invoked with an undefined argument for the template folder path, it falls back on process.cwd in order to determine the template file search path. Since the process.cwd can be literally ANYWHERE on the system, extending parent templates doesn't work unless you give the path relative to the node process.cwd in every extends directive. I don't know about everyone else, but I don't' want my template directives looking like: {% extends "have/to/change/this/everytime/i/run/node/from/a/differnt/direcory.html" %}

I didn't create any tests for this PR since I wasn't in the mood for mocking Consolidate and doing funky stuff like changing the node process working directory in order to reproduce this issue in tests. It will have to suffice that this PR solves the issue and passes the existing test suite.

[etherealite]

Hmm, I'm just now realizing that my code overwrites any existing values on options.settings passed to consolidate. I'll have to make an amendment to deal with this case. I'll update the PR when I get to it sometime within the next couple of days.

[int64ago]

Hi, thanks for the PR and explanation, I personally don't want to patch for a specific template engine.

92 maybe an imperfect solution.

[etherealite]

The way I see things, supporting passing a custom instance of nunjucks.Environment through options.options.nunjucksEnv via the koa-views middleware factory function is what's keeping my patch from being engine agnostic.

If I could get the guys behind Consolidate to allow the nunjucks engine to be configured in a manner consistent with how they do it for the Dust engine (via options.views) then I could change my code from

          if (!options.hasOwnProperty('nunjucksEnv')) {
             state.settings = { views: path }
           }

to something like

            if (!options.hasOwnProperty('views')) {
              state.views = path
            }

Would that be acceptable? The only other solution I can think of would be to write an update to the readme warning people who are using Nunjucks that they are required to set the views directory twice in the manner that you describe in #92.

[int64ago]

Consider the scene: Use an engine but not Nunjucks or Dust, and pass settings or views as custom variable, the patch will overwrite the variable.

I think an update to readme is more acceptable.

[etherealite]

I could fix the concern of overwriting variables passed in through locals on line 21 by adding an extra check. This could be done by making sure state.views would only be set when the expression !options.hasOwnProperty('views') && !locals.hasownProperty('views') was true. It would still require a patch in Consolidate though so that makes it even less worth pursuing.

Alright, I see what you mean. I guess I'll close this PR and make a new one for the readme changes.