Spec Layouts/Includes/Partials

[ForbesLindesay]

35 Is becoming a little unproductive because of all the +1s, if you have an interest and want to watch the discussion then please select watching thread at the bottom. Only comment if you have an opinion about what the API should be, or how it should be implemented.

I want to have a thread to tie down the spec and how we want to implement these features then if @visionmedia agrees with the proposal, I'll write the implementation and submit a pull request.

Syntax

<% extends('layout/path.ejs') %>
<%- include('child-template-path.ejs') %>
<%- include('child-template-path.ejs', {locals:object}) %>

This is easier to parse than a new syntax version, and we can still read these at compile time. We would only allow string literals for path names, and not support the use of copies of extends/include (i.e. <% var e = extends; %><% e('foo.ejs') %> would not be valid and neither would <% extends('foo' + extension) %>)

Functionality

Path resolution

Path names should be relative to the current file path. If no extension is provided, we use the extension of the current file, which we read from options.filename.

It is possible to override the entire resolution of a path to the file contents by providing options.fileLoader. The default implementation of this would look something like:

var path = require('path');
var fs = require('fs');

/**
 * Load the contents of a file
 * 
 * @param {string} source      The source template requesting include/layout
 * @param {string} destination The file path to resolve and load
 * @param {function(exception, string)} callback
 */
function fileLoader(source, destination, callback) {
  fs.readFile(path.resolve(source, destination), 'utf8', callback);
}

We can make this asynchronous because we only call it at compilation time. I think it is worthwhile to support overriding this as it would allow for template rendering on the client and where templates may be stored in a database or somewhere else entirely.

Extends

Extends takes the parent template as an argument, the parent template should get the same locals as the child template. In addition though, it should have access to the content of the client template. There are 2 options for how we do this:

Option 1

Provide the result of rendering the child template as the local contents to the parent template.

Pros:

• Simple to understand
• Good match for express pre v3.
• Simple to implement

Cons:

• Limited: It's difficult to have the child provide more than one block of content (e.g. providing Main content and sidebar navigation content)

  Option 2

Provide a syntax for specifying a block of content, which could then be used in the parent. It could be done such that all content of the child that was not in a block of some sort was then available to the parent template as contents, making this option compatible with Option 1

Proposal

I would propose that we implement Option 1 first, then extend it to support Option 2, maintaining backwards compatibility with the simpler Option 1.

Includes

Includes takes either 1 or 2 arguments. The first argument is the relative path to the child template to include. This is processed by the fileLoader (see Path Resolution above). The second optional argument is the locals to be provided to the include. If the second argument is undefined or not provided, then the locals of the calling template are used instead.

Implementation

TODO

[tj]

we'll have to name include() something different if we want dynamic partial-like things, probably just partial() again. The include support that I added is compile-time

[tj]

As far as layouts go, the easiest / quickest to implement would be to support only one level, and to not go all crazy like Jade

[ForbesLindesay]

My thinking with include was still that it would be at compile time that we loaded the file and included it, only how we call it would be different. I'm not even sure if I like that, it might be better to go for the less dynamic include.

[ForbesLindesay]

As for layouts, I think the inheritance chains are really important. I implemented it in QEJS and I use it all the time. I typically have one base layout that's pretty similar across most projects and adds the html boilerplate and jQuery/bootstrap if I'm using them. Then I have a layout for public pages, and another layout for authenticated pages. If it's a big site with sections, there may also be layouts for the sections.

I think we need to be careful not to suggest that EJS should be less powerful than Jade. It should perhaps be simpler, and make more effort to avoid new syntax, because the premise is built on you just using two languages you already know, rather than learning a new one. There's no point implementing things like mixins in EJS, because you can just use the native language functions, but when something would be really hard to implement without the support of the templating engine I think it's important for us to fill the void.

[ForbesLindesay]

I also think the inheritance chains would be pretty simple to implement because you'd just be calling into the ejs compile function of the parent, which could then do all the hard work for you.

[tj]

I've never used them in Jade beyond a single layout haha, I find it kinda overkill personally but hey

[ForbesLindesay]

I suspect I'm not alone in thinking it's an absolute killer feature. Are you OK with supporting it? We could ask somewhere to try and get a feel for how many people would use it if you prefer?

[tj]

it'll just take a bit longer to implement / test

[ForbesLindesay]

I don't think it'll add much to the complexity tbh. I've implemented it for QEJS and that went fine. The only difference here is that we're doing it at compile time rather than at render time. I think the ability to have blocks in the child template (option 2) would be much more complex. Are we agreed that we go with option 1 and implement option 2 only if people ask for it?

[tj]

it's not really that it's complex, it's just that I have 250+ other projects haha

[ForbesLindesay]

Cool, well I'm happy to make this fairly high on my list of priorities, so if we agree on what needs doing, I'll have something for a fortnight after it's agreed regardless of what strategy we want to go for. I just want to get something agreed because there's no point adding one more to the list of pull requests that sort of implement the feature, but are ill thought out or don't match what you see as the future of the library.

[tj]

I'm +1 for layouts and sync partials that cache. It's easy to over-complicate things as far as crazy inheritance goes etc, personally I usually favour simplicity over mega-DRY solutions

[ForbesLindesay]

Shall we make the file Loader sync then?

var path = require('path');
var fs = require('fs');

