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

Allow helpers to return primitives #647

Merged
merged 1 commit into from
Jun 2, 2015

Conversation

sethkinast
Copy link
Contributor

Previously returning a primitive would crash rendering with no way to recover.
Primitives are always HTML-escaped with no way to turn that off unless you override with a filters param. If you need to adjust the escaping, you need to return a Chunk and do the work yourself.

Closes #645

@smfoote
Copy link
Contributor

smfoote commented Apr 28, 2015

Looks good. My only question is, what is the difference between this.await and chunk.await?

@sethkinast
Copy link
Contributor Author

At the top of the function chunk gets aliased to this.

On Tue, Apr 28, 2015, 9:18 AM Steven notifications@github.com wrote:

Looks good. My only question is, what is the difference between this.await
and chunk.await?


Reply to this email directly or view it on GitHub
#647 (comment).

@smfoote
Copy link
Contributor

smfoote commented Apr 28, 2015

Got it. Thanks.

if (ret instanceof Chunk) {
return ret;
}
return chunk.reference(ret, context, 'h');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's have this accept a filters param from the helper and pass it to chunk.reference. 'h' will still be the default though.

Otherwise, looks good!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really like this. Thanks for the idea!

@sethkinast sethkinast force-pushed the helper-primitive branch 2 times, most recently from d070fe3 to a538b8c Compare April 29, 2015 18:39
}
return ret;
return chunk.reference(ret, context, 'h', filters);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know this isn't going to be easy but we should support pragmas and not hardcode the h here.

e.g.

{%esc:s}
  {@unsafe}{~n}
  {%esc:h}
    {@unsafe}
  {/esc}
{/esc}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Super legit, let me dive into that.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the great catch. Test cases added.

@sethkinast
Copy link
Contributor Author

To clarify from discussion offline with Jimmy:

Currently you can write a context helper that does this:

"url": function(chunk, context) {
  return 'http://' + context.get('url'); // safe
}

But you can't write a helper that does the same thing:

dust.helpers.addHTTP = function(chunk, context, bodies, params) {
  return 'http://' + context.get('url'); // Exception!
  return chunk.write('http://' + context.get('url')); // Evil! params.value is not escaped
};

So this is just intended to normalize these.

@sethkinast
Copy link
Contributor Author

As discussed in the WTF I've removed support for a filters param; we can maybe revisit it later.

So the new capability is only to normalize the behavior of global helpers and context helpers.

@sethkinast
Copy link
Contributor Author

Hahaha @jimmyhchan

I read the code for chunk.reference and look what it already does:

{foo|js|s}
function() {
  var d = 1;
  return {
    foo: function(chunk, context, bodies, params) {
      chunk.write(JSON.stringify(params));
    }
  };
}
{"auto":"h","filters":["h"]}

Dust already automatically passes auto and filters as params, so they are already reserved! That makes me feel a lot better about actually formalizing that for helpers...

Previously returning a primitive would crash rendering with no way to recover. You can still return a Chunk and do more complex work if you need to.

Helpers act like references or sections depending on if they have a body. When they have no body, they act like a reference and look in `params.filters` for filters to use. When they have a body, they act like a section. You can return thenables and streams normally.

{@return value="<Hello>" filters="|s" /}
{@return value="<Hello>"}{.} World{/return}

Closes linkedin#645
sethkinast added a commit that referenced this pull request Jun 2, 2015
Allow helpers to return primitives

Previously returning a primitive would crash rendering with no way to recover. You can still return a Chunk and do more complex work if you need to.

Helpers act like references or sections depending on if they have a body. When they have no body, they act like a reference and look in `params.filters` for filters to use. When they have a body, they act like a section. You can return thenables and streams normally.

{@return value="<Hello>" filters="|s" /}
{@return value="<Hello>"}{.} World{/return}

Closes #645
@sethkinast sethkinast merged commit e48bf62 into linkedin:master Jun 2, 2015
@sethkinast sethkinast deleted the helper-primitive branch June 9, 2015 21:11
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

Successfully merging this pull request may close these issues.

Returning a primitive from a helper should work
4 participants