Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Allow multiple blocks to be defined inside mixins and includes #631

Closed
jasonkuhrt opened this Issue May 7, 2012 · 48 comments

Comments

Projects
None yet

I would like to be able to write my markup like thus:

//panel.jade

section.panel
  block head
    header
      h1.title!= title
  .content
    yield

invocation:

//some_file.jade
    include panels/panel
      block head
        //nothing
      img( src="#{work.urls.main}/poster_280.jpg" )

Update based on feedback the new proposal would be:

//panel.jade
mixin panel
  section.panel
    block head
      header
        h1.title Default
    .content
      yield

invocation:

//some_file.jade
    + panels/panel
      disable head
      img( src="#{work.urls.main}/poster_280.jpg" )
Contributor

chowey commented May 7, 2012

This sort of makes extend and include behave the same. Is that a good direction to take?

Hey @chowey

  • panel is a template which I use to provide structure. It would be included a dozen or so times on the page. @extends is a once-per-file feature and therefore bears no comparison as far as I can see.
  • Jade's templating feature-set is too complex for too little gain, IMO. Consider something like Haskell's heist which provides one clear and powerful concept for the user. Jade doesn't need a mish-mash of mixins and includes and extends when an elegantly simpler and more powerful combination of the 3 would not just suffice but actually enhance the ergonomics of the API.
Contributor

chowey commented May 7, 2012

Yeah actually mixins kind of work like extends right now, except they take arguments, are defined in the same file, and don't have the "append" or "prepend" features.

We could make mixins do append and prepend (actually a more consistent API, don't know why I didn't think of that earlier).

I think if we go that route, then include should just include the file, extends is redundant because include/mixins could do the work that extends did before.

So basically I propose that you don't do

include panels/panel
  block head
    //nothing
  img( src="#{work.urls.main}/poster_280.jpg" )

but instead something like

include panels/panel

+panel
  block head
    //nothing
  img( src="#{work.urls.main}/poster_280.jpg" )

This would make panel defined as a mixin instead. Then, in mixins, we would support yield for non-block statements, and blocks would be supported as they are in extends.

It is not backwards compatible in mixins, but that may be ok because the new mixin block features are not documented yet. I think the existing include/extends behavior would need to be left as-is for backwards compatibility.

Contributor

chowey commented May 7, 2012

(For the above, you would need to define mixin panel in the "panels/panel.jade" file.)

donpark commented May 7, 2012

I think overall architecture needs reviewing, particularly with
seamless WebComponents support in mind, instead of just
looking at include, extends, block, and mixin.

Also we need to keep in mind that there are two different
units of work here: component/function and module/file.

Contributor

chowey commented May 7, 2012

The purpose of extends is to provide a layout. It does that nicely. It is not meant to build widgets.

The purpose of include is to provide drop-in-place markup. You can do some fancy things with includes by using loops and including multiple times, changing variables with each loop. But it is pretty much limited to that.

The purpose of mixin is to give a shortcut for typing in markup that you will be re-using. It takes arguments. You can pass blocks to it (a new feature). It sort of works to build widgets but you don't get multiple blocks.

None of these is fully a "template".

I maintain that the best candidate for templating is mixins. I say this because the purpose of mixins is closest to that of templates. So we would be fixing up mixins to better achieve their design intent, not changing the design intent entirely.

We would need to improve mixins like so:

  • We should use yield in a mixin to indicate where to render the block. This replaces the block keyword that we are currently using.
  • We should use block like it is used in a layout (i.e. it has a name and can be prepended, appended or replaced).
  • Calling mixins could look like this:
mixin panel
  .panel
    .header
      block header
        h1 Default title
    yield
    if block.footer
      .footer
        block footer

+panel
  block header
    h1 My important announcement
  block footer
    a(href='next.html') Next
  p This is my main content.
  p And this is another important point.

which would render:

<div class="panel">
  <div class="header">
    <h1>My important announcement</h1>
  </div>
  <p>This is my main content.</p>
  <p>And this is another important point.</p>
  <div class="footer"><a href="next.html">Next</a></div>
</div>

