Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
ilkkao opened this issue Jul 26, 2016 · 2 comments
Closed

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

ilkkao opened this issue Jul 26, 2016 · 2 comments

Comments

@ilkkao
Copy link

ilkkao commented Jul 26, 2016

Related to this: nodejs/node#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
Copy link
Contributor

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
Copy link
Author

ilkkao commented Jul 26, 2016

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.

shellscape added a commit that referenced this issue Oct 24, 2016
fixes #52: passing viewPath array and no layoutPath
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants