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

Fix inconsistency in resolving dust reference #47

Closed
vybs opened this issue May 27, 2012 · 4 comments
Closed

Fix inconsistency in resolving dust reference #47

vybs opened this issue May 27, 2012 · 4 comments
Assignees

Comments

@vybs
Copy link
Contributor

vybs commented May 27, 2012

given JSON

{
  "person": {
    "name": "Larry",
     "age": 45
 },
 "another": {
 in : {
  "name": "Tommy",
  "age": 44
  }
 }
}



  {#person}
    {name}, {age}
      {#another.in} // it is not a block, it is looking for local path, inside of person context
      {name}
      {age}
     {/another.in}
  {/person}

Expected : The above should work and output the data within the block. "another"
Observed: It does not output Tommy 44, instead only Larry 45 is shown

Alternatively,

The following works,

   {#another.in}
     {name}, {age}
   {/another.in}

why? since another is the top level block in the above example, using the . operator works. Any sub contexts using .operator do not work.

The only way to make the above work is reference a subcontext children is via creating a # context

 {#person}
   {name}, {age}..
  {#another}
   {#in}
     {name}
   {age}
  {/in}
 {/another}

{/person}

@ghost ghost assigned vybs May 27, 2012
@rragan
Copy link
Contributor

rragan commented May 28, 2012

Something similar to this issue has been causing me headaches. I took a stab at revising getPath to handle cases like this. Code is below. It needs some review from those more familiar with the innards of dust than I am so far plus throwing it against a battery of tests lest it break something. It seems to cure your cases and fix my headaches so I'm hopeful. Code adds two functions and modifies existing getPath. Note: find method is a near duplicate of get but returns pointer to location rather than value in location. I debated. I didn't consolidate them but one could have get call find to reduce code bulk.

// Find parent stack level with this key. Returns pointer or undefined if not found.
Stack.prototype.find = function(key) {
var stk = this, value;
while(stk) {
if (stk.isObject) {
value = stk.head[key];
if (!(value === undefined)) {
return stk.head;
}
}
stk = stk.tail;
}
return undefined;
};

Context.prototype.getPath = function(cur, down) {
var stk = this.stack,
len = down.length,
ostk = stk;

if (cur && len === 0) return stk.head;
if (!stk.isObject) return undefined;
stk = stk.head;
// See if path can be found from current position
stk = getSubPath(stk, down, len);
if (!stk) {
// See if there is a match to first path elt up-stack
stk = ostk.find(down[0]);
if (stk) {
// if found up-stack, see if whole path matches
return getSubPath(stk, down, len);
}
}
return undefined;
};

// Match a subpath in "down" from current position. Returns final match or undefined.
function getSubPath(obj, down, len) {
var i = 0;
while(obj && i < len) {
obj = obj[down[i]];
i++;
}
return obj;
}

@vybs
Copy link
Contributor Author

vybs commented May 29, 2012

@rragan

From what I understand,

get ( walks up the tree, so it is bottom up traversing of the context stack he creates )
Context.prototype.get = function(key) {
var ctx = this.stack, value;

while(ctx) {
if (ctx.isObject) {
value = ctx.head[key];
if (!(value === undefined)) {
return value;
}
}
ctx = ctx.tail;
}
return this.global ? this.global[key] : undefined;
};

getPath ( is walking down the tree from the "current stack head" )

  {#person}
   {name}, {age}
     {#another.in}  // the this.stack.head or the current stack head  is now pointing to { "name" :Larry, "age" : "45"}
     {name}
     {age}
     {/another.in}
   {/person} 

Hence it fails to find the another inside of the current stack head : { "name" :Larry, "age" : "45"}

Instead if it was a block, it would do a "get" and walk up the tree.

But the real confusing part is that, using the block # does not make it a new block at that point ( and use the the get )

It is the combination of # and . ( that is not spec'ed out clearly, as to what needs to happen )

so I am open to fixing this abd basically using the solution you propose.

@rragan
Copy link
Contributor

rragan commented May 29, 2012

I'm not sure I agree. I took your example and changed it slightly by adding a simple value as a peer of "person" and "another".

var json = {
"person": {
"name": "Larry",
"age": 45,
},
"another": {
in : {
"name": "Tommy",
"age": 44
},
},
"simpleAnother": "simpleValue"
}

{#person}
{name}, {age}
{#another}
{#in}
{name}-{age}
{/in}
{/another}
{/person}

{#person}
{simpleAnother}
{/person}

Then if I reference {simpleAnother} in the {#person} context, the get will go up the stack and find "simpleAnother".
However, if I happen to need simpleAnother to be a more complex object like name + age, I can no longer reference the parts of it -- this seems wrong.

Handlebars has a mechanism to allow accessing structured values from ancestors of the current context. https://github.com/wycats/handlebars.js
I'm not a big fan of Handlebars ../ notation but just letting paths find the path root "up" the tree and descend to a matched value feels natural to me.

The original docs for dust say
"To avoid brittle and confusing references, paths never backtrack up the context stack. If you need to drill into a key available within the parent context, pass the key as a parameter."
This seems very clear that using a path only goes down from the current context. I'm not sure why such references are brittle and confusing unless at the time he wrote this it was in light of handlebars use of .. to access parent contexts. I could see that being brittle and confusing. I find that {another} finding a simple value but {another.in.name} not finding the element of a basic structure up the tree is confusing to me. Finding path references "up" the tree seems as natural as finding a simple element up the tree -- certainly no more brittle.

As an alternative, he suggests passing the key as a parameter (e.g.
{#person name=another.in.name age=another.in.age}
++{name}, {age}
{/person}

This works but for any but the simplest substructure or with many values it quickly becomes onerous.

I keep hitting up against this issue when I do things that add a layer to the context stack because what was a current path ceases to be current and getPath won't find it.
Things I'm exploring that might hit this include
{@Local var="name"}valueForLocalVar{/local}
Though I was proposing allowing globals {+name} to be passed, the more I look a them the less I like them as a model for variables. They turn out to be just like the JS global var with all the problems of people stepping on your global name. I'd much rather have something like:
{@Local p1="2" p2="{b}"}
stuff that can ref the locals
{/local}
and avoid global name issues. However, adding the local names to the stack causes previous pathed "current context" references to break. If pathed references would just look "up" the tree, this and other scoping helpers would work fine.

Cheers, Rich


From: Veena Basavaraj [reply@reply.github.com]
Sent: Monday, May 28, 2012 7:25 PM
To: Ragan, Richard
Subject: Re: [dustjs] Fix inconsistency in resolving dust reference (#47)

@rragan

I was a little irritated with the inconsistency, but digging deeper, I think agree with his implementation now.

From what I understand,

get ( walks up the tree, so it is bottom up traversing of the context stack he created ),

Context.prototype.get = function(key) {
var ctx = this.stack, value;

while(ctx) {
if (ctx.isObject) {
value = ctx.head[key];
if (!(value === undefined)) {
return value;
}
}
ctx = ctx.tail;
}
return this.global ? this.global[key] : undefined;
};

getPath ( is walking down the tree from the "current stack head" )

  {#person}
   {name}, {age}
     {#another.in}  // the this.stack.head or the current stack head  is now pointing to { "name" :Larry, "age" : "45"}
     {name}
     {age}
     {/another.in}
   {/person}

Hence it fails to find the another inside of the current stack head : { "name" :Larry, "age" : "45"}

Instead if it was a block, it would do a "get" and walk up the tree


Reply to this email directly or view it on GitHub:
#47 (comment)

@vybs
Copy link
Contributor Author

vybs commented May 29, 2012

thanks for your detailed response.

We both seem to agree that the implementation is legit and is according to what he explains:)

the difference is basically between when it uses get and when it uses getPath ( a dot operator forces it to be local, walking down the tree )

(function(){dust.register(null,body_0);function body_0(chk,ctx){return chk.section(ctx.getPath(false,["person","in"]),ctx,{"block":body_1},null).write("------------------------");}function body_1(chk,ctx){return chk.reference(ctx.get("simpleAnother"),ctx,"h").reference(ctx.get("name"),ctx,"h").write(", ").reference(ctx.get("age"),ctx,"h").section(ctx.getPath(false,["another","in"]),ctx,{"block":body_2},null);}function body_2(chk,ctx){return chk.write(" // bloc inside the top level person block").reference(ctx.get("name"),ctx,"h").reference(ctx.get("age"),ctx,"h");}return body_0;})();

I am not a big fan of ../ either.

But I see your pain, some of us as well have felt the same, where "." has confused us ( since we most often overlook the current stack head )

Our solution : avoid the path and avoid inline params if possible, instead use blocks ( this uses the get( ) method and allows us to access the values upto to the root )

So our resort has been to use the block context whenever we need to walk into or access the data of other nodes in the JSON

https://github.com/linkedin/dustjs/wiki/Dust.js-little-less-know-language-constructs
For instance, one of the example under traversing the JSON tree in dust

If we want to access the projects -> name inside the comments block, all we need to do is create the team->projects block

Down side : we have to be careful in making sure that some typos in the JSON or missing data in some cases, does not let dust walk way more than we intended to.

Another topic you bring up is the helper : local

We do want to create a local scope at times,

for creating local contexts ( we create a new context, copy only the required context.get("somekey"), copy the inline params to it ans do a dust.makeBase( newlocalcontext) )

is this what you intended?

@vybs vybs closed this as completed Jun 18, 2012
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

2 participants