koajs / koa-hbs

Handlebars templates for Koa.js
MIT License
159 stars 43 forks source link

path.join() doesn't allow array parameter in Node 6 #52

Closed ilkkao closed 7 years ago

ilkkao commented 7 years ago

Related to this: https://github.com/nodejs/node/issues/1215

koa-hbs converts user given viewPath to an array if it's a string:

if (!Array.isArray(this.viewPath)) {
  this.viewPath = [this.viewPath];
}

Then in getLayoutPath(), if layoutsPath is not defined, this.viewPath is given as a parameter to path.join. Which in latest Node leads to:

TypeError: Path must be a string. Received [ '/Users/ilkkao/git/mas/server/views' ]
    at assertPath (path.js:7:11)
    at Object.join (path.js:1213:7)
    at Hbs.getLayoutPath (/Users/ilkkao/git/mas/node_modules/koa-hbs/index.js:219:15)

https://github.com/gilt/koa-hbs/blob/master/index.js#L219 line seems problematic. It doesn't do the right thing if the viewPath is an array even with node <6 I think. Older node versions return a comma separated list of paths. That will lead to file not found.

shellscape commented 7 years ago

this is a good find, thanks. not sure if I'll have time to get tests in place and make a fix until the weekend - if you'd like to create a PR in the meantime it'd be welcome!

ilkkao commented 7 years ago

In my app I fixed this by switching from the problematic config:

app.use(hbs.middleware({
  defaultLayout: 'layouts/main',
  viewPath: path.join(__dirname, 'views')
}));

to

app.use(hbs.middleware({
  layoutsPath: path.join(__dirname, 'views/layouts'),
  defaultLayout: 'main',
  viewPath: path.join(__dirname, 'views')
}));

This should be a straightforward workaround. So not that critical to make the real fix quickly. I may have free time, not sure.