Skip to content
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

Includes can't extend blocks #1267

Closed
buschtoens opened this issue Oct 23, 2013 · 29 comments
Closed

Includes can't extend blocks #1267

buschtoens opened this issue Oct 23, 2013 · 29 comments

Comments

@buschtoens
Copy link

Included files that contain block extensions (like append body) won't extend the previously declared blocks. Instead of that, they yield in place.

//- boilerplate.jade
!!! 5
html
  head
    block boilerplate-head
  body.root
    block boilerplate-body

include navbar

//- navbar.jade
// extends boilerplate
// This somewhere would causes an infinite loop

append boilerplate-body
  nav.navbar Test

append boilerplate-head
  link(rel="stylesheet", href="/nav.css")
<html>
  <head></head>
  <body></body>
</html>
<nav class="navbar">Test</nav>
<link rel="stylesheet" href="/nav.css">

The included file appears to get its own context and therefore its own scope of block names. It then can't find the boilerplate-head / boilerplate-body and assumes that they don't exist and so creates them in place, as if this was a new block definition.

I consider this encapsulation to be unintended and can't really see a possible use case for it (open for suggestions). But what I can tell is, that it took me about 1 hour to really understand what's going on (this is my 4th attempt to correctly file this issue).

What I would expect is, that includees live in the same scope as their includers or at least somewhat bubble up to it, so includer blocks and mixins, etc. are accesible inside the includees. This would allow for a conveinient way of organizing the components of a layout.

Ex: The navbar can resiude in an extra file and will extend all appropriate blocks with stylesheets, scripts, templates, etc.

@buschtoens
Copy link
Author

If I understood the code correctly (only had a quick scan), these lines should fix the issue, although they obviously don't.

@buschtoens
Copy link
Author

I simplified the case and what's happening now is just... odd.

//- block.jade
block test
  p block.jade

include include.jade

//- include.jade
append test
  p include.jade
<p>block.jade</p><p>block.jade</p>

...wat?

@buschtoens
Copy link
Author

I don't even... after re-uploading the app three times and reinstalling jade, it just worked. I... I can't understand this.
However... The case of my previous comment still stands.

@matejkramny
Copy link

Well perhaps it doesn’t work because the title is not indented as it should be within the append head.

Is it intentional that nested block (like example you posted) don’t work?

Matej Kramny
Sent with Sparrow (http://www.sparrowmailapp.com/?sig)

On Wednesday, 23 October 2013 at 03:57, Jan Buschtöns wrote:

Nested blocks don't work. See this example.
html head block head body block body p Hello World! append body p I belong to the . append head title And I want to go to the .

Hello World!

I belong to the <body>.

<title>And I want to go to the <head>.</title>

Nested block statements (append, prepend and block) are ignored and the block content is yielded in place.


Reply to this email directly or view it on GitHub (#1267).

@ForbesLindesay
Copy link
Member

It sounds like you were using an out of date version of jade.

@buschtoens
Copy link
Author

@ForbesLindesay thougt so, too... but after a night of sleep, things look different.

It didn't work; I was just too sleepy to notice it. Sorry for all the noise, but now I got a test case that fails (for realz).

//- layout-1.jade

#layout-1-1
  block layout-1-1
    p Content from layout-1, inside block #1
    include include-1

#layout-1-2
  block layout-1-2
    p Content from layout-1, inside block #2
//- layout-2.jade

extends layout-1

append layout-1-1
  p Content from layout-2, appends block #1
  include include-2

append layout-1-2
  p Content from layout-2, appends block #2
//- include-1.jade

p Content from include-2, at root level

append layout-1-2
  p Content from include-2, appends block #2
//- include-2.jade

p Content from include-2, at root level

append layout-1-2
  p Content from include-2, appends block #2

layout-2.jade, which is the file to be rendered, extends layout-1.jade.
layout-1.jade includes include-1.jade. layout-2.jade includes include-2.jade.
The includes are included inside the layout-1-1 block.
I would expect the root level nodes of the includes to be included inside the layout-1-1 block.
Further append layout-1-2 statements inside the includes should work and the nodes should be appended to layout-1-2.

Therefore, I would expect the following output.

<div id="layout-1-1">
  <p>Content from layout-1, inside block #1</p>
  <p>Content from include-1, at root level</p>

  <p>Content from layout-2, appends block #1</p>
  <p>Content from include-2, at root level</p>
</div>
<div id="layout-1-2">
  <p>Content from layout-1, inside block #2</p>
  <p>Content from include-1, appends block #2</p>

  <p>Content from layout-2, appends block #2</p>
  <p>Content from include-2, appends block #2</p>
</div>

Actual output is.

<div id="layout-1-1">
  <p>Content from layout-1, inside block #1</p>
  <p>Content from include-1, at root level</p>

  <p>Content from include-1, appends block #2</p>

  <p>Content from layout-2, appends block #2</p>
  <p>Content from layout-2, appends block #1</p>

  <p>Content from include-2, at root level</p>
  <p>Content from include-2, appends block #2</p>
</div>
<div id="layout-1-2">
  <p>Content from layout-1, inside block #2</p>
  <p>Content from include-1, appends block #2</p>
  <p>Content from layout-2, appends block #2</p>
</div>

(new lines added to improve readability a bit)

@buschtoens
Copy link
Author

A further (even simpler) case of include madness.

//- layout.jade

#layout-1
  block layout-1
    p Content from layout, inside block #1

#layout-2
  block layout-2
    p Content from layout, inside block #2

include include
//- include.jade
append layout-2
  p Content from include, appends block #2

Expected:

<div id="layout-1">
  <p>Content from layout, inside block #1</p>
</div>
<div id="layout-2">
  <p>Content from layout, inside block #2</p>
  <p>Content from include, appends block #2</p>
</div>

Gotten:

<div id="layout-1">
  <p>Content from layout, inside block #1</p>
</div>
<div id="layout-2">
  <p>Content from layout, inside block #2</p>
</div>
<p>Content from layout, inside block #2</p>

It seems like the block layout-2 statement in layout.jade replaces append layout-2 in include.jade, which is interpreted as a new block definition and not as an append to the previously created block.

This partially explains the weird behaviour, reported before.

@BaamHDD
Copy link

BaamHDD commented Oct 23, 2013

I ran into the exact same issue yesterday.

@buschtoens
Copy link
Author

@ForbesLindesay I don't consider this issue closed. This definitely is an issue.

@thomas-riccardi
Copy link

@silvinci this example has the same issue without include: not only the append yields even if we would not expect it to, but the append operation is ignored (probably it happens too late...)

@thomas-riccardi
Copy link

In any way, the include should be a dumb "copy the file here" like in the C preprocessor.
This should fix many issues, including my issue 2 of #1261.

@buschtoens
Copy link
Author

@triccardi-systran 👍

A preprocessor would be a solution to many problems here.

@pvoisin
Copy link

pvoisin commented Mar 17, 2014

After reading all that and having stumbled upon similar issues I'm still wondering if include is now supposed to be able to append stuff into blocks? That definitely is something to have since it allows great layouts/components composition (unless I'm blind).

@TimothyGu
Copy link
Member

@buschtoens Not sure if you noticed or not, but this is definitely not a problem in include.

//- layout.jade

#layout-1
  block layout-1
    p Content from layout, inside block #1

#layout-2
  block layout-2
    p Content from layout, inside block #2

append layout-2
  p Content from include, appends block #2

returns the exact same thing as the one with include.


Also, the append and prepend commands are all Inheritance commands and is therefore not applicable to include. I believe include does act as a dumb C #include (with proper indentation of course).


In one of your examples:

//- boilerplate.jade
!!! 5
html
  head
    block boilerplate-head
  body.root
    block boilerplate-body

include navbar
//- navbar.jade
// extends boilerplate
// This somewhere would causes an infinite loop

append boilerplate-body
  nav.navbar Test

append boilerplate-head
  link(rel="stylesheet", href="/nav.css")

When used as-is the boilerplate would be "preprocessed" to this:

//- boilerplate.jade
!!! 5
html
  head
    block boilerplate-head
  body.root
    block boilerplate-body

append boilerplate-body
  nav.navbar Test

append boilerplate-head
  link(rel="stylesheet", href="/nav.css")

It does not make any sense to append something in the file. So of course append doesn't work.


The extends would of course cause an infinite loop as you are making boilerplate include navbar which extends boilerplate.

Assuming you fixed that (and the deprecated !!!):

//- boilerplate.jade
doctype html
html
  head
    block boilerplate-head
  body.root
    block boilerplate-body
//- navbar.jade
extends boilerplate

append boilerplate-body
  nav.navbar Test

append boilerplate-head
  link(rel="stylesheet", href="/nav.css")

When you compile the navbar it would show everything correctly.


If you would really want boilerplate to be the includer/child, then this would work:

//- boilerplate.jade
doctype html
html
  head
    include navbar-head
    block boilerplate-head
  body.root
    include navbar-body
    block boilerplate-body
//- navbar-head.jade

link(rel="stylesheet", href="/nav.css")
//- navbar-body.jade

nav.navbar Test

Unless I'm missing something, I will not reopen this issue.

@buschtoens
Copy link
Author

Doesn't concern me anymore anyways. I went over to the ember.js camp. But thanks for revisiting this nonetheless.

If someone else runs into this, they'll at least know what's up. 😊

@TimothyGu
Copy link
Member

@buschtoens Handlebars, ew

@buschtoens
Copy link
Author

@TimothyGu haha, I feel you. But I'm using emblem, which is basically jade for handlebars. :)

@TimothyGu
Copy link
Member

@buschtoens Cool I'll check it out. Thanks! :)

@ryardley
Copy link

@ForbesLindesay @TimothyGu Not being able to have an include alter code in the layout makes me want to try to move on to different engines.

The issue here is that to build an app you need to group like things together so they can be thought of as a unit where in a webpage things that are related often have separate components that need to appear at different spots within the markup. So it is important to group a particular functional component in a single include location and have it put the code that component needs in the places that component needs them.

Therefore the idea of getting include files being able to override blocks higher up in the hierarchy is an important one as the buckets a component cares about can be setup in the layout as 'general' locations which limits dependencies and keeps everything very composable.

The alternative is to split up everything in multiple spots like you have suggested which makes reasoning about what is going on more difficult and makes changes more cumbersome.


Maybe I am missing something about Jade/Pug or there is a roadmap feature I don't know about but currently the only solution for single point inclusion I can see is to have templates extend widgets that extend widgets that extend layouts which is crazy thing as you then have an insane inheritance chain to manage:

post.jade --|> comments_widget.jade --|> news_widget.jade --|> layout_left.jade

Here express renders the post.jade template as the model is a post let's say. If I want to remove the news_widget from the post template I need to go to the comments_widget which is madness.

If all I have to do is remove the line in post.jade that includes the news_widget and then it wont be there things are starting to make much more sense.

@TimothyGu
Copy link
Member

If the layout for comments_widget.jade is not static then you should consider moving away from includes but rather use mixins for such widgets:

//- post.jade
doctype

include comments_widget.jade

html
  body
    p Lorem
    +comments_widget({ news: false })
//- comments_widget.jade
include news_widget.jade

mixin comments_widget(options)
  ul.comments
    li Great post!
  if options.news !== false
    +news_widget

@ryardley
Copy link

Ok I see what you mean. So suggested approach is to use nested mixins and specify the configuration through vars and then use logic to conditionally manage dynamic parts. This approach still causes maintenance problems however as a result of all the logic within the mixin when compared to simply yanking out an include line. Removing a dead feature still requires sifting through lots of conditional logic.

Having includes processed before inheritance is intuitive for folks coming from haml/rails and I think this should be on the roadmap.

@TimothyGu
Copy link
Member

Having includes processed before inheritance is intuitive for folks coming from haml/rails and I think this should be on the roadmap.

Technically it is this way, even currently.

Can you make a Git repo (or Gist) of your preferred layout? It might help my understanding to see real code.

@ryardley
Copy link

Looking at the code it seems like includes (https://github.com/pugjs/pug-linker/blob/master/index.js#L21) is actually occurring after inheritance (https://github.com/pugjs/pug-linker/blob/master/index.js#L17) Perhaps there is a reason for this?

I will make a repo to show you what I mean.

@TimothyGu
Copy link
Member

L. 17 is only looking for the Extends node, not actually completing the inheritance. After inclusions are applied is the inheritance applied.

I'm actually not sure of the rationale for this at this moment, but I believe it's to prevent some corner cases. @ForbesLindesay, any ideas?

IMO pug-linker is the hairiest code in the entire Pug code base so it's totally normal to not understand it…

@ryardley
Copy link

@ForbesLindesay
Copy link
Member

pug-linker is the way it is in order to be as compatible as I can make it with the quirks of the 1.x.x jade versions while also being (hopefully) marginally easier to understand/follow. Once 2.0.0 is stabilised, I'm hoping we can think about a complete re-write for the linker as part of the 3.0.0 migration with some breaking changes. pug-linker is mainly their to codify the current behaviour.

@lopugit
Copy link

lopugit commented Aug 22, 2017

Has there been any news on this?

I desperately, desperately, desperately need this functionality, I can't see how this use case wasn't foreseen during production of jade, why wouldn't somebody using some variable scope in an include not want to maybe append some html data to another block in the layout...?

@lopugit
Copy link

lopugit commented Aug 22, 2017

What I meant to comment was has their been any development or solution for this?

Write now I'm working on a way to duplicate my template logic to be able to append some html right below the body, because I just need to create modals while I create other template compiled html, but modals, or bootstrap modals specifically, need to be children of the body

@mralexgray
Copy link

the fact that this was closed makes me very suspicious that the maintainer doesn't understand why someone might want to USE pug.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

10 participants