/**
 * Load the contents of a file
 * 
 * @param {string} source      The source template requesting include/layout
 * @param {string} destination The file path to resolve and load
 */
function fileLoader(source, destination, callback) {
  return fs.readFileSync(path.resolve(source, destination), 'utf8');
}

[ForbesLindesay]

Would keep things really nice and simple

[ForbesLindesay]

If we go the sync and cache route, should we make them fully dynamic, removing the restriction that they must be string literals?

[tj]

I'd still leave include as compile-time, but partial(name, locals) would be this new thing. We can just take the filename passed to ejs, grab the dirname() from that, join(dir, name) from the partial() call to resolve it, and then cache[path] = cache[path] || read(path) blah blah

[ForbesLindesay]

Yeh, sounds good, do you want to do as proposed and make the functionality pluggable? The main use case I see for doing so would be as a client side component where you could provide the other templates as an object of some sort. We could just hard code it to use fs like you suggest above, that would simplify things and it wouldn't be difficult to switch to plugins later.

[tj]

nah, I'm highly anti-extend on the client-side, I'm not convinced there's any use there, on the server it's reasonable since you're constructing large strings of markup

[ForbesLindesay]

Fair enough, we'll just use the file system

[ForbesLindesay]

New Proposal:

Syntax

<% extends('layout/path.ejs') %>
<%- partial('child-template-path.ejs') %>
<%- partial('child-template-path.ejs', {locals:object}) %>

extends must be passed a string literal, partial works just like any other function.

Functionality

Path resolution

Paths are resolved relative to the filename of the original file. The extension of the source file is used unless one is provided. The same resolution is used for both extends and partials

Extends

Extends takes the parent template as an argument, the parent template should get the same locals as the child template. In addition though, it should have access to the content of the client template as the variable contents. This means that to print the child template, in the layout template, you'd use:

<%- contents %>

Partials

Partials take either 1 or 2 arguments. The first argument is the relative path to the child template to include. This is loaded in synchronously the first time, but cached for future requests. The second optional argument is the locals to be provided to the include. If the second argument is undefined or not provided, then the locals of the calling template are used instead.

[tj]

it may be useful to default partial() to merge the parent's locals (this is what express did in 2x)

[ForbesLindesay]

So are you suggesting that:

File 1

<%- partial('file2.ejs', {bar: 'bar'}) %>

File 2

<%- foo + bar %>

render('file1.ejs', {foo: 'foo'});

would render foobar That would seem fine to me if that is what you are suggesting.

[tj]

yup, keys passed directly to partial() would of course take precedence but otherwise inherit the parent's

[ForbesLindesay]

cool, I think that's all the functionality settled then?

[tj]

yup! for now at least, ideally separate pull-requests so they're not getting mixed up

[ForbesLindesay]

Will do

[TimNZ]

I don't understand why we can't specify an absolute path to a view relative to the 'views' directory specified in express. I'm using ejs-locals but I think that is still an issue with ejs the way I read above.

The below directory hierarchy is impossible to support even though it seems to make sense to me.

views -> home.ejs -> account ------> accounthome.ejse -> inclues ------> header.ejs ------> defaultsidenav.ejs ------> sidelisting.ejs

1. I have <%- include includes/defaultsidenav %> in home.ejs
2. I have <%- include includes/sidelisting.ejs %> in defaultsidenav.ejs
3. I have <%- include ../includes/defaultsidenav %> in account/accounthome.ejs

home.ejs will load fine account/accounthome will fail as includes/defaultsidenav will fail as it can't find includes/sidelisting.

Shouldn't includes/partials be relative to the current view as opposed to the original view? I should just be able to have <%- include ./sidelisting.ejs %> in defaultsidenav.ejs and it is always found regardless of parent.

Hopefully I'm just doing something wrong here as otherwise it looks like I have to store all views in one directory with one includes directory.

Alternative is to support /absolute path relative to engine views directory.

[tj]

yes includes should be relative to the file using the directive, if not then that's a bug

[TimNZ]

Ok thanks, will debug and submit a pull request for correct module, most likely ejs-locals

[Martin-Luther]

Hello V&F,

I have 2 proposals that i can implement right away to solve this issue.

Scenario 1 : using the same include method

in the "layout.html" you will have :

<% include $myPartialVar %>

and you will still be able to use the default way of including :

<% include my-partial.ejs %>

NB.: the "$" sign will tell ejs to eval the local variable "myPartialVar" passed along with the others defined in the controller (I'm using expressJs)

NB.: if the "$" is a problem for you, just tell me which sign you would like put...

OR

Scenario 2

We can go ahead and create a different method handling only the case with the varPartial to render... So, in the "layout.html" you will have :

<% includeSync myPartialVar %>

I have been trying both cases and it worked pretty well me... Allow me to commit this evolution.

[ForbesLindesay]

tbh, we could just make extends work like a normal function by default. Simply use the same method as for partial. i.e. sync read with cache.

Thoughts @visionmedia?

[tj]

possibly, I still think it should be a different construct so you're opting-in, plus a function call of partial(var) disambiguates if it is or is not a variable

[ForbesLindesay]

I'll work on partial as soon as my current PRs are merged (I want to start with working tests :smile:)

[alvarotrigo]

As far as I understood this other repo is the version 2 of this same module, am I wrong?

If so... this feature never got added?

[mde]

You're correct. This repo is here only for historical reasons. It is no longer under development.