lazd / DOMly

The fast template system that creates and clones DOM nodes
MIT License
53 stars 9 forks source link

Merge parent data with child data (like dust.js, velocity etc) #22

Open georgkoester opened 9 years ago

georgkoester commented 9 years ago

I can create a patch - but would like to know if this has any chance. I am working in a project that is undergoing a change towards DOMly and would really like to have this in the default distribution. Maybe only as a per-template option or as new data variable 'merged' maybe (total would be: 'data', 'this', 'parent', 'merged').

Moustache works different I know, but seriously, I never understood this:

<div><h1 class="data.class">{{data.i18n("major_heading")}}</h1></div>
<ul>
  <foreach data.items>
    <li>parent.i18n(data.type): <span class="{{parent.class}} description">data.text </span>
  </foreach>
</ul>

Why can't this just be:

<div><h1 class="data.class">{{data.i18n("major_heading")}}</h1></div>
<ul>
  <foreach data.items>
    <li>data.i18n(data.type): <span class="{{data.class}} description">data.text </span>
  </foreach>
</ul>

In the moustache case if an extension requires another iteration to be added I also needn't add all those extra parent references:

<div><h1 class="data.class">{{data.i18n("major_heading")}}</h1></div>
<foreach data.groups>
  <h2>data.name</h2>
  <ul>
    <foreach data.items>
      <li>parent.parent.i18n(data.type): <span class="{{parent.parent.class}} description">data.text </span>
    </foreach>
  </ul>
</foreach>

I would love to look at this from a design perspective, maybe I am missing something that supports the moustache case.

lazd commented 9 years ago

Problems with this proposal

I haven't worked enough with dust.js and velocity to have seen this in practice, so forgive my ignorance please! I see a couple problems:

How to access parent data if both parent and child have a property by the same name?

At a glance, it looks like this would have problems if there was a property on the current object (data) that collides with the an object further up the object hierarchy (parent or parent.parent etc). Let's say you had the following structure representing a one-level deep menu:

{
  name: 'Furniture',
  url: '/furniture'
  children: [
    {
      name: 'Couches',
      url: '/furniture/couches'
    },
    {
      name: 'Tables',
      url: '/furniture/tables'
    }
  ]
}

And a template like:

<!-- Links to the parent's URL -->
<a href="{{data.url}}">{{data.name}}</a>
<if data.children>
  <nav>
    <foreach data.children>
      <!-- Links to the childs's URL -->
      <!-- Here, we need to show both the parent and the child's name -->
      <!-- If we've combined objects, how do we get to the parent's name? -->
      <a href="{{data.url}}">{{????.name}} -> {{data.name}}</a>
    </foreach>
  </nav>
</if>

As you can see, it is no longer obvious how to access the parent's name property in the above example.

Impossible to check if a property is undefined on the current object

Furthermore, it would be impossible to check the direct existence of a property on a the current object without potential interference from parent objects.

Let's say we have a data object like:

{
  name: 'Furniture',
  url: '/furniture'
  children: [
    {
      name: 'Couches' // No url property
    },
    {
      name: 'Tables',
      url: '/furniture/tables'
    }
  ]
}

And a template like:

<!-- Links to the parent's URL -->
<a href="{{data.url}}">{{data.name}}</a>
<foreach data.children>
  <!-- If we combine the parent and child data, how is it possible to know that the child does not specify a URL? -->
  <if data.url>
    <!-- Links to the childs's URL, if present -->
    <a href="{{data.url}}">{{data.name}}</a>
  <else>
    {{parent.name}} -> {{data.name}}
  </if>
</foreach>

Because you can't know for certain where the data.url property is coming from, this will lead to unexpected results.

DOMly's solution to the i18n part of the problem

You've got a couple tools at your disposal.

Use a method of this

You could have a helper function that you invoke when you want to render a template:

function renderTemplate(template, data) {
  var context = {
    i18n: function(string) { return translated[string]; }
  };

  // Call the template with this === context
  template.call(context, data);

  // Return context in case you've used handles in the template
  return context;
}

Then, inside of your template, this becomes the context object, so you can use this.i18n:

<div><h1 class="{{data.class}}">{{this.i18n("major_heading")}}</h1></div>
<foreach data.groups>
  <h2>{{data.name}}</h2>
  <ul>
    <foreach data.items>
      <li>{{this.i18n(data.type)}}: <span class="{{parent.parent.class}} description">{{data.text}}</span>
    </foreach>
  </ul>
</foreach>

Use a global

You can also just refer to a global i18n function:

<!-- This template assumes that MyApp.i18n is accessible in the global space -->
<div><h1 class="{{data.class}}">{{MyApp.i18n("major_heading")}}</h1></div>
<foreach data.groups>
  <h2>{{data.name}}</h2>
  <ul>
    <foreach data.items>
      <li>{{MyApp.i18n(data.type)}}: <span class="{{parent.parent.class}} description">{{data.text}}</span>
    </foreach>
  </ul>
</foreach>

Setup variables with <js>

This is as flexible as it gets. You could also use <js> blocks to setup variables you use later in your templates:

<js>
  var className = data.class;
  var i18n = data.i18n;
</js>
<div><h1 class="{{className}}">{{i18n("major_heading")}}</h1></div>
<foreach data.groups>
  <h2>{{data.name}}</h2>
  <ul>
    <foreach data.items>
      <li>{{i18n(data.type)}}: <span class="{{className}} description">{{data.text}}</span>
    </foreach>
  </ul>
</foreach>

The above options allow you to acomplish what you're trying to do and are 100% predictable. Thoughts?

georgkoester commented 9 years ago

Problems are minor in light of the the advantages of proposal

Problem 1 How to access parent data if both parent and child have a property by the same name?

Use parent special property, just as before:

<!-- Links to the parent's URL -->
<a href="{{data.url}}">{{data.name}}</a>
<if data.children>
  <nav>
    <foreach data.children>
      <!-- Links to the childs's URL -->
      <!-- Here, we need to show both the parent and the child's name -->
      <!-- If we've combined objects, how do we get to the parent's name? By using the parent context -->
      <a href="{{data.url}}">{{parent.name}} -> {{data.name}}</a>
    </foreach>
  </nav>
</if>

Better than current situation, because this happens less frequently than accessing a variable that is not part of the loop.

Problem 2 Impossible to check if a property is undefined on the current object

This can be fixed by either: The programmer has to ensure that variables are not undefined, but instead set to null:

{
  name: 'Furniture',
  url: '/furniture'
  children: [
    {
      name: 'Couches', // No url property
      url: null
    },
    {
      name: 'Tables',
      url: '/furniture/tables'
    }
  ]
}

Or: Providing the special property current:

<!-- Links to the parent's URL -->
<a href="{{data.url}}">{{data.name}}</a>
<foreach data.children>
  <!-- If we combine the parent and child data, how is it possible to know that the child does not specify a URL? -->
  <if current.url>
    <!-- Links to the childs's URL, if present -->
    <a href="{{data.url}}">{{data.name}}</a>
  <else>
    {{parent.name}} -> {{data.name}}
  </if>
</foreach>

Thanks for providing the solutions. I think I messed up my point when I tried to find a simple and easy to follow example. In my big project I see this all the time (handlebars problem):

...
{{#if object}}
...
{{#each list}}
...
{{#unless object2}}
...
{{#each list2}}
...
{{#partial ../../../../types}}
...

When templates get bigger following the ../ -trail gets increasingly complicated. Of course this is also contrary to all successful imperative programming languages. Imagine code like this in JS (this is handlebars-style JS):

var aFunc = function(param) {
  var a = parent.getA();

  for (var i = 0; i<parent.list.length; i++) {
    var current = parent.parent.list[i];
    parent.parent.doIt(current, parent.a);
  }  
};

This is how I feel when using handlebars in a more complex project setting, where templates aren't just three-liners. Merging contexts is intuitive and straightforward, let's include it, if only as an optional behavior.

lazd commented 9 years ago

DOMly has zero client-side runtime or dependencies. Introducing something like this would at least necessitate including a flatten(object) utility function to do the flattening (else the DOMly template functions would become bloated). That flatten function would look something like this.

The desired effect can be achieved by passing your data object to your own utility function before passing it along to the template:

// Using the simple flatten function from the Fiddle above
var fragment = template(flatten(data));

Since the data was flattened before it was passed to DOMly, there is no need to call flatten() within your templates when passing data along to helpers or partials. If you need slightly different output, including the parent and current properties, you can easily modify your flatten() function to include this output.

Since this can be easily accomplished with DOMly in its current form in a totally flexible fashion, and because adding this feature would necessitate including a runtime with DOMly, I don't think this should be part of DOMly.

@georgkoester, given the above, can you accomplish what you set out to do? If not, let's keep the conversation going and find a solution. Otherwise, let's close this one out.

georgkoester commented 9 years ago

@lazd a flatten function would not suffice. This is about a context stack where if locally a variable isn't found the next context on the stack is searched - just like a language interpreter stack with local variables. E.g. when starting a run through a foreach block a context is added to the stack and after the run it is removed again. This would make the runtime more complicated of course.

lazd commented 9 years ago

I see, so block scoping, effectively. I think I have a strategy to accomplish this with a little preprocessing and a little helper method inside, I'll report back after I have some time to play with it.

georgkoester commented 9 years ago

Wow awesome, I would love to see this addition. Will look into this a bit tonight.

georgkoester commented 9 years ago

After reading the compiler: The hardest part seems to be finding a good mechanism for the little stack crawling function.

lazd commented 9 years ago

Currently, DOMly creates a number of variables are named data_0, data_1 with the data for each block, then assigns data = data_1 as necessary. What we need to do is put those in an array -- effectively a stack of symbol tables -- then write a helper function that simply walks the array looking for the property of interest. It's pretty easy to do! I'll see what I can hack out next week, but if you want to take a stab at it, feel free!

I would like it to be behind an option, however, so we wouldn't incur the allocation of an array for every template render unless the option was on.

lazd commented 9 years ago

@georgkoester, check out the issue/22 branch for a solution!

Basically, you can pass useScope: true to the compiler and you now have a magical variable, scope, that, when dereferenced i.e. scope.property, will return the nearest value corresponding to property.

Given a template like this:

<h1>{{scope.category}}</h1>
<ul>
  <foreach data.items>
    <li>{{scope.category}}: {{scope.name}}</li>
  </foreach>
</ul>

The compiled template function looks like this:

(function anonymous(data_0) {
  var frag = document.createDocumentFragment();
  var data = data_0;
  var stack = [];
  stack.push(data_0);
  function __resolve(p) {  for (var i = stack.length - 1; i >= 0; i--) {    if (stack[i].hasOwnProperty(p)) return stack[i][p];  }  return "";}
  var el0 = document.createElement("h1");
  el0.textContent = __resolve("category");
  frag.appendChild(el0);
  var el2 = document.createElement("ul");
  var iterated_1 = data_0["items"];
  for (var i1 = 0, ni1 = iterated_1.length; i1 < ni1; i1++) {
    var data_1 = data = stack[1] = iterated_1[i1];
    var el6 = document.createElement("li");
    el6.textContent = __resolve("category")+": "+__resolve("name");
    el2.appendChild(el6);
  }
  stack.pop();
  frag.appendChild(el2);
  return frag;
})

With the following data:

{
  category: 'Furniture',
  items: [
    {
      name: 'Sofas'
    },
    {
      name: 'Tables'
    }
  ]
}

The output looks like this:

<h1>Furniture</h1>
<ul>
  <li>Furniture: Sofas</li>
  <li>Furniture: Tables</li>
</ul>

This is implemented by maintaining a stack (called stack) of the "symbol tables" corresponding to the available properties in each "block" created by a foreach or for in loop. Additionally, a __resolve(property) inner function is declared to perform a search of the stack.

When templates reference scope.property, __resolve('property') is called, which iterates in reverse order over the stack and returns the first instance of property in the stack

Because the __resolve() function is defined in each template, a 142 byte overhead is incurred per template. If DOMly had a runtime, then __resolve() would live in it and would instead be called as DOMly.resolve(stack, property), avoiding this overhead. In order the keep the compiler simple, the data_N variables are still maintained if useScope: true is passed, which incurs an additional 6-7 byte overhead for each loop vs. using stack in place of the variables.

The reason data_N variables were not ditched in favor of stack was to avoid the performance overhead of creating and garbage collecting an array for each template render, although I have yet to test whether this impact would be measurable.

lazd commented 9 years ago

I pushed another branch (issue/22-no-vars) that removes the usage of data_N vars and relies on scope exclusively (things got a little dirty there for a bit, heh, sorry for the issue reference pollution).

I quickly ran the benchmarks, and it seems the performance impact is barely measurable. Note that none of the benchmarks use the scope variable -- the performance comparison I'm talking about is between using data_N-style variables vs creating a scope array and doing scope[N] everywhere.

lazd commented 9 years ago

@georgkoester, any thoughts on the two branches I pushed?

georgkoester commented 9 years ago

Hi @lazd , that works awesome and is a nice simple change. I was working on switching the data variable completely to that kind of scoping and found that it complicates the code considerably, and requires to rework the testing setup. Thanks!

georgkoester commented 9 years ago

Seeing https://github.com/lazd/DOMly/commit/06cedca071797a367030a28b0b541c9fc41b760b I am really glad that the feature adds so neatly into the codebase! I was looking into making data block-scoped, removing the old access mode. I was not finished but I wasn't so neat. A good design choice!

lazd commented 9 years ago

@georgkoester if this is looking right to you, let's merge this and release a new version of DOMly.

lazd commented 9 years ago

Apologies for the issue reference barf due to all the rebasing, performance statistics are in for the various branches I've put together to close this issue. Note that the output of issue/22 is identical to master and any variation seen between master and issue/22 is due to environmental factors on the test machine:

benchmarks safari benchmarks firefox benchmarks chrome

See the spreadsheet here for the detailed numbed.

You can run the tests yourself:

git checkout master
grunt bench:domly
git checkout issue/22
grunt bench:domly
git checkout issue/22-no-vars
grunt bench:domly

Here is the build output that is being benchmarked:

You can see that Firefox and Safari don't have any statistically meaningful variation between the branches, but Chrome has some different results for the Markup and Variables benchmarks.

I can't get this result consistently, so I assume it may be due to GC pauses happening at odd times.

The main point is that issue/22-no-vars is using an array inside of the template, and that master/issue/22 are not.

I think the benchmarks are acceptable for either branch, I'm going to do some testing on iOS and see what I can find.