-
Notifications
You must be signed in to change notification settings - Fork 513
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
Adds support of layout and blocks to EJS #147
Conversation
- Imports layout files specified with <% layout ... %> - Parses blocks identified with <% block ... %> - Exposes blocks to layout files with local `block` variables - Supports multi-layered layouts; child blocks override parent blocks Adds unit tests for layouts Adds layouts to README
<% layout layout-page -%> | ||
<% block title -%> | ||
Page Title | ||
<% block page -%> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
personally i find the implicit ending a little confusing, I'd vote for <% block title %>
to act like an empty placeholder, and then <% endblock %>
(or whatever it's called) to specify blocks that do have default content, otherwise it reads a little unclear to me at least
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<% extend
is a bit more traditional than <% layout
as well but other than that everything else looks good!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fair enough. I can change up the name.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just to clarify, are you thinking something like:
<% extend layout-file %>
...
<% block content %>
...
<% endblock %>
And any content not in between a block/endblock section will get added to the default block?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yup i think that sounds about right, more like jinja stuff that most people are used to
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I just realized I misinterpreted your first comment. Let me backup and explain.
What I'm addressing is, when a template is extending a layout file, what to do with any content that falls outside of block/endblock pairs:
<% extend layout-file %>
What to do with me?
<% block title %>...<% endblock %>
Me too?
<% block content %>...<% endblock %>
There are three way to deal with this content: 1) throw an exception, 2) throw it away, 3) collect the content together into a default
block. With option 3, the above template would essentially be the equivalent of:
<% extend layout-file %>
<% block title %>...<% endblock %>
<% block content %>...<% endblock %>
<% block default %>
What to do with me?
Me too?
<% endblock %>
I misunderstood you earlier comment and thought you were describing option 3, so jumped ahead without explaining myself. Sorry about that. Hope this clears it up a bit.
Edit: I should mention that my current implementation uses option 3. It's easy enough to switch to option 2. Option 1 is a little trickier, as it should consume whitespace without exception, but any of the options should be any easy change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now to address what you were actually talking about earlier. I have a couple concerns about using a single-line placeholder.
- Performance: In order to determine if the content that follows a single-line block is default content, you will have to scan ahead to the end of the file looking for an endblock statement. This won't be achievable using the current one forward-pass parser that's written. The best way I can think to implement this would be to add a pre-parse step that scans the file and tags any single-line blocks with an <% endblock %>. Then in the second pass, the parser can assume that any content following a <% block %> statement is default content.
- Potential ambiguity: Can default content include a block? In other words, would you allow this:
<% block container %>
<div class="container">
<% block content %>
</div>
<% endblock %>
If so, then the parser is going to have an ambiguity problem. Is the above code a single-line container
block, followed by a content
block with default content, or a container
block with default content containing a single-line content
block? If you don't want to allow this behavior it's not an issue, but I can see it being useful in certain scenarios.
- Local block variable: My reason for opting to use a local variable instead of an embed statement was to allow the ability conditionally include decorative HTML in the layout file. For example:
<% if (blocks.title) { %>
<div class="page-title">
<h1><%- blocks.title %></h1>
</div>
<% } %>
This doesn't mean that you can't also have a command statement to include a block, but <%- blocks.title %> seems just as concise as <% block title %>, and precludes the other issues.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah we'd have to use a different syntax for placeholders with no default content. I'm not sure I'd do anything with the "default" content, if I was looking at a template and saw something like this:
<% block head %>
some stuff
<% endblock %>
some other content here
<% block content %>
some stuff
<% endblock %>
I wouldn't really expect much out of the content in-between, at least it wouldn't be clear where it's going anyway, especially when it spans multiple blocks that are otherwise unrelated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In my opinion, the content out of blocks should be ignored. Giving the layout and blocks, the child just specify the block it would like to replace, other content just inherit the layout.
If many child tpls have sth that out of block, you could define a block in the layout, then you could explicitly inherit and replace the block in child tpl.
I think maybe this way would be more clear and familiar to developers :) see also #142
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool, thanks for your feedback. I'll discard the external content.
I'm feeling uninspired as to a name to give a single line block besides sblock
, so I'll go with that for now - can always rename. Potentially we could add a pblock
as well to include overridden parent content to address prepend/append feature described in #142.
This commit is the first step to providing a default block to contain layouts. The `Channel` class contains the output buffer. The active channel can them be switched out to provide multiple buffers. Note: Used an object buffer instead of an array based on performance tests. Tests can be viewed at http://jsperf.com/array-vs-object-buffer.
Adds an explicit `endblock` statement to close layout blocks. If a block is not closed before another block is opened or the end of file is reached, an exception will be thrown. Any content not contained inside of a block/endblock section will be added to the `default` block.
Moves template commands into a separate function, cleaning up main parse loop.
I've finally gotten back to this. I've added the ability to define default content for a block. In order to support this I've made some pretty extensive modifications to the code. As a shorthand to embed a block with no default content, you can use
I'm not particularly happy with the label |
What about supporting blocks like twig does. This is quite convenient. |
This version of EJS is no longer maintained. Current work on the EJS module for NPM is happening at https://github.com/mde/ejs. There's some discussion of layouts/blocks happening here: mde/ejs#62 |
thx |
I've added supoort for layouts to EJS. The key features are:
block
variablesI saw that @ictliujie also has a pull request for this feature (#142), but since I was also working on my own implementation I thought it should submit it for discussion. The key differences are:
blocks
variable: Layouts can access blocks using a localblocks
variable. This provides the greatest flexiblity for implementation.