RandomEtc / ejs-locals

Express 3.x layout, partial and block template functions for the EJS template engine.
298 stars 63 forks source link

Make use of Express@2.5 style of relative path for partials. #20

Closed kc-dot-io closed 9 years ago

kc-dot-io commented 11 years ago

Recently upgraded from express 2.x to 3.x and found that having this small change in place really eased the transition for me because I didn't have to go add preceding slashes to every partial / include in my site. We could change to use options.settings.views instead of options.settings['views'] if you want to be really particular. Otherwise the only other thing I had to change was res.render('x',{ layout: 'y' }) is now: res.render('x',{ _layoutFile: 'y' }) ...easy to replace using sed -i so not a big deal.

Thanks for this repo. It was a huge help in migrating from 2.x to 3.x, hopefully this patch helps someone else down the line who also might have piggy-backed of the app.use('views', xxxx) feature of express 2.x like I did to augment relative paths for partials.

ralyodio commented 11 years ago

+1 if this also fixes the partial including a partial problem of looking in the wrong folder.

RandomEtc commented 11 years ago

Thanks for this. I'm a little behind with fixes here but I'll try to take a look this weekend.

kc-dot-io commented 11 years ago

No problem. It can probably be flushed out a bit and I am happy to do so and resubmit based on feedback. I tried to improve the _layoutFile override as well without conflicting with layout() but caused a never ending recursion so I'll save that for another day. Let me know if you have thoughts on that though because it was the secondary pain point when upgrading.

RandomEtc commented 11 years ago

I'm evil. I think I might have just made things worse for you...

I added a test in 40a7ef274acf95fd8d420cd739dae58356ed3dfa that loads a partial in a subdirectory, which in turn loads another partial. In this case the use of a subdirectory breaks your patch. I think the behavior of the new test is correct though...

I think if you want absolute paths for partials we still need to get options.settings.views in there as the root, as you did, but it would need to be applied only if the requested partial starts with /, as I do for layouts. It sounds like you were relying on different behavior in Express 2.0 - do you think I got my new test wrong? (Genuine question, not rhetorical).