baryshev / ect

Fastest JavaScript template engine with embedded CoffeeScript syntax
http://ectjs.com
MIT License
627 stars 70 forks source link

Issue with named blocks when a partial is used inside of for loops #83

Open paulyoung opened 9 years ago

paulyoung commented 9 years ago

Below is a contrived example that demonstrates my issue.

base

<% content 'before' %>
<p><% content %></p>
<% content 'after' %>

person

<% extend 'base' %>

<% block 'before' : %>
  <style type="text/css">
    #<%- @id %> {
      color: <%- @favoriteColor %>;
    }
  </style>
<% end %> 

<span id="<%- @id %>">Hello <%- @name %>!</span>

<% block 'after' : %>
  <script>
    var span;
    span = document.getElementById('<%- @id %>');
    console.log(span.innerText);
  </script>
<% end %>

people

<%
people = [
  {
    name: 'Alice'
    favoriteColor: 'red'
  }
  {
    name: 'Bob'
    favoriteColor: 'blue'
  }
]
%>

<% for person, index in people : %>
  <%
    options =
      id: "person_#{index}"
      name: person.name
      favoriteColor: person.favoriteColor
  %>
  <% include 'person', options %>
<% end %>

This produces:

<style type="text/css">
  #person_0 {
    color: red;
  }
</style>

<span id="person_0">Hello Alice!</span>

<script>
  var span;
  span = document.getElementById('item_0');
  console.log(span.innerText);
</script>

<style type="text/css">
  #person_0 {
    color: red;
  }
</style>

<span id="person_1">Hello Bob!</span>

<script>
  var span;
  span = document.getElementById('item_0');
  console.log(span.innerText);
</script>
paulyoung commented 9 years ago

Reproducable on http://ectjs.com:

screen shot 2014-08-18 at 17 37 56

paulyoung commented 9 years ago

The issue seems to stem the tracking of named blocks being somewhat naïve.

https://github.com/baryshev/ect/blob/0edfa8c4283153128363b53878e1bbdc4e6211c5/lib/ect.js#L303-L306

In my example, a block with the same name already exists (because I'm using the partial inside of a loop) and therefore the existing block is used.

kuba-kubula commented 9 years ago

This is problem of coffee-script itself, not ECT. You need to do something similar to this:

for i in [0..10]
  do (i) ->
    print i
kuba-kubula commented 9 years ago

And yes, blocks are not a good idea here.

paulyoung commented 9 years ago

This is problem of coffee-script itself, not ECT. You need to do something similar to this:

for i in [0..10]
  do (i) ->
    print i

I don't think that's correct. As you can see in the example above, the context is fine for unnamed blocks.

...
<span id="person_0">Hello Alice!</span>
...
<span id="person_1">Hello Bob!</span>
...

The issue appears to be that blocks can't be redefined as in #80.

If you can provide an example of how using do fixes the issue, that would be great.

paulyoung commented 9 years ago

And yes, blocks are not a good idea here.

What do you mean by that?

kuba-kubula commented 9 years ago

Blocks are not good because #80 I'd use a function returning HTML

<% fn (attr) => %>
write something: <%= attr %>
<% end %>
...
<%- fn (attributes...) %>
paulyoung commented 9 years ago

Thanks for the tip. Seems to be the best option as a workaround for now.