search

RandomEtc / ejs-locals

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

Views / layouts relative to a set path #4

Closed GothAck closed 11 years ago

GothAck commented 12 years ago

It'd be nice to be able to reference a view / layout from a set path e.g.

set an option for the views root to be /project/root/views render('localthing') => /project/root/views/local/localthing.ejs render('/rootthing') => /project/root/views/rootthing.ejs

Hope that makes sense, would solve having to use relative paths when layouts are stored in one central folder.

RandomEtc commented 12 years ago

Sorry for slow response. I'm not sure I understand here.

For the first one, I think if your views are in /views then render('localthing') should find /views/localthing.ejs. I'm not sure how it would know to look in the local folder?

For the second one, I think if your views are in /views then render('/rootthing') should already find /views/rootthing.ejs. If it doesn't that might be a bug. How does it compare with the default express/ejs behavior?

GothAck commented 12 years ago

Sorry been out of the node loop for a month.

Right, render('/rootthing') currently seems to render relative to the filesystem root, so not hugely helpful, think it just needs to look at options.settings.views.

Setting options.layoutFile always references relative to the directory of the current template, even if it starts with .

Maybe both need to reference options.settings.views if it exists, otherwise use relative to current template?

RandomEtc commented 12 years ago

I think we might disagree about the behavior of layout.

Although it is possible to use this library to port Express 2 EJS apps over to Express 3 (just changing layout to layoutFile should do it for most people), the real purpose of this library for me is exposing the layout function inside templates. For those I'm 100% sure that the layout template path should be relative to the calling template path.

That said I think it should be possible to change the behavior of layoutFile to satisfy us both. If layoutFile is passed in with the options to a res.render call in your app it should do whatever layout did in Express 2. It should be possible to do this without changing the behavior of the layout function in templates. We should add tests for both behaviors.

RandomEtc commented 12 years ago

I just saw that this is a pull request with code attached - thanks for that. I'll take a look and see if I can get us to the situation I just described.

I'd also like to use the app settings to get the default template extension and stop hard-coding .ejs everywhere. I'll see if that's possible too.

I've been considering an API breaking change: rename the layout function to extends and rename the _layoutFile option to layout for full backwards compatibility with Express 2.x. Thoughts?

GothAck commented 12 years ago

Don't think I explained myself as well as I could, having layout() is fine, and using _layoutFile is also great with me, just wanted to fix the absolute lookups of templates according to the settings.views path.

I hope that the pull request provided explains better :)

Do we need backwards compatibility with Express 2.x? If we do, maybe inherits('layout') is more descriptive than extends('layout') as we're inheriting the wrapped html (similar to Mako layouts in Python)?

jpravetz commented 12 years ago

I'm using this fix and it is working for me. The issue is that the default layout.ejs file needs to be in a fixed location, and not relative to the ejs being laid out. So specifying « app.locals._layoutFile = '/layout.ejs'; » works. Adding an example of layout and partials would be helpful for the next person.

jpravetz commented 12 years ago

And if adding examples, make sure that you mention that the path for partials is relative to the root file, not the file that is including the partial. So if you have views/product/index.js using views/product/partials/p1 then call partial('partials/p1') and if p1 includes views/product/partials/p2 then the call should be partial('partials/p2') [not partial('p2')]. Or you can specify partials relative to the root by doing partial('/product/partials/p1')

GothAck commented 12 years ago

Partials are still relatively unless the path begins with / so not sure if the docs need to change dramatically... On Aug 17, 2012 7:49 PM, "jpravetz" notifications@github.com wrote:

And if adding examples, make sure that you mention that the path for partials is relative to the root file, not the file that is including the partial. So if you have views/product/index.js using views/product/partials/p1 then call partial('partials/p1') and if p1 includes views/product/partials/p2 then the call should be partial('partials/p2') [not partial('p2')]. Or you can specify partials relative to the root by doing partial('/product/partials/p1')

— Reply to this email directly or view it on GitHubhttps://github.com/RandomEtc/ejs-locals/pull/4#issuecomment-7831541.

RandomEtc commented 12 years ago

Sorry I've been slow to merge/resolve this issue. It would help me if you could add tests that fail before the change and pass after, so I can keep things straight. I'll try to resolve the issue myself anyway but if you get there first that would be useful. Thanks!

RandomEtc commented 11 years ago

OK, I just pushed 0.2.0 which should respect absolute layout paths (and the default is /layout.ejs) - if the path is absolute it is resolved relative to options.settings.views. I also use options.settings['view engine'] where possible.

Please give this a try. If it doesn't fix your issue then please add some failing tests that help explain it for me!

Thanks for all your input and sorry for the delay!