fullstack-development / react-redux-starter-kit

Modular starter kit for React+Redux+React Router projects.
https://demo.fullstack-development.com/
MIT License
91 stars 13 forks source link

Выпилить shared/helpers/getErrorMsg #138

Open sk1e opened 4 years ago

sk1e commented 4 years ago

Вместо выкидывания исключения можно использовать discriminated union улучшив тем самым типобезопасность и исключив необходимость в лишнем импорте.

Znack commented 4 years ago

Тут норм, все согласны? @in19farkt @kinda-neat @chmnkh

in19farkt commented 4 years ago

Не очень понял альтернативу. getErrorMsg мы обычно юзаем в блоке catch, в который может прилететь всё что угодно, но обычно мы ждем ошибку. причем тут discriminated union?

sk1e commented 4 years ago

Не очень понял альтернативу. getErrorMsg мы обычно юзаем в блоке catch, в который может прилететь всё что угодно, но обычно мы ждем ошибку. причем тут discriminated union?

альтернатива - использовать if/else вместо try/catch и возвращать один из двух объектов { success: true, result: T } | { success: false, errorMessage: string } при вызове методов API

in19farkt commented 4 years ago

Мне кажется такая себе альтернатива. Тебе всё равно придется написать try-catch, но уже в слое api, причем в него придется завернуть каждый метод api, чтобы если че не просрать эксепшен, возникший в каких-нибудь конвертерах, а он может там возникунть, т.к. мы не ручаемся за то что придет с сервера.

in19farkt commented 4 years ago

Используя try-catch ты имеешь возможность различать разные типы ошибок с помощью instanceof и доставать из ошибки разную доп информацию, в варианте предлагаемым тобой придется добавлять вложенный уровень - discriminated union по типу ошибки, по-моему это как-то коряво.

sk1e commented 4 years ago

Мне кажется такая себе альтернатива. Тебе всё равно придется написать try-catch, но уже в слое api, причем в него придется завернуть каждый метод api, чтобы если че не просрать эксепшен, возникший в каких-нибудь конвертерах, а он может там возникунть, т.к. мы не ручаемся за то что придет с сервера.

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

Ещё была идея с повышением абстракции, которая включала привязку сервиса к rest и http методам, к redux для более удобной работы со стейтом авторизации и объявлением моков для каждого метода, чтобы была возможность переключения всего сервиса на мок-данные через переменную окружения, например (чтобы было проще работать когда бэкенда нет и когда он периодически падает). Думаю эти задачи будет разумнее делать вместе и думаю лучше это делать в этой задаче. Я тогда потом, когда будет время, сформулирую все абстракции, изменю текст этого ишью и пингану всех.

Znack commented 4 years ago

Только давай не свои костыльные реализации типа { success: true, result: T } | { success: false, errorMessage: string }, а нормальные ADT, которые за нас уже давно придуманы и обвешаны кучей комбинаторов :) В данном случае это Either<E, A>.

эксепшен, возникший в каких-нибудь конвертерах, а он может там возникунть, т.к. мы не ручаемся за то что придет с сервера.

Можно это и без эксепшенов обрабатывать, например, используя подход как в io-ts. ну или используя сам io-ts, который вполне себе production ready :)

В целом я за уход от эксепшонов и try/catch, я за нормальный type-safe control flow, который удобнее всего строить на Either и других ADT :) Что на бекенде в Хаскеле, что на фронте с TS моя практика показывает, что это вполне возможно и вполне хорошо работает

sk1e commented 4 years ago

а нормальные ADT, которые за нас уже давно придуманы и обвешаны кучей комбинаторов :) В данном случае это Either<E, A>.

Поясни плз. Какие комбинаторы? Что за Either? Ты не забыл что мы всё ещё на тайпскрипте пишем?)

Edit: нашёл в io-ts, надо опробовать будет

Znack commented 4 years ago

Edit: нашёл в io-ts, надо опробовать будет

круто 👍

in19farkt commented 4 years ago

Все равно не понимаю как вы с помощью Either и io-ts полностью уйдете от try/catch. Руками будете проверять всё, чтобы получить флаг true/false и вернуть left или right?

Either крут, в том плане, что мы явно понимаем, что может вернуться эксепшен из этой функции и придется учитывать это используя эту функцию. С этим я не спорю. Но это работает когда мы по какому-то флагу хотим выбросить эксепшен в теле функции, и не работает, когда мы используем стороннюю функцию, с которой может случиться всё что угодно и которая может выкинуть что-то типа "Can not read property 'bla-bla' from undefined". Особенно это актуально когда работаешь с бекендом, с которого в любой момент может прийти всё что угодно.

Вернемся к теме ишью. Иша о том, что нужно выпилить хелпер getErrorMsg, который нужен, чтобы просто в catch удобно выдернуть текст ошибки. Полностью от catch-ей мы избавиться не можем, поэтому пусть этот хелпер остается, в противном случае придется его постоянно на проектах дописывать. Сам код хелпера я бы заменил на такой:

function getErrorMsg(error: any) {
  if (error) {
    return error.message || String(error);
  }
  return 'Unknown error';
}

Еще в тс скоро подвезу Assertions in control flow analysis. Удобная штука, но с Either такое не проканает :)

Znack commented 4 years ago

Ну в частности с бекендом мы можем не использовать левые сторонние функции, а только type-safe exception-free решения, в частности io-ts как раз такое — ты можешь спокойно накидать конвертеры, которые не будут кидать cannot read property foo of undefined ни при каком теле ответа, который тебе бекенд выплюнул :)