Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Нужно абортить ns.Update если в момент начала отрисовки у модели state=invalid #649

Open
chestozo opened this issue Apr 7, 2017 · 30 comments

Comments

@chestozo
Copy link
Member

chestozo commented Apr 7, 2017

В 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
Copy link
Member

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

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

@chestozo
Copy link
Member Author

chestozo commented Apr 7, 2017

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

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

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

  • рендерился вид в состоянии ошибки
  • очередной update приводил к нормальной перерисовке (очень часто это выглядит как моргание)

Стало

  • абортим update если одна из моделей с state=invalid
  • очередной update приводит к нормальной перерисовке (и по идее ничего не моргнёт)

@vitkarpov
Copy link
Member

Кул!

@chestozo
Copy link
Member Author

chestozo commented Apr 7, 2017

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

@vitkarpov
Copy link
Member

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

@chestozo
Copy link
Member Author

chestozo commented Apr 7, 2017

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

@chestozo
Copy link
Member Author

chestozo commented Apr 7, 2017

Пилю тут #650

@chestozo
Copy link
Member Author

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

@vitkarpov
Copy link
Member

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

@chestozo
Copy link
Member Author

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

@vitkarpov
Copy link
Member

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

@chestozo
Copy link
Member Author

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

@vitkarpov
Copy link
Member

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

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

@vitkarpov
Copy link
Member

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

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

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

@chestozo
Copy link
Member Author

Точно есть )

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

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

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

@vitkarpov
Copy link
Member

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

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

@chestozo
Copy link
Member Author

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

@vitkarpov
Copy link
Member

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

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

@vitkarpov
Copy link
Member

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

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

@chestozo
Copy link
Member Author

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

@vitkarpov
Copy link
Member

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

@chestozo
Copy link
Member Author

yep )

@vitkarpov
Copy link
Member

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

@chestozo
Copy link
Member Author

вооооот да )

@vitkarpov
Copy link
Member

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

👍

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

@chestozo
Copy link
Member Author

chestozo commented Apr 12, 2017

Итого:

  • или абортить update - вроде бы как не делаем лишнюю отрисовку, но и не показываем изменившиеся валидные модели
  • или рендерить так, как будто всё хорошо - делаем лишнюю отрисовку, но показываем изменившиеся валидные модели (if any)
    :)

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

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

@vitkarpov
Copy link
Member

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

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

@chestozo
Copy link
Member Author

а сам-то сам )

@chestozo
Copy link
Member Author

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

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

@chestozo
Copy link
Member Author

Забубенил такой фикс #651

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants