Iterating over component data not as expected #583

Closed
daffl opened this Issue Nov 29, 2013 · 8 comments

Comments

Projects
None yet
5 participants
@daffl
Contributor

daffl commented Nov 29, 2013

A component with a simple template like

    <ul>
        {{#each data}}
        <li>{{@key}} : {{.}}</li>
        {{/data}}
    </ul>

Will not iterate as expected over the values in the data map. With a map like

    var data = new can.Map({
        some: 'test',
        things: false,
        other: 'things'
    });

The template in a component will output:

: [object Object]
: .map3
: [object Object]
: test
: false
: things
: [object Object]
: 5

Where the same template rendered via Mustache directly is the correct

some : test
things : false
other : things

Demo Fiddle: http://jsfiddle.net/GzpnF/1/

@justinbmeyer

This comment has been minimized.

Show comment
Hide comment
@justinbmeyer

justinbmeyer Nov 30, 2013

Contributor

How does it get the map? Can you add a test or a better explanation of the setup code.

Sent from my iPhone

On Nov 29, 2013, at 2:22 PM, David Luecke notifications@github.com wrote:

A component with a simple template like

<ul>
    {{#each data}}
    <li>{{@key}} : {{.}}</li>
    {{/data}}
</ul>

Will not iterate as expected over the values in the data map. With a map like

var data = new can.Map({
    some: 'test',
    things: false,
    other: 'things'
});

The template in a component will output:

: [object Object]
: .map3
: [object Object]
: test
: false
: things
: [object Object]
: 5
Where the same template rendered via Mustache directly renders the correct

some : test
things : false
other : things

Reply to this email directly or view it on GitHub.

Contributor

justinbmeyer commented Nov 30, 2013

How does it get the map? Can you add a test or a better explanation of the setup code.

Sent from my iPhone

On Nov 29, 2013, at 2:22 PM, David Luecke notifications@github.com wrote:

A component with a simple template like

<ul>
    {{#each data}}
    <li>{{@key}} : {{.}}</li>
    {{/data}}
</ul>

Will not iterate as expected over the values in the data map. With a map like

var data = new can.Map({
    some: 'test',
    things: false,
    other: 'things'
});

The template in a component will output:

: [object Object]
: .map3
: [object Object]
: test
: false
: things
: [object Object]
: 5
Where the same template rendered via Mustache directly renders the correct

some : test
things : false
other : things

Reply to this email directly or view it on GitHub.

@daffl

This comment has been minimized.

Show comment
Hide comment
@daffl

daffl Nov 30, 2013

Contributor

I thought the Fiddle illustrates the issue. As far as I can tell it doesn't really matter how the component gets the map (pre-initialized, as a plain object on the prototype or set as .attr in the inserted event)..

Contributor

daffl commented Nov 30, 2013

I thought the Fiddle illustrates the issue. As far as I can tell it doesn't really matter how the component gets the map (pre-initialized, as a plain object on the prototype or set as .attr in the inserted event)..

@justinbmeyer

This comment has been minimized.

Show comment
Hide comment
@justinbmeyer

justinbmeyer Nov 30, 2013

Contributor

Odd, my email did not include the fiddle link, but the website does.

Sent from my iPhone

On Nov 30, 2013, at 2:47 AM, David Luecke notifications@github.com wrote:

I thought the Fiddle illustrates the issue. As far as I can tell it doesn't really matter how the component gets the map (pre-initialized, as a plain object on the prototype or set as .attr in the inserted event)..


Reply to this email directly or view it on GitHub.

Contributor

justinbmeyer commented Nov 30, 2013

Odd, my email did not include the fiddle link, but the website does.

Sent from my iPhone

On Nov 30, 2013, at 2:47 AM, David Luecke notifications@github.com wrote:

I thought the Fiddle illustrates the issue. As far as I can tell it doesn't really matter how the component gets the map (pre-initialized, as a plain object on the prototype or set as .attr in the inserted event)..


Reply to this email directly or view it on GitHub.

@scorphus

This comment has been minimized.

Show comment
Hide comment
@scorphus

scorphus Dec 6, 2013

Contributor

Oh, I've spent more than 2 hours banging my head around this one, with no success. Any tips on where I should look? Thanks.

Contributor

scorphus commented Dec 6, 2013

Oh, I've spent more than 2 hours banging my head around this one, with no success. Any tips on where I should look? Thanks.

@imjoshdean

This comment has been minimized.

Show comment
Hide comment
@imjoshdean

imjoshdean Dec 6, 2013

Contributor

The problem is your can.Map in scope, for whatever reason, gets "upgraded" to a can.compute, by default with can.computes in #each we treat it as a list. #each goes something like this:

1.) If you are a compute, or you've an observable list, live bind iterate with can.view.lists
2.) Else if you're an array, iterate over like a normal array
3.) Else if you're a can.Map, get the keys with can.Map.keys, and live bind iterate over that
4.) Else if you're an object, iterate over the properties.

So in can.view.Scope it gets stuck at the first one, because the can.Map is wrapped in a can.compute. Is there a reason we're doing this? Perhaps some obvious solution that I'm missing.

Contributor

imjoshdean commented Dec 6, 2013

The problem is your can.Map in scope, for whatever reason, gets "upgraded" to a can.compute, by default with can.computes in #each we treat it as a list. #each goes something like this:

1.) If you are a compute, or you've an observable list, live bind iterate with can.view.lists
2.) Else if you're an array, iterate over like a normal array
3.) Else if you're a can.Map, get the keys with can.Map.keys, and live bind iterate over that
4.) Else if you're an object, iterate over the properties.

So in can.view.Scope it gets stuck at the first one, because the can.Map is wrapped in a can.compute. Is there a reason we're doing this? Perhaps some obvious solution that I'm missing.

matthewp added a commit to matthewp/canjs that referenced this issue Dec 7, 2013

Adds test for #each with can.Component
This commit adds a test that uses can.Component to render an {{#each}} block. The bug is unique to can.Component because can.Component wraps the can.Maps as computes. for #583

matthewp added a commit to matthewp/canjs that referenced this issue Dec 7, 2013

Allow #each to interate over Maps within computes
This commit fixes issue #583 by allowing the live bindings within can.view.lists to work with can.Maps as well as more list-y things. This is needed because using #each with a can.compute always goes through can.view.lists.
@matthewp

This comment has been minimized.

Show comment
Hide comment
@matthewp

matthewp Dec 9, 2013

Contributor

Decided the fix was whack, going to try a different approach. Should be easy to fix it right within each, however to do so you need to check if the compute value is a can.Map and not a can.List. To do that you have to get the value of the compute... but for some reason doing that causes the "no memory leaks with #each (#545)" test to fail. Any ideas why that would be?

Contributor

matthewp commented Dec 9, 2013

Decided the fix was whack, going to try a different approach. Should be easy to fix it right within each, however to do so you need to check if the compute value is a can.Map and not a can.List. To do that you have to get the value of the compute... but for some reason doing that causes the "no memory leaks with #each (#545)" test to fail. Any ideas why that would be?

@justinbmeyer

This comment has been minimized.

Show comment
Hide comment
@justinbmeyer

justinbmeyer Dec 9, 2013

Contributor

@matthewp ping me tomorrow to go over it.

Contributor

justinbmeyer commented Dec 9, 2013

@matthewp ping me tomorrow to go over it.

@daffl

This comment has been minimized.

Show comment
Hide comment
@daffl

daffl Dec 13, 2013

Contributor

What's the verdict on this one?

Contributor

daffl commented Dec 13, 2013

What's the verdict on this one?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment