search

bem / html-differ

Сompares two HTML
http://bem.info/tools/testing/html-differ/
MIT License
211 stars 44 forks source link

Fix _sortObj #142

Closed golyshevd closed 8 years ago

golyshevd commented 8 years ago

Без этого фикса ключи объектов внутри массивов не сортируются, поэтому рекурсивно вызываю _sortObj на каждом элементе массива

@eGavr

@truerenton @remnev

qfox commented 8 years ago

А это точно правильно?

golyshevd commented 8 years ago

А это точно правильно?

Ни один прежний тест не упал, мой тест прошел. Теперь ключи объектов железобетонно будут отсортированы на любой вложенности в любые контейнеры. По сути пришлось сделать кастомный JSON.stringify, оригинальный я использую только для сериализации примитивов.

Если знаете узкие места, я могу еще тестов добавить, скажите только какие кейсы хотите

qfox commented 8 years ago

Если знаете узкие места

По стандарту ключи в объектах не упорядочены. Реализации в разных популярных браузерах не следуют этому правилу и периодически что-то ломается, когда одни разработчики завязываются на реализацию, а другие на стандарт. Понять разработчиков браузеров можно — им нужно поддерживать наследие. Понять html-differ с таким поведением пока сложно.

Теперь ключи объектов железобетонно будут отсортированы

Да, но зачем? Это точно правильно их так сортировать? Почему текущее поведение неверное?

qfox commented 8 years ago

Есть кейс из жизни:

<body class="i-bem" data-bem='{"i-global":{},"b-page":{}}'> — было
<body class="i-bem" data-bem='{"b-page":{},"i-global":{}}'> — стало

2 часа искали причину.

golyshevd commented 8 years ago

Почему текущее поведение неверное?

Потому что по прежней логике объекты

[{a: 1, b: 2}]
// и
[{b: 2, a: 1}]

не эквивалентны, а точнее не всегда, потому что JSON.stringify может поменять местами ключи при сериализации. Прежняя реализация сортировки объекта не корректна, потому что спецификация не гарантирует какого-либо порядка при итерации по ключам объекта. Но в прежней реализации было сделано предположение, что порядок ключей соответствует порядку их присваивания, и это работает постолько поскольку. На это нельзя полагаться.

Есть кейс из жизни:

Тут тесты бы не упали и при прежней логике, но это проделки JSON.stringify

eGavr commented 8 years ago

@zxqfox

тут, кажется, действительно автор пулл-реквеста пофиксил злостную багу, которую я описал в issue – https://github.com/bem/html-differ/issues/33

qfox commented 8 years ago

Убедили. Вариантов нет, придется резать.

golyshevd commented 8 years ago

Если тут все норм, выпустите новую версию enb-bem-tmpl-specs?

eGavr commented 8 years ago

@golyshevd

про выпуск enb-bem-tmpl-specs это к @blond

eGavr commented 8 years ago

Спасибо за фикс

golyshevd commented 8 years ago

Ну тогда хотя бы этот модуль зарелизьте ) Спасибо!

eGavr commented 8 years ago

Готово ;)