ericf / express-handlebars

A Handlebars view engine for Express which doesn't suck.
BSD 3-Clause "New" or "Revised" License
2.31k stars 382 forks source link

Add support for array as views #126

Open sundarj opened 9 years ago

sundarj commented 9 years ago

Fixes #124.

I've added a fix based on your help, and now when views is an array, res.render checks both folders (I've tested). I've tried to match the existing codebase/style as closely as possible, but there's probably still a few things you can fix (like the _getView comment).

Hope it's good enough!

ericf commented 9 years ago

I'm actually wondering if getting the view's "name" (the value passed to res.render()) is unknowable when the app is using multiple views dirs.

I'm not sure what should be considered the best match. We can use path.relative() on each of the views dirs to get back possible values to use as the view's "name", and that will work on Windows, but what should be the criteria to pick a winner?

sundarj commented 9 years ago

Ah, so if there happen to be multiple .handlebars files named the same thing in different views? Perhaps an error message asking one of them to be renamed, or giving each a unique name, e.g. res.render('index-0'), would work?

ericf commented 9 years ago

If you have the following views dirs:

  1. /path/to/views/
  2. /path/to/views/pages/
  3. /path/to/other/views/

And the view that's being rendered is at: /path/to/views/pages/home.hbs

Using file.indexOf(dir) === 0 you'd get the following results:

  1. true
  2. true
  3. false

Using this._getTemplateName(dir, file) on the true results above, you'd get the possible view names:

  1. pages/home
  2. home

So the question is which one of these do you pick? Is it simply the shortest one? I think that in this case it would be reasonable to pick #2.

sundarj commented 9 years ago

Yeah, that's why I used the strip-filename regex. With those three views dirs, you'd only get one possibility: /path/to/views/pages/. But I do agree it'd be reasonable to pick #2, and it could be overridden by specifiying pages/home, I suppose?

ericf commented 9 years ago

and it could be overridden by specifiying pages/home, I suppose?

Nope it can't because no matter what's specified to res.render(), Express always passes the full path to the view file into the template engines.

HenrikGr commented 9 years ago

Is this fixed? If so, which version do I need?

samuelfine commented 8 years ago

Taking a step back: much of this would be resolved if view.render() simply included the name of the view, right? View.prototype.render in /express/lib/view.js has access to this.name, but doesn't pass it along to this.engine(). If we knew the view name, we'd know—in the context of @ericf's example—that the view is, in fact, pages/home and not home.

duncanhall commented 8 years ago

Is there any update/decision on this? Restricting to a single views folder is really quite limiting.

vanraizen commented 8 years ago

Just ran into this issue today as well, would also like to know if there are any updates.

hguillermo commented 7 years ago

I really need this feature too. @ericf are you planning to merge this? are you waiting for something?

hhope1271 commented 7 years ago

Also really need this feature.

I noticed after going through express-handlebars.js that nothing seems to need the view property. By removing that property and commenting out this line: view = this._getTemplateName(path.relative(viewsPath, viewPath));, then express-handlebars seems to work just fine with an array of view paths.

My question is: Is there any reason I shouldn't be pulling out the view property of the options object?

gabrielerzinger commented 5 years ago

Any updates on this? Did anyone cloned this repo and merged this...?

UziTech commented 3 years ago

Development has moved to https://github.com/express-handlebars/express-handlebars/ if you would like to create a PR on that repo we could add this.