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

Issue : GH-266 store rendered template in global does not work for partials.! #292

Closed
vybs opened this issue Jun 23, 2013 · 9 comments
Closed

Comments

@vybs
Copy link
Contributor

vybs commented Jun 23, 2013

Too bad that the context.global__template_name__ does not reflect the partial name.

Hence the fix is to store a stack of template names and push/pop and in the helpers do a peek() to get the current template.

the fix is simple to push the current template elem in chunk.partial and pop it off before returning from the function call.

#274

@rragan your thoughts.? also should I change the template name to be $template_name to be consistent?

@rragan
Copy link
Contributor

rragan commented Jun 24, 2013

Curious, what do you need the names for? Debugging? If it is only debugging, maybe there should be a build option so we don't pay this cost all the time.
Would you also want the name of a helper in addition to partials?

@vybs
Copy link
Contributor Author

vybs commented Jun 24, 2013

name of helper, i guess we have to inject it in the compiler.

Here is our i18n helper we use for looking up strings on the client/browser, we CDN the translated strings
The reason why It is good to store the current template name : instead of having to tell every @i18n call to include the template name in the params
Second, we could also autogenerate the template name at build time, but it is duplicate code leading to more payload in the JS templates. At our scale, we'd like to optimize every bit we can for the JS downloaded via CDN

Third, the number of i18n strings in our templates accounts to >50% of the markup.

dust.i18n = dust.i18n || {};
dust.i18n.cache = dust.i18n.cache || {};

/**
 * Return translated text for the specified key in the specified template.
 * Note: template is optional, if given overrides the name of the current template rendered
 * Example:
 * <p>{@i18n key="hello_world"}Hello World{/i18n}</p>
 * <p>{@i18n key="hello_world" text="Hello World"/}</p>
 * <p>{@i18n key="close" template="foo/global"/}</p>
 * Output:
 * <p>Hello World</p>
 * @param chunk
 * @param context
 * @param bodies
 * @param params
 *      <p>template="foo/global", lookup template cache</p>
 *      </>hide="true", does not render the i18n in place, stores it in the given template cache</p>
 *      </p>output="json" , stores the value in the current template cache</p>
 */
dust.helpers.i18n = function(chunk, context, bodies, params){

 if(params && params.hide === 'true'){
   // do not render
   return chunk;
 }
 if(params && (typeof params.key !== 'undefined')){
  var key= params.key,
      overrideTemplateName = params.template,
      currentTemplateName = context.global.__template_name__[context.global.__template_name__ - 1];

  var templateName = overrideTemplateName || currentTemplateName;

  if(typeof templateName !== 'undefined') {
    var templateDictionary = dust.i18n.cache[templateName];
    if(templateDictionary){
      var text = templateDictionary[params.key];
      if(text){
       if(!params.output){
        return chunk.write(text);
       }
       else {
        context.stack.head[key] = text;
        return chunk;
       }
      }
     }  
     // fallback to english for self-closing
     var text = params.text;
     if(text){
      return chunk.write(text);
     }
     // fallback to english for non self-closing
     else if(bodies.block) {
      return chunk.render(bodies.block, context);
    }
  }
  return chunk;
 }
};
})();

@rragan
Copy link
Contributor

rragan commented Jun 24, 2013

Prefer $ names for consistency

Sent from my iPhone

On Jun 23, 2013, at 5:14 PM, "Veena Basavaraj" <notifications@github.commailto:notifications@github.com> wrote:

name of helper, i guess we have to inject it in the compiler.

Here is our i18n helper we use for looking up strings on the client/browser, we CDN the translated strings

dust.i18n = dust.i18n || {};
dust.i18n.cache = dust.i18n.cache || {};

/**

  • Return translated text for the specified key in the specified template.

  • Note: template is optional, if given overrides the name of the current template rendered

  • Example:

  • {@i18n key="hello_world"}Hello World{/i18n}

  • {@i18n key="hello_world" text="Hello World"/}

  • {@i18n key="close" template="foo/global"/}

  • Output:

  • Hello World

  • @param chunk

  • @param context

  • @param bodies

  • @param params

  •  <p>template="foo/global", lookup template cache</p>
    
  •  </>hide="true", does not render the i18n in place, stores it in the given template cache</p>
    
  •  </p>output="json" , stores the value in the current template cache</p>
    

    */
    dust.helpers.i18n = function(chunk, context, bodies, params){

    if(params && params.hide === 'true'){
    // do not render
    return chunk;
    }
    if(params && (typeof params.key !== 'undefined')){
    var key= params.key,
    overrideTemplateName = params.template,
    currentTemplateName = context.global.template_name[context.global.template_name - 1];

    var templateName = overrideTemplateName || currentTemplateName;

    if(typeof templateName !== 'undefined') {
    var templateDictionary = dust.i18n.cache[templateName];
    if(templateDictionary){
    var text = templateDictionary[params.key];
    if(text){
    if(!params.output){
    return chunk.write(text);
    }
    else {
    context.stack.head[key] = text;
    return chunk;
    }
    }
    }
    // fallback to english for self-closing
    var text = params.text;
    if(text){
    return chunk.write(text);
    }
    // fallback to english for non self-closing
    else if(bodies.block) {
    return chunk.render(bodies.block, context);
    }
    }
    return chunk;
    }
    };
    })();


Reply to this email directly or view it on GitHubhttps://github.com//issues/292#issuecomment-19884358.

@vybs
Copy link
Contributor Author

vybs commented Jun 24, 2013

thanks, will send a PR for this change ( so I assume we are ok with storing a array of visited templates with push and pop. Is there a way to expose peek() on arrays in JS in a neat way? rather than doing array[array.length -1]?

@rragan
Copy link
Contributor

rragan commented Jun 24, 2013

shift/unshift and ref first element. Not sure how perf compares with push/pop

Sent from my iPhone

On Jun 23, 2013, at 6:08 PM, "Veena Basavaraj" <notifications@github.commailto:notifications@github.com> wrote:

thanks, will send a PR for this change ( so I assume we are ok with storing a array of visited templates with push and pop. Is there a way to expose peek() on arrays in JS in a neat way? rather than doing array[array.length -1]?


Reply to this email directly or view it on GitHubhttps://github.com//issues/292#issuecomment-19885037.

@vybs
Copy link
Contributor Author

vybs commented Jun 24, 2013

stack vs queue
http://jsperf.com/push-pop-vs-unshift-shift/3

push/pop is faster.

@rragan
Copy link
Contributor

rragan commented Jun 24, 2013

Not sure how critical name of helper is. The code in the helper knows its own identity and if it calls partials the name changes anyway. Just asking to trigger thinking about it.

Rich

From: Veena Basavaraj [mailto:notifications@github.com]
Sent: Sunday, June 23, 2013 5:15 PM
To: linkedin/dustjs
Cc: Ragan,Richard
Subject: Re: [dustjs] Issue : GH-266 store rendered template in global does not work for partials.! (#292)

name of helper, i guess we have to inject it in the compiler.

Here is our i18n helper we use for looking up strings on the client/browser, we CDN the translated strings

dust.i18n = dust.i18n || {};

dust.i18n.cache = dust.i18n.cache || {};

/**

  • Return translated text for the specified key in the specified template.

  • Note: template is optional, if given overrides the name of the current template rendered

  • Example:

  • {@i18n key="hello_world"}Hello World{/i18n}

  • {@i18n key="hello_world" text="Hello World"/}

  • {@i18n key="close" template="foo/global"/}

  • Output:

  • Hello World

  • @param chunk

  • @param context

  • @param bodies

  • @param params

  •  <p>template="foo/global", lookup template cache</p>
    
  •  </>hide="true", does not render the i18n in place, stores it in the given template cache</p>
    
  •  </p>output="json" , stores the value in the current template cache</p>
    

    */

dust.helpers.i18n = function(chunk, context, bodies, params){

if(params && params.hide === 'true'){

// do not render

return chunk;

}

if(params && (typeof params.key !== 'undefined')){

var key= params.key,

  overrideTemplateName = params.template,

  currentTemplateName = context.global.__template_name__[context.global.__template_name__ - 1];

var templateName = overrideTemplateName || currentTemplateName;

if(typeof templateName !== 'undefined') {

var templateDictionary = dust.i18n.cache[templateName];

if(templateDictionary){

  var text = templateDictionary[params.key];

  if(text){

   if(!params.output){

    return chunk.write(text);

   }

   else {

    context.stack.head[key] = text;

    return chunk;

   }

  }

 }

 // fallback to english for self-closing

 var text = params.text;

 if(text){

  return chunk.write(text);

 }

 // fallback to english for non self-closing

 else if(bodies.block) {

  return chunk.render(bodies.block, context);

}

}

return chunk;

}

};

})();


Reply to this email directly or view it on GitHubhttps://github.com//issues/292#issuecomment-19884358.

@vybs
Copy link
Contributor Author

vybs commented Jun 24, 2013

not sure I understand, second this is our internal helper on how we do i18n, the helper name and the i18n string cache are different objects.

can you elaborate more?

@rragan
Copy link
Contributor

rragan commented Jun 24, 2013

It was just a question to make us think about completeness. If we want to know name of initial template while it runs or current partial while it runs, do we want to know name of currently running helper within the helper. My take is no -- especially based on your i18n because the helper name would hide the name of the partial/template it was being used in.

vybs added a commit that referenced this issue Jul 17, 2013
GH-292 support template name in nested partials, need a stack to store the current name
vybs added a commit that referenced this issue Jul 17, 2013
add support for GH-292, support the template name in nested partials
@vybs vybs closed this as completed Jul 17, 2013
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