-
Notifications
You must be signed in to change notification settings - Fork 9
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
Merge parent data with child data (like dust.js, velocity etc) #22
Comments
Problems with this proposalI 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 ( {
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 Impossible to check if a property is undefined on the current objectFurthermore, 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 DOMly's solution to the i18n part of the problemYou've got a couple tools at your disposal. Use a method of
|
Problems are minor in light of the the advantages of proposalProblem 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:
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 objectThis can be fixed by either: The programmer has to ensure that variables are not undefined, but instead set to
Or: Providing the special property
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):
When templates get bigger following the
This is how I feel when using handlebars in a more complex project setting, where templates aren't just three-liners. |
DOMly has zero client-side runtime or dependencies. Introducing something like this would at least necessitate including a 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 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. |
@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 |
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. |
Wow awesome, I would love to see this addition. Will look into this a bit tonight. |
After reading the compiler: The hardest part seems to be finding a good mechanism for the little stack crawling function. |
Currently, DOMly creates a number of variables are named 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. |
@georgkoester, check out the Basically, you can pass 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 When templates reference Because the The reason |
I pushed another branch (issue/22-no-vars) that removes the usage of I quickly ran the benchmarks, and it seems the performance impact is barely measurable. Note that none of the benchmarks use the |
@georgkoester, any thoughts on the two branches I pushed? |
Hi @lazd , that works awesome and is a nice simple change. |
Seeing 06cedca I am really glad that the feature adds so neatly into the codebase! |
@georgkoester if this is looking right to you, let's merge this and release a new version of DOMly. |
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 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 I think the benchmarks are acceptable for either branch, I'm going to do some testing on iOS and see what I can find. |
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:
Why can't this just be:
In the moustache case if an extension requires another iteration to be added I also needn't add all those extra parent references:
I would love to look at this from a design perspective, maybe I am missing something that supports the moustache case.
The text was updated successfully, but these errors were encountered: