steroids / react

SteroidsJS core library
MIT License
13 stars 0 forks source link

Некорректная работа роутинга #192

Open devmartynov opened 1 year ago

devmartynov commented 1 year ago

Глоссарий

МО - обычный маршрут. Маршрут в конфигурации которого не указано поле role либо не является одним из значений 'login' | 'modal' | '404'

МОК - МО, но в url которого есть квери параметры. Пример: /book?page=3

ММ - маршрут-модалка. Маршрут в конфигурации которого есть поле role='modal'

ММК - ММ, но в url которого есть квери параметры. Пример: /book?page=3

НН - нажатие на системную кнопку "Назад" в браузере

--> - переход на маршрут. Пример: --> МО следует читать как "переход на обычный маршрут"

|--> - начальная загрузка. Аналог перезагрузки страницы.

+ - показывает последовательность действий. Пример: |--> МО + --> ММ + НН следует читать как "Загрузка страницы на обычном маршруте, затем переход на маршрут-модалку, затем нажатие на системную кнопку 'назад' в браузере"

x - закрытие модального окна

Тест кейсы

  1. |--> МО
  2. |--> МО + --> ММ + НН
  3. |--> МО + --> ММ + x
  4. |--> МО + --> ММК + НН
  5. |--> МО + --> ММК + x
  6. |--> МОК
  7. |--> МОК + --> ММ + НН
  8. |--> МОК + --> ММК + НН
  9. |--> МОК + --> ММ + x
  10. |--> ММ
  11. |--> ММ + x
  12. |--> ММ + --> МО + НН
  13. |--> ММК + --> МО + НН

Примеры некорректной работы

  1. |--> МО + --> ММ + x

/book/kray-rodnoy/chapter_2 --> /book/kray-rodnoy/chapter_2/dictionary/1 + x = /book/kray-rodnoy/chapter_2?articleId=1

  1. |--> МО + --> ММК + x

/book/kray-rodnoy/chapter_2 --> /book/kray-rodnoy/chapter_2/audio-text?audioId=2 + x = /book/kray-rodnoy/chapter_2?audioId=2

  1. |--> МОК + --> ММ + x

/book/kray-rodnoy/dictionary?chapterId=2 --> /book/kray-rodnoy/dictionary/1 + x = /book/kray-rodnoy/dictionary?articleId=1

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

Дополнительно

Связанный PR. В рамках этого MR было сделано исправление, которое сказывается на работе других тест кейсов. А именно стали некорректно работать пункты: 12, 13

perlexed commented 1 year ago

На каких тест-кейсах роутинг работает некорректно и в чем заключается некорректность?

devmartynov commented 1 year ago

На каких тест-кейсах роутинг работает некорректно и в чем заключается некорректность?

я не закончил описание этого ишью. Я добавлю ответственного, когда закончу

perlexed commented 1 year ago

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

Кто сможет этим заняться? @devmartynov @Daria-Kuzminykh @Dmitriy-Babiy ?

devmartynov commented 1 year ago

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

Кто сможет этим заняться? @devmartynov @Daria-Kuzminykh @Dmitriy-Babiy ?

я займусь

devmartynov commented 1 year ago

@perlexed

я займусь

Примеры некорректной работы

  1. /book/kray-rodnoy/chapter_2 --> /book/kray-rodnoy/chapter_2/dictionary/1 + x = /book/kray-rodnoy/chapter_2?articleId=1
  2. book/kray-rodnoy/chapter_2 --> /book/kray-rodnoy/chapter_2/audio-text?audioId=2 + НН = not found
  3. /book/kray-rodnoy/chapter_2 --> /book/kray-rodnoy/chapter_2/audio-text?audioId=2 + x = /book/kray-rodnoy/chapter_2?audioId=2
  4. /book/kray-rodnoy/chapter_3?test=3 —> /book/kray-rodnoy/chapter_2/audio-text?audioId=3 + НН = not found
  5. /book/kray-rodnoy/dictionary?chapterId=5 —> /book/kray-rodnoy/dictionary/9 + x = /book/kray-rodnoy/dictionary?articleId=9
  6. /book/kray-rodnoy/dictionary/1 + x = /book/kray-rodnoy/dictionary?articleId=1

not found - это статус из хука useLayout

perlexed commented 1 year ago

@devmartynov это для информации? или от меня по задаче что-либо нужно?

devmartynov commented 1 year ago

@perlexed

Я неправильно понял вопрос

Кто сможет этим заняться?

Отнес этот вопрос к тому, что надо проверить тесты на исправленном коде. Поэтому надо снова людей пингануть с этим вопросом:)

Кто может заняться решением проблемы? @Daria-Kuzminykh @Dmitriy-Babiy

Daria-Kuzminykh commented 1 year ago

Промежуточный результат:

  1. Убрала логику с rememberedPrevRouterParamsRef, потому что такая же логика работает внутри usePrevious.
  2. Основная проблема: prevRouteParams в какой то момент обновляются лишний раз, поэтому становятся равными routeParams.
  3. Для сохранения параметров использовала usePreviousDistinct вместо usePrevious. Это решило проблему с вариантами 3, 7, 8. В вариантах 4, 5 проблема осталась. Вариант 6 не смогла воспроизвести.

Еще показалось, что нужно вот такое изменение в проекте:

image

Но оно не решило никакой проблемы. Но и не сломало ничего. Его я не сохраняла

Все изменения сохранила в репо react ветке routing-bug

Варианты дальнейшего решения:

  1. Написать свой usePrevious, который будет нормально работать на всех вариантах
  2. Переписать логику роутинга модалок как нибудь по другому
Daria-Kuzminykh commented 1 year ago

Итог:

Кейсы 3, 7, 8 пофикшены в этом pr Кейс 5 пофикшен в репо проекта Край родной в ветке update_steroids (см. последний коммит) Кейсы 4, 6 работают некорректно из-за редиректа на страницу not-found в проекте Край родной в файле AudioTextModal.tsx (оставила там туду, как можно пофиксить эту проблему)

@devmartynov Дальнейшие шаги:

  1. Поставила тебя ревьювером в этом pr, посмотри изменения, прокомментируй если нужно
  2. Смержить pr с исправлениями в репо react и поднять версию
  3. Пофиксить проблему с редиректом на страницу not-found в проекте
devmartynov commented 1 year ago

@Daria-Kuzminykh добавился еще один кейс некорректной работы:

1) |--> МОК + x

Пример:

|--> /book/kray-rodnoy/chapter_2/audio-text?audioId=2+ x = /book/:bookName/:chapterId

2) Также есть вот такой момент:

ММ --> ММ

То есть это ситуация, когда пытаемся открыть два маршрута(role='modal') одновременно. В этом случае первое модальное окно закрывается.