Skip to content

<%= and <%- output null#559

Closed
colinta wants to merge 8 commits into
jashkenas:masterfrom
colinta:master
Closed

<%= and <%- output null#559
colinta wants to merge 8 commits into
jashkenas:masterfrom
colinta:master

Conversation

@colinta

@colinta colinta commented Apr 16, 2012

Copy link
Copy Markdown
Contributor

actually, in the version I was using in my project, <%= test %> outputs "" when {test:null}. but in the latest underscore, it outputs "null"

javascript
console.log(_('[<%= test %>] {<%- test %>}').template({test: null}));


outputs `[null] {null}`

This patch outputs `[] {}`.

@colinta

colinta commented Apr 16, 2012

Copy link
Copy Markdown
Contributor Author

I'm not a fan of the fix for interpolate (bfc7639), though it is simple and does pass the tests. But maybe there is a more elegant solution.

@colinta

colinta commented Apr 16, 2012

Copy link
Copy Markdown
Contributor Author

I've added a try..catch block to detect undefined variables. This brings it more into django-esque behavior.

In doing so, I also added similar support (for null and undefined) into _.escape, which may be undesirable. Please let me know, so I can fix my project code. I think the template language benefits from this feature, but that's me!

I did not create a minified version. I stick to uglifyjs, but that's not what you are using here (too much diff)

@octatone

Copy link
Copy Markdown
Contributor

It seems like you are adding type/value tests to a function purposed for blindly escaping HTML characters. Would it not make more sense to check the variable type/value before passing it to the template in your own code?

I understand why you might want _.escape to do what you are asking, but I think it's outside the realm of what _.escape is meant for.

@colinta

colinta commented Apr 17, 2012

Copy link
Copy Markdown
Contributor Author

@octatone, Yeah, I agree about _.escape. As a core function, I don't like the idea of putting type checks in there.

Then again, I hate that '' + null === "null" and var a; '' + a === "undefined". I would much prefer these to be "". _sigh_.

I've moved the code for that into the _.template method. I hope that someone from documentcloud gets a chance to consider the relative value of having _.escape(null) === "", but until then, I'm still very happy if the following code works:

<form>
<table>
  <tr>
    <th>Name</th><th>Email</th>
  </tr>
</table>
<button id="add_another">+ Add Another</button>
</form>

<script type="text/html" id="user_form">
<tr>
  <td><input type="text" name="user-<%= index %>-name" value="<%- user.name %>" /></td>
  <td><input type="text" name="user-<%= index %>-email" value="<%- user.email %>" /></td>
</tr>
</script>

<script type="text/javascript">

  var data = [
        {name: 'Colin', email: 'colinta@gmail.com' }
      , {name: null, email: null }
      , {}
    ]
    , template = _chz.user_form;

  _(data).each(function(row, index) {
    $('table').append(template({user: row, index: index}));
  });

  $('#add_another').on('click', function() {
    var index = $('table tr').length - 1;

    $('table').append(_chz.user_form({user: {}, index: index }}));
    return false;
  });

</script>
```html

I would expect this to result in:

```html
<form>
<table>
  <tr>
    <th>Name</th><th>Email</th>
  </tr>
<tr>
  <td><input type="text" name="user-0-name" value="Colin" /></td>
  <td><input type="text" name="user-0-email" value="colinta@gmail.com" /></td>
</tr>
<tr>
  <td><input type="text" name="user-1-name" value="" /></td>
  <td><input type="text" name="user-1-email" value="" /></td>
</tr>
<tr>
  <td><input type="text" name="user-2-name" value="" /></td>
  <td><input type="text" name="user-2-email" value="" /></td>
</tr>
</table>
<button id="add_another">+ Add Another</button>
</form>

Right now, this code ends up with "null" and "undefined" strings all over the place. Coming from mustache, django and jinja, this was a surprise to me.

@braddunbar

Copy link
Copy Markdown
Collaborator

I think that the try...catch is a bit overkill, but I can definitely see the usefulness of printing '' instead of 'null' or 'undefined'. Also, it would be great to see a performance comparison.

@colinta

colinta commented Apr 18, 2012

Copy link
Copy Markdown
Contributor Author

I know. I cringed when I put that in there, but that was the only way I could come up with that allowed <%= a.b..c %>, where a is undefined.

I haven't done a thorough comparison, but a rough one - running the 'utility' tests, puts them neck and neck. I will post a thorough benchmark later today.

On Apr 18, 2012, at 7:13 AM, brad dunbarreply@reply.github.com wrote:

I think that the try...catch is a bit overkill, but I can definitely see the usefulness of printing '' instead of 'null' or 'undefined'. Also, it would be great to see a performance comparison.


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

@braddunbar

Copy link
Copy Markdown
Collaborator

I don't think that we should swallow errors like <%= a.b %> where a is undefined. It prevents getting a good stack trace, especially when the expression contains a function call as in <%= foo() %>.

@colinta

colinta commented Apr 18, 2012

Copy link
Copy Markdown
Contributor Author

It's your call. I would be happy with either of two solutions:

  1. output nothing (coerce null/undefined to "")
  2. raise an error

The only solution I am not happy with is when it outputs "null" or "undefined". To me, that is coercing to an incorrect string.

@jashkenas

Copy link
Copy Markdown
Owner

This is a duplicate of the earlier #556 -- so let's move the conversation over there. In any event, we aren't going to add a try/catch to our interpolations.

@jashkenas jashkenas closed this Apr 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

Successfully merging this pull request may close these issues.

4 participants