mailru / fest

javascript templates
Other
128 stars 28 forks source link

IE8L: each перебирает параметры из прототипа #108

Closed burlakilia closed 8 years ago

burlakilia commented 9 years ago

Добавил проверку, что если параметр из прототипа то не обрабатывать его в итераторе. Иначе возникают проблемы при подключенных полифилах.

RubaXa commented 9 years ago

Где именно? Приведи пример.

burlakilia commented 9 years ago

Например добовляем к странице в ie8 скрипт автополифилера. После чего пишем простейший шаблон с each, например

..
<fest:script>
  var a = [1,2,3];
</fest:script>
...
<fest:each iterate="a" index="i" value="item">
   <fest:value>item + ', '</fest:value>
</fest:each>

Ожидаем что в итоговом html будет: '1, 2, 3,' , но в ie где подключен полифил видим следующее

1, 2, 3, function indexOf(searchElement) { for (var array = this, index = 0, length = array.length; index < length; ++index) { if (array[index] === searchElement) { return index; } } return -1; }, function filter(callback, scope) { for (var array = this, arrayB = [], index = 0, length = array.length, element; index < length; ++index) { element = array[index]; if (callback.call(scope || window, element, index, array)) { arrayB.push(element); } } return arrayB; }, function map(callback, scope) { for (var array = this, arrayB = [], index = 0, length = array.length, element; index < length; ++index) { element = array[index]; arrayB.push(callback.call(scope || window, array[index], index, array)); } return arrayB; }, function forEach(callback, scope) { for (var array = this, index = 0, length = array.length; index < length; ++index) { callback.call(scope || window, array[index], index, array); } }, 
RubaXa commented 9 years ago

Мы в прошлый раз пришли к выводу, что это не баг и человек, сделавший такое сам виноват, нельзя ходить по массиву через for in

burlakilia commented 9 years ago

Мне кажется это не правильно, т.к. в зависимости от браузера, это решение будет иметь различное поведение.

RubaXa commented 9 years ago

Но добавляя эту проверку мы снижаем производительность, на это диалог завершился, у нас есть два цикла, для каждого типа объектов.

@eprev @AndrewSumin решайте, я в прошлый раз был за единый цикл для любого типа объекта, а эти два для узкой специализации.

eprev commented 9 years ago

Все верно, each для объектов, for для массивов всегда использовался. Я вообще против каких либо вливаний в 0.8, если хотим на AST переходить.

eprev commented 9 years ago

Я прогнал через бенчмарк в dev:

w/o x 76,938 ops/sec ±0.66% (99 runs sampled)
w/  x 74,764 ops/sec ±0.88% (96 runs sampled)

Без проверки hasOwnProperty на 2k ops/sec быстрее.

У меня в ветке dev уже используется hasOwnProperty. Предлагаю смержить, а в 1.x убрать это под опцию компилятора.

@AndrewSumin ?