marko-js / marko

A declarative, HTML-based language that makes building web apps fun
https://markojs.com/
MIT License
13.4k stars 644 forks source link

w-body ignored for composite tag #132

Closed kristianmandrup closed 9 years ago

kristianmandrup commented 9 years ago

I have an accordion tag like this <div w-body="" class="ui $data.ui accordion"></div>

When I use it like:

    <ui-accordion>
      <acc-block active="true" label="Yeah" text="hello world"></acc-block>
    </ui-accordion>

It compiles to:

function create(__helpers) {
  var str = __helpers.s,
      empty = __helpers.e,
      notEmpty = __helpers.ne,
      escapeXmlAttr = __helpers.xa;

  return function render(data, out) {
    out.w('<div w-body class="ui ' +
      escapeXmlAttr(data.ui) +
      ' accordion"></div>');
  };
}
(module.exports = require("marko").c(__filename)).c(create);

and so, renders:

<div w-body="" class="ui accordion"></div>

I would have thought the compiler would use w-body attribute to understand, that the (internal) body tags of the accordion tag should be inserted as children to the container tag to build up the accordion?

    /// accordion start tag
    if (input.renderBody) {
        input.renderBody(out);
    }
    /// accordion end tag
patrick-steele-idem commented 9 years ago

Did you forget the w-bind?

kristianmandrup commented 9 years ago

oh, I thought w-bind was only required for widgets? This is just a custom tag

kristianmandrup commented 9 years ago

With w-bind I get:

  return function render(data, out) {
    out.w('<div w-body w-bind class="ui ' +
      escapeXmlAttr(data.ui) +
      ' accordion"></div>');
  };

So this only works for widgets then?

patrick-steele-idem commented 9 years ago

w-body is also only for widgets (anything prefixed with w- is handled by marko-widgets). If you are not using marko-widgets then you need to do the following:

<div class="ui $data.ui accordion">
    <invoke function="data.renderBody(out)" if="data.renderBody"/>
</div>
kristianmandrup commented 9 years ago

why not enable w-body for tags? why have two different ways of achieving the same?

kristianmandrup commented 9 years ago

Great! it works :) But then the marko docs need to be updated to reflect this.

https://github.com/marko-js/marko/#tag-renderer

patrick-steele-idem commented 9 years ago

It's more complicated for the marko-widgets use case since marko-widgets will handle preserving nested HTML content when the outer widget is rerendered. It will also handle updating the nested HTML content when the outer widget does not need to be rerendered, but new nested content was provided. In addition, w-body must be bound to a container HTML element in order for everything to work.

kristianmandrup commented 9 years ago

Why not make some kind of convenience attribute or tag like w-body for this common use case at least?

kristianmandrup commented 9 years ago

Fx <tag-body/>, which simply calls <invoke function="data.renderBody(out)" if="data.renderBody"/>

kristianmandrup commented 9 years ago

Trying to patch this fix in marko

core-tag-transformer.js

var BodyNode = require('./BodyNode');
...
    [
        'tag-body', function(attr, node) {
            console.log('tag-body attr');
            var bodyNode = this.compiler.createNode(BodyNode, {
                    renderBody: true,
                    pos: node.getPosition()
                });
            node.parentNode.replaceChild(bodyNode, node);
            bodyNode.appendChild(node);
        }
    ],

BodyNode.js

'use strict';
function BodyNode(props) {
    BodyNode.$super.call(this, 'tag-body');
    if (props) {
        this.setProperties(props);
    }
}
BodyNode.prototype = {
    doGenerateCode: function (template) {
      template.statement('').indent(function () {
          this.generateCodeForChildren(template);
      }, this).line('');
    }
};

module.exports = BodyNode;

accordion-tag.marko

<div class='container' tag-body=''>
</div>

accordion-tag.js

var template = require('./accordion-tag.marko');

exports.renderer = function(input, out) {
  console.log('accordion input', input);
  template.render(input, out);
};

accordion-tag.json

{
    "renderer": "./accordion-tag.js"
}

marko-taglib.json

...
        "test-accordion": "taglib/accordion-tag.json",
<test-accordion>
  <h3>I'm your Buddy</h3>
</test-accordion>

What am I missing? I really don't like the idea of having to pollute my tags with <invoke everywhere...

It generates this:

  return function render(data, out) {
    __tag(out,
      ______taglib_accordion_tag_js,
      {},
      function(out) {
        out.w('<h3>I\'m your Buddy</h3>');
      });
  };
}

So I guess I just need to make the inner body output function self-calling!?

kristianmandrup commented 9 years ago

And I even try this, trying to simply wrap the InvokeNode invocation directly...

var InvokeNode = require('./InvokeNode');

function BodyNode(props) {
    console.log('BodyNode', props);
    return InvokeNode({function: "data.renderBody(out)", if: "data.renderBody"});
}

module.exports = BodyNode;
patrick-steele-idem commented 9 years ago

Hi @kristianmandrup, you don't need to patch marko to support this. It can be done completely in user land. Let me put together a separate project that you can use as an example.

kristianmandrup commented 9 years ago

Yeah, I know, but still... I'm just trying to understand some of the marko internals. Why won't this work?

'use strict';

var InvokeNode = require('./InvokeNode');

function BodyNode(props) {
  console.log('BodyNode', props);
  InvokeNode(props);
}

module.exports = BodyNode;

Looks like a pretty logical way to wrap <invoke> tag to me...

        'tag-body', function(attr, node) {
            var bodyNode = this.compiler.createNode(BodyNode, {
                    function: "data.renderBody(out)",
                    if: "data.renderBody",
                    pos: node.getPosition()
                });
            console.log('bodyNode', bodyNode);
            node.parentNode.replaceChild(bodyNode, node);
            bodyNode.appendChild(node);
        }
kristianmandrup commented 9 years ago

Thanks ;) I give up for now... tried all the ways I could think of.

kristianmandrup commented 9 years ago

Haha ;) As always, when I finally gave up a saw the light, and now I got it working in elegant fashion, exactly like wanted to :)

Pure and simple!

    [
        'tag-body', function(attr, node) {
            // node.setAttribute('renderBody', true, false);
            var invokeNode = this.compiler.createNode(InvokeNode, {
                    function: "data.renderBody(out)",
                    if: "data.renderBody",
                    pos: node.getPosition()
                });
            node.appendChild(invokeNode);
        }
    ],
kristianmandrup commented 9 years ago

My only problem is the test:

  1 failing

  1) render tag-body:
     Unexpected output for "/Users/kristianmandrup/repos/markoa/marko/test/fixtures/templates/tag-body/template.marko":
EXPECTED (/Users/kristianmandrup/repos/markoa/marko/test/fixtures/templates/tag-body/expected.html):
---------
<div class="container"><h3>I'm your Buddy</h3></div>

---------
ACTUAL (/Users/kristianmandrup/repos/markoa/marko/test/fixtures/templates/tag-body/actual.html):
---------
<div class="container"><h3>I'm your Buddy</h3></div>
---------
  Error: Unexpected output for "test/fixtures/templates/tag-body/template.marko":
  EXPECTED (test/fixtures/templates/tag-body/expected.html):
  ---------
  <div class="container"><h3>I'm your Buddy</h3></div>

  ---------
  ACTUAL (test/fixtures/templates/tag-body/actual.html):
  ---------
  <div class="container"><h3>I'm your Buddy</h3></div>
  ---------
      at EventEmitter.<anonymous> (test/util.js:72:27)
      at testRender (test/util.js:90:14)
      at Context.<anonymous> (test/util.js:116:13)

Very strange!?