If you want to put your mixin definitions in an external file (e.g. you plan to re-use them), then you would include that file.

Is that more or less what we're asking for?

It is also totally possible to make include work like you say. However, I think mixins were intended to be the templating feature.

I could work on implementing this but I won't start unless @visionmedia says ok. It is a fair bit of work to do if it won't be approved anyway.

@chowey I think you're interpretation would certainly be a big improvement.

  • Enhance mixins
  • Tweak include
  • Deprecate extends

If we create a way for mixins handle files like include then we can also deprecate include.

Playing around:

mixin panel
  .panel
    .header
      block header
        h1 Default title
    yield
    if block.footer
      .footer
        block footer
+ panels/panel
+ panel
  block header
    h1 My important announcement
  block footer
    a(href='next.html') Next
  p This is my main content.
  p And this is another important point.

I also then see an opportunity for direct mixin access without needing to include, like this:

+ panels/panel
  block header
    h1 My important announcement
  block footer
    a(href='next.html') Next
  p This is my main content.
  p And this is another important point.

To encourage code-sharing / mixing-libraries we could facilitate namespaces like:

soundtrack-panel = +panels/soundtrack
+ soundtrack-panel
  block header
    h1 My important announcement
  block footer
    a(href='next.html') Next
  p This is my main content.
  p And this is another important point.

or hash-access like:

+ panels
// or panels = +panels
+ panels.big
  block header
    h1 My important announcement
  block footer
    a(href='next.html') Next
  p This is my main content.
  p And this is another important point.

We could say that a mixin without params or nested content works like an include. That seems intuitive?

@donpark Details would be great. Sounds interesting...

Contributor

chowey commented May 7, 2012

I am against making mixins do work with files. I think importing a file and defining a mixin should be logically separate.

You can do

include lib/mytemplates
+panel
  // ...

instead of

+lib/mytemplates/panel
  // ...

and I think keeping it separate is logically better. It is also less messy on the implementation side.

The + syntax for calling a mixin has been around since 0.24 or something (sort of in its alpha stage).

@chowey I might have a different perspective but I see yours very clearly and agree it makes sense. The existence of include and mixin is not a problem and as you say there is a perfectly reasonable case why its more logical this way.

Contributor

tj commented May 8, 2012

things have definitely evolved a bit but I still think they're pretty distinct sets of functionality other than maybe the new include block stuff. include could potentially replace extends if we had multiple block support

include layout
  append head
    script(src='jquery.js')

  block content
    h2 stuff here

or something, not in love with it but I definitely prefer less concepts as long as they work nicely. Personally I dont think building widgets at the template engine level is all that great, a lot of that can be done in client-side js and works better IMO, at least for my use-cases, I pretty much never build widgets with Jade that inherit from each other etc, and only mixins for really repetitive things that can't really be expressed clearly without a bit of abstraction

@visionmedia at-scale I see your point about js widgets but for a lot of brochure sites, micro blogs, or portfolio sites I think Jade should still offer a great code-reuse experience. And its not as if we have to sacrifice simplicity to achieve it. Merely adding multiple block support to mixins will get us a lot of the way there.

Contributor

tj commented May 8, 2012

definitely

donpark commented May 8, 2012

If deprecating extends and adding general block support is what the consensus is, I like it.

Can we make a decision so that @chowey might spike an implementation? It seems like we have a pretty general agreement about how to improve the system.

Contributor

tj commented May 10, 2012

personally I have no issue with what we currently have but I'm open to the suggestions if it works out fine

There's one nuance not covered yet that replacing extends with include introduces:

With extends:

extends foo

block a
  ...

block b
  ...

content...

Now with include:

include foo
  block a
    ...
  block b
    ...
  content...


// or

include foo
  content...

// access block 'a' defined within foo without nesting?
block a
  ...

// or namespace access block 'a' defined within foo without nesting?
block foo.a
  ...

// so that if we have another include
include bar

we can extend another block of the same name ('a') defined inside bar, again without nesting

block bar.a
  ...

Its more complicated given that we can have many includes whereas extend was simple because we were limited to one.

Contributor

tj commented May 11, 2012

yeah, that's why im not personally too worried on changing this immediately until we have a good solution

@visionmedia If we reduced the commit to just improving mixin block abilities, I think that would be a good safe bet without risking future backtracking.

donpark commented May 11, 2012

Just brainstorming. What if extends is re-purposed as a way to redefine/override include operations like this?

extends lib/foo
  block a
    ...

extends lib/bar
  block b
    ...

.container
  .foo
    include lib/foo
  .bar
    include lib/bar

@chowey chowey referenced this issue May 19, 2012

Closed

Include blocks #645

Contributor

chowey commented May 19, 2012

I made a patch that makes include work with blocks. It turns out this is almost trivial to implement.

On the other hand, making mixins work like I said is horrible.

So include support is what we get.

@chowey Thanks man! It's a nice addition at least.

donpark commented May 19, 2012

thanks @chowey. this will come in handy.

holic commented Oct 5, 2012

Any updates on this?

darky commented Nov 14, 2012

Yeah, it's not release...

I bumped into this problem today to my surprise. I had hoped you could write widgets, jade files that includes all the CSS/JS and HTML for it to work. The snag was that you can't use blocks in included files, as discussed above.

Layout.jade

!!! 5
html
  head
    block styles
      link(rel="stylesheet", href="style.css")
    block scripts
      script(src="jquery.js")
  body
    block content

Widget.jade

append styles
  link(rel="stylesheet", href="widget.css")
append scripts
  script(src="widget.js")

p Some fancy widget
canvas ....

These are tied by another jade file, like Index.jade

block content
  header
    ...
  #content
  #widgets
    include widget

But instead of appending the link and script tags in the correct place they are at best dumped straight into the body if you add a block content somewhere.

This is how I imagined how extend and include where suposed to work. I hope this pr doesn't get lost and that @visionmedia tries to take all these feedback into consideration, which from what I get see here doesn't feel like it.

Contributor

tj commented Jan 9, 2013

https://github.com/component/component is a lot better for that, it's a bit of a hacky thing to attempt on the server end IMO, but i can see how it's desirable for people who have to use server rendering.

Igor10k commented Nov 12, 2013

Twig implements something similar to what is being described here as an embed

I'm personally quite surprised that this feature is not often requested. Creating reusable pieces of code is always great and this feature really helps. I see people ending up with copy-pasting the same html pieces over and over and using includes.

And using just one block within a mixin is kind of not an option because you can't be sure if tomorrow you won't need another block there. And if you will you'll have to reimagine the mixin completely.

I would second all the comments here about this being a great boon for code re-use. I'm currently using a 'component factory' approach to many projects, and this would make things a lot easier (this, or solving #374).

Owner

ForbesLindesay commented Feb 4, 2014

This has seen relatively little activity for a long time. If someone wants do implement it (along with tests) I'm happy to consider it, but I'm not going to implement it myself.

This would be extremely useful in my current project. This is an old issue so things might have changed by now, but I can't find any doc on it. So my question is: is there now a way to achieve using multiple named blocks in a mixin?

@visionmedia

Igor10k commented Jun 1, 2014

Nothing was changed regarding to this case. But there's a hacky way to implement something similar though by splitting mixin code into few nested mixins, like

mixin main
  mixin header
    // some code that should always be there
  mixin footer
    // some more code
  block

Then you'll be able to insert code between those nested mixins.

+main
  p Welcome to my website
  +header
  p Some great content goes here
  +footer
  p Copyright 2014

This workaround is not as convenient as named blocks would be but it saved me once. :)

Owner

ForbesLindesay commented Jun 1, 2014

@Igor10k that does work really well. I've actually been using similar techniques in some of my projects for a while now. It would be really great to add some jade test cases that demonstrate the feature, to ensure we don't accidentally break that use case in the future.

Thanks @Igor10k! I got it to work, but with a slightly different syntax than you provided. My project (which you can see linked above since I accidentally added a reference to this issue) uses Jade version 1.3.1

The syntax I'm using now is as follows:

//- mixin declaration
mixin imageAndCaption()
    div.someClass
        +image()
        div.myCaption
            +caption()

//- use the mixin
+imageAndCaption()
    mixin image
        img(src="/my/image.png")
    mixin caption
        p this is my caption

So I never had to use the block keyword. And the nested mixins are defined by the caller of the enclosing mixin, not defined in the enclosing mixin itself

This syntax makes a lot of sense and I really like it. It also allows variables to be passed around, which makes iterations easy (which is what I actually have in my project)

@Igor10k @bigpopakap that looks like a nice way of doing it - cheers for sharing. @bigpopakap I especially like your take on it. Because I work in a very component-based manner, one thing I would add is namespacing the nested mixins to their parent. So:

+post("Post title")
    mixin post_teaser
        p [...]
    mixin post_body
        p [...]
        p [...]
        p [...]

@ForbesLindesay ForbesLindesay added the mixin label Jun 4, 2014

Owner

ForbesLindesay commented Jun 4, 2014

@bigpopakap That code example doesn't (and shouldn't) work. The issue is that the mixins you are writing inside the caller's block won't ever actually get defined. The JavaScript generated by that example is:

    var buf = [];
    var jade_mixins = {};
    jade_mixins["imageAndCaption"] = function() {
        var block = this && this.block, attributes = this && this.attributes || {};
        buf.push('<div class="someClass">');
        jade_mixins["image"]();
        buf.push('<div class="myCaption">');
        jade_mixins["caption"]();
        buf.push("</div></div>");
    };
    jade_mixins["imageAndCaption"].call({
        block: function() {
            jade_mixins["image"] = function() {
                var block = this && this.block, attributes = this && this.attributes || {};
                buf.push('<img src="/my/image.png"/>');
            };
            jade_mixins["caption"] = function() {
                var block = this && this.block, attributes = this && this.attributes || {};
                buf.push("<p>this is my caption</p>");
            };
        }
    });
    return buf.join("");

As you can see, the calls to jade_mixins["image"] = and jade_mixins["caption"] = never actually get called since they are inside the block function. To fix this, we have to actually add the block keyword at the start of the imageAndCaption mixin. It is there just to cause the inner mixins to get defined:

//- mixin declaration
mixin imageAndCaption()
    block
    div.someClass
        +image()
        div.myCaption
            +caption()

//- use the mixin
+imageAndCaption()
    mixin image
        img(src="/my/image.png")
    mixin caption
        p this is my caption

which results in JavaScript like:

    var buf = [];
    var jade_mixins = {};
    jade_mixins["imageAndCaption"] = function() {
        var block = this && this.block, attributes = this && this.attributes || {};
        block && block();
        buf.push('<div class="someClass">');
        jade_mixins["image"]();
        buf.push('<div class="myCaption">');
        jade_mixins["caption"]();
        buf.push("</div></div>");
    };
    jade_mixins["imageAndCaption"].call({
        block: function() {
            jade_mixins["image"] = function() {
                var block = this && this.block, attributes = this && this.attributes || {};
                buf.push('<img src="/my/image.png"/>');
            };
            jade_mixins["caption"] = function() {
                var block = this && this.block, attributes = this && this.attributes || {};
                buf.push("<p>this is my caption</p>");
            };
        }
    });
    return buf.join("");

In turn resulting in the html:

<div class="someClass">
  <img src="/my/image.png"/>
  <div class="myCaption">
    <p>this is my caption</p>
  </div>
</div>

You also need to be careful to always define both inner mixins or the previous definition may get used. An alternative would be to define defaults like:

//- mixin declaration
mixin imageAndCaption()
    mixin image
      //- this will get overridden
      img(src="/my/placeholder.png")
    mixin caption
      //- this will get overridden
      p Un-captioned image
    block
    div.someClass
        +image()
        div.myCaption
            +caption()

//- use the mixin
+imageAndCaption()
    mixin image
        img(src="/my/image.png")
    mixin caption
        p this is my caption

Hmm I'm not sure what's happening then, because I have it working. Could it be that in my project, mixin image and mixin caption are actually being defined globally?

If you have the time, my relevant change is linked below. In that code, carousel is the equivalent of imageAndCaption from my example above, carouselImage is the equivalent of image, and carouselCaption is the equivalent of caption

bigpopakap/kapileaswar@ccec5d7

@ForbesLindesay Attempting your alternative to define defaults is getting me "Anonymous blocks are not allowed unless they are part of a mixin." I tested you code as well as the hacks I made to mine. =)

Would really like to be able to setup the defaults, as 90% of the calls would use it...

I'm on jade 1.3.1

Owner

ForbesLindesay commented Jun 10, 2014

Sorry, that's a bug. I've opened an issue to get it fixed.

@thomporter
this worked for me:

file mixins/box.jade:
mixin box   
    //- MIXIN BLOCK
    block

    //- WORKAROUND FOR DEFAULTS
    mixin mixin(mixin,mixinDefault)
        - try {
            if mixin
                +#{mixin}
            else
                - throw new Error();
        - } catch(err) {
            if mixinDefault
                +#{mixinDefault}
        - }

    //- DEFAULT COMPONENTS
    mixin headerDefault
        p
            | i am a default header
    mixin contentDefault
        p
            | i am default content
    mixin footerDefault
        p
            | i am a default footer 

    //- BOX LAYOUT
    div&attributes(attributes)
        +mixin('header','headerDefault')
        +mixin('content','contentDefault')
        +mixin('footer','footerDefault')
file layout.jade:
//- [...]
include mixins/box
+box
    mixin header
        p
            | i am a overridden header

klarezz commented Nov 11, 2014

another suggestion for named blocks:

// initialization
- var globals={};

mixin set(key)
    - globals[key]=this.block

mixin A
    block
    - globals.content()

+A
    +set('content')
        p A content

is there a way to pass a jade syntax to javascript without using a mixin.block?

Owner

TimothyGu commented Jan 20, 2015

Condensed version of @anticamper's workaround:

mixin myMixinWithTwoBlocks(uniqueID)
  block

  mixin contentDefault
    p No content provided

  .author
    +author
  .body
    +content
+myMixinWithTwoBlocks
  mixin author
    a(href="https://my.website") Timothy
  mixin content
    p Hola señor. ¿Cómo está?
    p Bien, ¡gracias!

+myMixinWithTwoBlocks
  mixin author
    | Juan
  mixin content
    +contentDefault

The default trick doesn't work, as the lifetime of the first content mixin lasts throughout the file. As a result, you have to always manually specify content to be contentDefault.

Output (manually beautified):

<div class="author">
  <a href="https://my.website">Timothy</a>
</div>
<div class="body">
  <p>Hola señor. ¿Cómo está?</p>
  <p>Bien, ¡gracias!</p>
</div>
<div class="author">
  Juan
</div>
<div class="body">
  <p>No content provided</p>
</div>

Thanks @anticamper & @TimothyGu - pure awesomeness.
I just built a tabs-element mixin with this, works like a charm! <3

Thanks @anticamper, @TimothyGu , and @CREEATION
Nested mixins are good solution, but what if I what repeat a block code or change the block orders within my mixin?

mixin table
    block tableheader
    block customrow
    - var row = 0
    while row<10
        - var col = 0
        tr
            while col<10
                td cell
                col++
        row++
    block customrow
    block tablefooter
+table
    block tableheader
        //- table header here
    block tablefooter
        //- table footer here
    block customrow
        //- custom row here

Found this solution without having to use nested mixins
https://codepen.io/nekitk/pen/bdMZgb?editors=1010

Ad-Ok commented Oct 24, 2017

@nataliawojtkowska that's great, but what if I need conditions for displaying inner content? For example, show blocks.main() only?

nataliawojtkowska commented Oct 25, 2017

@Ad-Ok
Then you can add an if statement like

if blocks.side
    - blocks.side()

https://codepen.io/14islands/pen/QqXYrb?editors=1010

If that's what you mean 😊

EDIT: @Ad-Ok I forgot to clear the blocks array, otherwise you'd still get the blocks.side from setting it previously. Take a look at the updated pen

Ad-Ok commented Oct 26, 2017

@nataliawojtkowska Thank you very much! It's exactly what I wanted.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment