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.Update если в момент начала отрисовки у модели state=invalid #649

Open chestozo opened 7 years ago

chestozo commented 7 years ago

В ns.Update есть такой код

Vow.invoke(this._requestAllModels.bind(this))
    .then(function(result) {
        // >>> Каким-то образом тут у какой-то из модели получаем state='invalid'
        // >>> И мы отрендерим error моду для вида.
        this._updateDOM();
        this._fulfill(result);
    }, this._reject, this)
    .then(null, this._reject, this);

У нас есть стабильный кейс (напишу тест), когда глобальный update запускается и в момент отрисовки одна из моделей в состоянии invalid. Update считает, что модель в состоянии ошибки и рендерит error моду для вида. Но у нас уже запущен (или вот вот запустится) другой update и всё перерисует.

Кажется, тут надо abort-ить update, а не рисовать error моду иначе всё моргает в интерфейсе.

vitkarpov commented 7 years ago

Кажется, тут надо abort-ить update, а не рисовать error моду иначе всё моргает в интерфейсе.

Я ж правильно понял, что, вообще говоря, error-мода вьюшки должна рисоваться. Т.е. если следующий апдейт не запустится или в следующем апдейте состояние модели не изменится?

chestozo commented 7 years ago

По идее, error мода нужна для состояния, когда модель в ошибке (state=error). А тут у нас модель в состоянии "меня проинвалидировали" (state=invalid) - кто-то вызвал model.invalidate().

Если считать, что этот кто-то, кто вызывал invalidate затем запустит ns.page.go() - то всё будет хорошо.

Итого предложение: Было

Стало

vitkarpov commented 7 years ago

Кул!

chestozo commented 7 years ago

Демо видео https://cl.ly/1q38461f1x2j

vitkarpov commented 7 years ago

Замутил фикс? :)

chestozo commented 7 years ago

У меня есть одна идея фикса, но она мне не до конца нравится.. Идея в том, чтобы кидать exception в ns.View.prototype._getModelsForTree если model.status === 'invalid' - я не до конца понимаю, насколько это безопасно.

chestozo commented 7 years ago

Пилю тут #650

chestozo commented 7 years ago

Один из вариантов обхода - не вызывать совсем model.invalidate(), вместо этого выполнять ns.forcedRequest.

vitkarpov commented 7 years ago

А не инвалидировать модель где?

chestozo commented 7 years ago

Ну где-то в коде ) у нас просто случай ручной инвалидации..

vitkarpov commented 7 years ago

под «где» я подразумевал на стороне приложения, да. уточнил, что я правильно тебя понял. но ведь это не решает проблему, в целом, верно? в следующий раз опять кто-то инвалидирует и будет париться что ж там случилось?

chestozo commented 7 years ago

да ( тут есть идея, что может ок рисовать вид с невалидной моделью? Потому что когда-то недавно эти данные были валидными. Ну т.е. у модели есть данные, просто status=invalid. Что скажешь? Ну т.е. даже update не отменять

vitkarpov commented 7 years ago

тут есть идея, что может ок рисовать вид с невалидной моделью?

Мне казалось, что статус invalid модели, по смыслу, как раз говорит о том, что вьюшке пора перерисоваться. Когда-то данные были валидными, потом их кто-то поменял. Никто не обязан дергать апдейт прямо при изменении модели (хотя обычно виды как раз и подписывают апдейт на эти изменения), но когда апдейт все же доберется до отрисовки видов, то они не имею право рисоваться по невалидным данным.

vitkarpov commented 7 years ago

Что скажешь? Ну т.е. даже update не отменять

Я соглашусь с тем, что вот прямо сейчас это место переделать может быть довольно сложно. Это ок.

Я сейчас больше про концепцию и признание того, что баг в каком-то виде в этом месте есть.

chestozo commented 7 years ago

Точно есть )

В общем варианта по идее 2:

По идее, оба должны привести к тому, что моргать не будет.

vitkarpov commented 7 years ago

рендерить так, как будто всё хорошо

Если так делать, то где гарантия, что, в итоге, нарисуется актуальный вид? :) Я вот этот момент упускаю

chestozo commented 7 years ago

Гарантии нет ) есть гипотеза, что раз модель невалидна, кто-то должен её перезапросить и отрендерить. aka выполнить ns.page.go().

vitkarpov commented 7 years ago

Чтобы лучше понять, попробую привести пример: меняю модель руками, т.е. делаю set какого-то поля. Запускаю апдейт. Модель перезапрашивать не надо, надо, чтобы вид отрисовался по новым данным.

В таком случае, как будет?

vitkarpov commented 7 years ago

делаю set какого-то поля

Вопрос здесь, скорее, делает ли это модель невалидной или нет?

chestozo commented 7 years ago

В таком случае всё хорошо, модель валидна, всё отрисуется не моргнув.

vitkarpov commented 7 years ago

Ага, понял что я путаю: когда поменял модель, то вид будет невалидным, а не модель.

chestozo commented 7 years ago

yep )

vitkarpov commented 7 years ago

Тогда и вправду получается, что если модель невалидная, то кто-то, в итоге, ее все же перезапросит — иначе зачем было помечать невалидной.

chestozo commented 7 years ago

вооооот да )

vitkarpov commented 7 years ago

рендерить так, как будто всё хорошо )

👍

Я ж правильно понял, что если так делать, то существующие тесты не падают? Т.е. более простой фикс получится

chestozo commented 7 years ago

Итого:

Я ж правильно понял, что если так делать, то существующие тесты не падают? Т.е. более простой фикс получится

Ща заценим )))

vitkarpov commented 7 years ago

Ща заценим )))

В ночную смену вышел на работу? ;)

chestozo commented 7 years ago

а сам-то сам )

chestozo commented 7 years ago

Я ж правильно понял, что если так делать, то существующие тесты не падают? Т.е. более простой фикс получится

тесты не упали ... что удивительно )

chestozo commented 7 years ago

Забубенил такой фикс https://github.com/yandex-ui/noscript/pull/651