Chore/use new modals

[egoarka]

btw. I don't know why so many changes in imports

upd.
that's because of small and handy feature enabled in my in vscode

{
  "editor.codeActionsOnSave": {
    "source.organizeImports": true
  }
}

[irbisdev]

Зачем в этом PR изменения по форматированию кода мобильных компонентов и грейд версии CocoaPods?

[irbisdev]

И не уверен, что стоит во всех подряд изменённых файлах менять порядок импортируемых компонентов.

[egoarka]

@irbisdev обновил первый пост, это из-за фичи в вскод, я ее выключил

а форматировал видимо потому что код смотрел и сейвил, постараюсь больше не делать так (откатил эдитор к дефолтным настройкам по максимуму)

[irbisdev]

Наверное, лучше тогда ревертнуть часть изменений связанных с форматированием мобильного кода и импортов, чтобы ребятам было проще глянуть File changes твоего PR

[egoarka]

да, окей, тогда ща этим займусь

[egoarka]

@irbisdev review it, please

[irbisdev]

import _ from 'lodash';
lodash обязательно использовать? Имхо, не выглядит так, что без него не обойтись. И мы его get(...) нигде не используем.

[egoarka]

@irbisdev я планировал юзать лодаш, или не юзать вообще что-ли? если мешает, я могу билд кастомный сделать, чтобы только нужный функционал брал

[irbisdev]

А какую проблему такое использование решает? Почему ты не хочешь напрямую к значению параметра объекта обращаться?

[egoarka]

так как optional chaining'а в ts еще не завезли, get юзаю таким образом, точно знаю что в рантайме не грохнет
пс еще много чего юзаю с лодаша, но ща таких кейсов нет

[irbisdev]

Ты можешь вместо
_.get(this.props, ['ignoreTargetQuery'], false);
прост сделать
!!this.props.ignoreTargetQuery и если значение undefined/null/false, то ты получишь false

[egoarka]

да, это понятно, но если вложенные будут, то тебе придется изголяться

interface ass {
  a?: {
    b?: {
      c?: {}
    }
  }
}

const r = {} as ass
const getC = r.a!.b!.c

а если вдруг какой-то пропертти будет undefined, то получим рантайм эррор

просто я не вижу ничего плохого в использовании лодаша для этой цели, при том что он еще типы выводит для пропертей

[irbisdev]

Хочешь сказать, что в данном кейсе у тебя может не быть this.props?

[egoarka]

нет, я вообще сниппет выше для примера привел, в лодаше вообще явно только один проперти получем, на остальное не обращаем внимание

[irbisdev]

Снипет понятен, но у тебя далеко не тот же кейс, что в нем представлен. Пропсы явно всегда есть в рендере. Усложнять читабельность кода из-за возможностей lodash, которые избыточны в данном случае, не выглядит рациональным.

[egoarka]

@irbisdev

как раз тот случай, чекни лайн _.get(router, ['query', q])

вложенность уже на два уровня

при том что q может быть undefined

[irbisdev]

Я же не про _.get(router, ['query', q]), а про _.get(this.props, ['ignoreTargetQuery'], false)

В первом случае, в принципе, понятно использование, но во втором явно избыточно.

[egoarka]

ничего страшного, никто от undefined или рантайм эррор не пострадает :)

[irbisdev]

Читабельность кода от этого страдает. Когда можно сделать !!this.props.ignoreTargetQuery вместо _.get(this.props, ['ignoreTargetQuery'], false), где _ - это импорт сторонней либы, то явно читабельнее и проще первое.

[egoarka]

твой пример не safety:

var a = { props: {} } 
!a.props.notexist

true

[irbisdev]

Хм. Мне казалось, что я чуть выше уже спрашивал.

Хочешь сказать, что в данном кейсе у тебя может не быть this.props?

[egoarka]

this.props существует всегда, я все это время говорил о пропсе/свойстве которое может не существовать на this.props

[irbisdev]

И что? Если ignoreTargetQuery === undefined/null/false, то в ts на !!this.props.ignoreTargetQuery вернёт false

[irbisdev]

Плюс, запусти, пожалуйста, yarn test

[egoarka]

вот эти хаки с !! никогда не юзал и не собираюсь, это прям совсем неявно

upd. обновил, тесты проходят

[irbisdev]

вот эти хаки с !! никогда не юзал и не собираюсь, это прям совсем неявно

Ок. Эт тогда лучше обсудить с @kor-ka. Я всё же за !!this.props.ignoreTargetQuery нежели _.get(this.props, ['ignoreTargetQuery'], false)

upd. обновил, тесты проходят

Зачем менять конфиг tslint? Не логичнее ли настроить свою тулзу, которую ты используешь для форматирования кода?

[egoarka]

зачем вообще tslint, какой от него профит, притера и тайпскрипта не хватает :) ?

[irbisdev]

[egoarka]

все решения с моей правкой конфига ведут на такому же солюшену как у меня

wmonk/create-react-app-typescript#263
palantir/tslint#1476

[kor-ka]

вот эти хаки с !! никогда не юзал и не собираюсь, это прям совсем неявно

upd. обновил, тесты проходят

Сказал человек который достаёт поле через строку

[irbisdev]

Если изменить название пропса или удалить его вовсе, то ts не заметит подвоха и пропустит использование старого названия в геттере через строку. На скрине как раз пример, когда я поменял название пропса и в _.get(this.props, ['ignoreTargetQuery'], false); ts проблем не видит

[kor-ka]

зачем вообще tslint, какой от него профит, притера и тайпскрипта не хватает :) ?

Профит в том чтобы явно/потенциально сломанный код падал ещё в тестах на ci

[egoarka]

@kor-ka не вижу ничего плохого, когда достаешь проперти по строке, при том что тайпскрипт помогает

убрал get, обратно конфиг вернул

[irbisdev]

Я про то, что он не будет ругаться, если ты его достаёшь строкой. На скрине как раз видно, что он ругается при нормальном использовании и не ругается при геттере строкой

[egoarka]

@irbisdev я тебя понял, да, тут возможно ты прав, но когда код писать будет, ты старый преперти закинуть потенциально не сможешь, тайпскрипт будет ругаться

[irbisdev]

А когда будешь его не с нуля писать, а поддерживать?

Пофиксь билд, пожалуйста.

[irbisdev]

Кстати, если я правильно понимаю, то надо перенести все старые модалки и не использовать в XModal роутер, в том числе:

• ImagePreviewModal
• PinMessageModal
• DeleteButton в orgView.page.tsx
• AvatarModal в UserProfileComponent.tsx
• WelcomePopup
• BrowserNotifications

(список не самодостаточен, так как составил при беглом просмотре find references для XModal)

[egoarka]

@irbisdev на роутах очень много завязано логики, я боюсь как бы не сломать существующую бизнес логику

я постарался максимально безопасно интегрировать императивные модалки, при этом не сломав существующий

[egoarka]

@irbisdev по поводу модалок выше - я выполнял фиксы по тем, которые есть в этом списке

https://www.notion.so/openland/Make-web-great-again-00498e855c0948498318a435abac5fa1