yandex-ui / noscript

Noscript: JavaScript MVC Framework for building SPA
http://yandex-ui.github.io/noscript/
MIT License
34 stars 22 forks source link

Сделать ns.Events консистентнее с jQuery Events #613

Open chestozo opened 8 years ago

chestozo commented 8 years ago

По следам #607

Хочется сделать ns.Events максимально консистентным с jquery. К примеру: ns.Events:

var hhh = function() { console.log(+ new Date()); };
ns.MAIN_VIEW.once('click', hhh);
ns.MAIN_VIEW.on('click', hhh);

ns.MAIN_VIEW.off('click', hhh);

console.log('1st');
ns.MAIN_VIEW.trigger('click')

console.log('2nd');
ns.MAIN_VIEW.trigger('click')

// 1st
// 1468485180585
// 2nd

jQuery:

var $body = $(document.body);
var hhh = function() { console.log(+ new Date()); };
$body.one('click', hhh);
$body.on('click', hhh);

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

console.log('1st');
$body.trigger('click')

console.log('2nd');
$body.trigger('click')

// 1st
// 2nd
iEgit commented 8 years ago

А точно после console.log('2nd'); не будет залогировано время?

vitkarpov commented 8 years ago

А точно после console.log('2nd'); не будет залогировано время?

Ну вот jQuery как раз снимает оба обработчика. Т.е. если несколько ссылок на один и тот же обработчик — он найдет их все и удалит, а у нас в ns.Events нашли в массиве первую ссылку, удалили и успокоились.

chestozo commented 8 years ago

А точно после console.log('2nd'); не будет залогировано время?

"примеры кликабельны" ;)

iEgit commented 8 years ago

В новой версии(0.8.6) снимется только once. А обработчик on останется.

iEgit commented 8 years ago

Мне кажется, что $body.off('click', hhh);, который снимает сразу два обработчика - это не лучшее решение jQuery, и не стоит быть в этом месте с ними заодно. А что, если я хочу снять только один обработчик?

vitkarpov commented 8 years ago

не стоит быть в этом месте с ними заодно

Мы просто не может быть незаодно, потому что jQuery переписать нельзя, а ns.Events можно. Речь ж о том, чтобы они были совместимы, потому что мы пользуем и тот и тот — в принципе, есть шанс нарваться на странный баг, когда в одном случае у тебя так работает, а в другом иначе.

vitkarpov commented 8 years ago

Ну, или мигрануть на новую версию jQuery, если такое поведение где-то поправили. Но это может быть болезненно и ненужно — лучше уж вообще от него отказаться :D

alexeyten commented 8 years ago

Хорош уже. Это нужно задокументировать как странный случай и не трогать. Я не могу представить зачем можно в здравом уме хотеть вешать один и тот же обрабочик событий дважды, а извращенцы должны страдать.

chestozo commented 8 years ago

А что, если я хочу снять только один обработчик?

А что если ты хочешь снять оба?

Я не согласен, что это полезная фича. Почему в jquery снимают оба мне ясно. Почему стоит снять только on мне не ясно.

iEgit commented 8 years ago

А что если ты хочешь снять оба?

Два раза сделаю off, нет проблем.

А почему в jQuery снимают оба?

chestozo commented 8 years ago

Два раза сделаю off, нет проблем.

А где 2 там и 3 ;)

Ну чтобы "2 раза не вставать" я так думаю. Ну т.е. off выключает обработку события и всё тут. А не то, что он что-то выключает, а что-то - не выключает. Это как-то странно.

Я согласен, с тем, что это кривые руки делать ns.events.on('myEvent', handler) + ns.events.once('myEvent', handler), но ещё раз повторюсь, это может быть багом и лучше не дать себе выстрелить в ногу.

3y3 commented 7 years ago

Давайте я вам приведу пример двойного навешивания обработчика:

    models: {
        'some-model': {
            'ns-model-changed.something': function() {
                ns.on('special:model:async-event', this.handle);
            }
        }
    },

    events: {
        'ns-view-hide': function() {
            ns.off('special:model:async-event', this.handle);
        }
    },

    methods: {
        handle: function(data) {
            if (data.finished) {
                ns.off('special:model:async-event', this.handle);
            } else {
                 ...
            }
        },

        someWaitMethod: function() {
             ns.once('special:model:async-event', this.handle);
        }
    }