[ns.Events] Неверно работает ns.Events.once

[iEgit]

ns.events.off не отписывает once события. Пример

const say = () => console.log('yo')
ns.events.once('yo', say)
ns.events.off('yo', say)
ns.events.trigger('yo')

// says 'yo'

Добавил ссылку на оригинальный обработчик и поправил off.

[iEgit]

@vitkarpov @i2r

[vitkarpov]

@chestozo @Katochimoto

[vitkarpov]

👍

[chestozo]

Круто, но для полноты надо поддержать двойной .once(), кажется, сейчас будут проблемы с перезатиранием __original.

[alexeyten]

С двойным once проблемы нет. Есть спецэффекты если я одну и ту же функцию добавлю через on и once, а потом вызову off на неё один раз. Удалится первая из добавленных функций, это может быть неожиданно.

[iEgit]

@chestozo проблемы не будет. Поговорили с @alexeyten, есть такая проблема.

const say = () => console.log('yo')
ns.events.once('yo', say)
ns.events.on('yo', say)
ns.events.off('yo', say) // снимет обработчик once, а не on

Cошлись на том, что с этим можно жить и достаточно описать эту особенность в документации.

Что думаете?

[vitkarpov]

А какое поведение ожидаемое в данном случае? Кажется, что должен снять оба обработчика, да?

[alexeyten]

@vitkarpov в том-то и дело, что любое будет неожиданным. Раньше .off снимал первый .on, но они все были одинаковые так что не было особенных проблем.

А вообще я с трудом представляю зачем может быть надо добавать одну и ту же функцию дважды.

[vitkarpov]

Ну, т.е. такой ситуации, как ты описал, по идее, не должно быть?

[vitkarpov]

В реальной жизни

[alexeyten]

Не должно. Поэтому я и предложил не пытатья её решить, а просто задокументировать, что поведение будет странным и что не нужно добавлять одну и ту же функцию в обработчики событий дважды

[alexeyten]

UPD: а, нет, не будет

[vitkarpov]

Ну и ок 👍

[alexeyten]

:+1:

[chestozo]

@alexeyten

А вообще я с трудом представляю зачем может быть надо добавать одну и ту же функцию дважды.

к примеру, по ошибке предлагаю в on() и в once() делать проверку и кидать warning, к примеру

@iEgit: Давай добавим тестов на:

• двойной once()
• once() + off() при этом off(eventName) и off(eventName, foo)

[alexeyten]

предлагаю в on() и в once() делать проверку и кидать warning, к примеру

Если только в DEV-сборке (как react и т.п.)

[vitkarpov]

Если только в DEV-сборке (как react и т.п.)

Есть ns.log.error — он как раз так и работает

[vitkarpov]

@iEgit ping? поправишь по комментариям, докинешь тестов:

• двойной once()
• once() + off()
• off(eventName) и off(eventName, foo)

[iEgit]

да, доделаю

[iEgit]

предлагаю в on() и в once() делать проверку и кидать warning, к примеру

А на что проверку? На то, что уже есть такой обработчик где-то?

[iEgit]

тесты добавил

[vitkarpov]

👍 @chestozo ?

[chestozo]

А какое поведение ожидаемое в данном случае? Кажется, что должен снять оба обработчика, да?

Я думаю мы ориентируемся на jquery. Там поведение такое - оба обработчика будут отписаны:

var $body = $(document.body);
var handler = function() {
    console.log(+new Date());
};

$body.one('click', handler);
$body.on('click', handler);

$body.off('click', handler);

$body.trigger('click');
// Ничего не происходит

[chestozo]

Может допилим до поведения "как в jquery"?

[iEgit]

А ты уверен, что нужно ориентироваться на эту великую технологию?

[vitkarpov]

Там поведение такое - оба обработчика будут отписаны

Вообще, кажется, логичным чтобы once и on в этом смысле работали одинаково, за тем лишь исключением, что once один раз выполнится.

Т.е. должно быть аналогично случаю:

foo.on('event', handler);
foo.on('event', handler);

foo.off('event', handler);

Сейчас это не так?

[chestozo]

А ты уверен, что нужно ориентироваться на эту великую технологию?

Пока используется jquery - лучше бы поведение наших эвентов было бы максимально аналогичным.

Сейчас это не так?

А я не знаю, давайте напишем тест :)

[vitkarpov]

А я не знаю, давайте напишем тест :)

https://github.com/yandex-ui/noscript/pull/607/files/a2917b0895c83c97132382849dd15099b23e680a#diff-99f6436afe06f3898363ed5e9798b602R144 — а вот есть тест на двойной once и off.

Работает консистентно с текущим поведением on:

• подписали дважды один и тот же обработчик
• сделали off
• убрался первый обработчик

Они сейчас все хранятся в одном массиве плоским списком, как только нашли первый handler, то удаляем его из массива и делаем break.

Мы не можем сделать поведение как в jquery, мы должны сделать как в ns.Events :)

[chestozo]

Поговорили голосом - завёл #613 По этой issue 👍 :)