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

changed each-behaviour for indexed arrays in 2.10 #14725

Open
Suven opened this issue Dec 16, 2016 · 8 comments
Open

changed each-behaviour for indexed arrays in 2.10 #14725

Suven opened this issue Dec 16, 2016 · 8 comments

Comments

@Suven
Copy link

Suven commented Dec 16, 2016

Hey!

I just noticed that Ember 2.10 introduces a changed behaviour when looping over indexed arrays with unset indexes.

Let's assume this array:

let a = [];
a[5] = 'foo';
a[6] = 'bar';

and this hbs:

{{#each testArray as |v k|}}{{k}},{{/each}}

In 2.9 this produces 5,6 and in 2.10 it produces 0,1,2,3,4,5,6.

I also made a twiddle for that

@pixelhandler
Copy link
Contributor

@Suven that seems like a bug. Perhaps the implementation has changed from using forEach to a for loop. (just a guess).

⚡ cat changed-each.js

a = [];
a[5] = 'foo';
a[6] = 'bar';

for (i = 0; i < a.length; i++) {
  console.log(i, a[i]);
}

a.forEach(function(v, i) {
  console.log(i, v);
});

⚡ node changed-each.js

0 undefined
1 undefined
2 undefined
3 undefined
4 undefined
5 'foo'
6 'bar'
5 'foo'
6 'bar'

@Suven
Copy link
Author

Suven commented Dec 16, 2016

Yup, that might be. I guess for-loops were tempting with perfomance having in mind first.

Just a little remark (although I guess that's obvious): This also happens when only the value is accessed (i.e each myArray as |val|)

@kamilogorek
Copy link
Contributor

I can confirm that it's bugged. Also using each-in instead of each temporary solves this problem.

@rwjblue
Copy link
Member

rwjblue commented Dec 27, 2016

The issue here is that (for performance reasons) we are using a standard for loop for {{#each support in 2.10+. Prior versions used .forEach and that natively support sparse arrays (and that is one of the reasons for some known performance issues).

We did it intend to support sparse arrays in prior versions and making changes to 2.10+ to support them would negatively impact performance of all usages of {{#each which is not an acceptable trade off for us.

We should document the fact that we do not support sparse arrays with {{#each in the API docs and add a test (likely a tweak of #14760) that shows the current behavior is intentional.

For folks that definitely need this support, I'd recommend using {{#each-in as someone else mentioned above.

@pixelhandler
Copy link
Contributor

@Suven I labelled this as "documentation" per @rwjblue comments above

For folks that definitely need this support, I'd recommend using {{#each-in as someone else mentioned above.

@lifeart
Copy link
Contributor

lifeart commented Sep 10, 2020

@rwjblue is it alive?

@rwjblue
Copy link
Member

rwjblue commented Sep 22, 2020

We need to write the docs I mentioned in my last comment.

@lifeart
Copy link
Contributor

lifeart commented Oct 17, 2020

@rwjblue #19147

@locks locks added Has PR and removed Help Wanted labels Feb 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants