Выпилить ContainersProvider

sk1e commented 4 years ago

Эта штука провоцирует на зацикленные импорты, т.к. каждая фича использующая ContainersProvider, импортирует все фичи, контейнеры которых там зарегистрированы т.е. образуется связь many-to-many, для исключения которой фичи и были придуманы. В целом не вижу пользы от этой абстракции.

in19farkt commented 4 years ago

Да, циклические зависимости могут возникать, но насколько я помню мы их как-то разруливали на уровне ContainersProvider.

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

sk1e commented 4 years ago

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

Можно просто передавать из модуля

in19farkt commented 4 years ago

Можно просто передавать из модуля

Расскажу зачем мы добавили этот ContainersProvider. На реальном проекте была ситуация:

есть цепочка фич (некоторые асинхронные): Feature1 -> Feature2 -> Feature3 (Feature1 использует Feature2, которая в свою очередь использует Feature3)
в другом модуле есть такая цепочка: Feature4 -> Feature3
на этом этапе мы уже устали прокидывать все эти вьюхи из модулей
в Feature3 появляется функционал, который нужно вынести в отдельную фичу Feature5 и вместе с этим мы должны через все имеющиеся цепочки прокинуть эту Feature5 в Feature3
а цепочка была не прямой, а какой-то хитрой, там еще на некоторых уровнях вклинивались компоненты из shared.

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

Вот тут мы прямо вспотели и поняли что это дич и нужно дать фиче возможность запросить нужную вьюху самостоятельно. Вся эта дич еще наслоилась на то, что у нас фичи асинхронные, и мы должны были в модулях, на верхнем уровне, всё это прогрузить, собрать в кучу и отрисовать

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

chmnkh commented 4 years ago

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

Можно просто передавать из модуля

это на практике такое фиаско будет, мы же офигеем прокидывать эти долбаные контейнеры

in19farkt commented 4 years ago

ну вот я описал что бывает на практике :) Причем насколько я помню у меня было 2 проекта, на которых я на эти грабли наступил, во втором это дошло до абсурда и мы решили написать ContainersProvider

chmnkh commented 4 years ago

ну да, я собственно и плюсую)

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

sk1e commented 4 years ago

ну да, я собственно и плюсую)

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

С чего это фиаско? Добавляешь пропс в интерфейс контейнера, передаёшь в этот контейнер пропсом контейнер который хочешь передать. Две строчки. Какие проблемы?

Другое дело, когда вместо контейнеров фича предоставляет более сложные абстракции, например, как с виджетами было

Тогда зависимости контейнера виджета можно описывать на этапе генерации фичи фабрикой фич из https://github.com/fullstack-development/react-redux-starter-kit/issues/145 И это не будет приводить к зацикленности импортов в отличие от ContainersProvider

in19farkt commented 4 years ago

@sk1e я выше описывал проблему.

У тебя есть цепочка из 3-4 зависимых друг от друга фич, и чтобы прокинуть контейнер фичи на самую глубину нужно изрядно попотеть, пройтись по 10 уровням вложенности (контейнеры фич же не сразу друг в друга отрисовываются, там есть промежуточные компоненты) и расширить интерфейс всех компонент, которые ты встречаешь на своем не легком пути.

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

in19farkt commented 4 years ago

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

sk1e commented 4 years ago

@sk1e я выше описывал проблему.

У тебя есть цепочка из 3-4 зависимых друг от друга фич, и чтобы прокинуть контейнер фичи на самую глубину нужно изрядно попотеть, пройтись по 10 уровням вложенности (контейнеры фич же не сразу друг в друга отрисовываются, там есть промежуточные компоненты) и расширить интерфейс всех компонент, которые ты встречаешь на своем не легком пути.

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

Ну хз. Если действительно надо, то для проброски зависимостей вглубь дерева есть реакт контексты. Более понятное решение по сравнению с непонятно откуда приплывшими контейнерами. Разве нет?

in19farkt commented 4 years ago

Чем вариант с контекстом принципиально отличается от варианта с текущим провайдером?

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

Хотя конечно соглашусь, что с новым апи контекста и хуками это будет выглядеть немного лучше и удобнее, чем текущий вариант провайдера. И тема с циклическими зависимостями тут по умолчанию решается, как раз за счет того что мы откладываем момент привязки конкретных контейнеров к контексту.

sk1e commented 4 years ago

Чем вариант с контекстом принципиально отличается от варианта с текущим провайдером?

тем что контексты - уже готовое решение реакта из коробки, а провайдер - наш костыль. Если есть возможность решить средствами реакта - зачем своё изобретать?

Ну и в данный момент провайдер не на этапе инициализации получает эти контейнеры и способствует циклическим импортам.

in19farkt commented 4 years ago

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

Ну да, но этот костыль писали до того как появилось нормальное Context API, поэтому можно сказать что это легаси))

К чему мы в итоге пришли, нужно

рефакторнуть эту часть и переписать на реакт контекст
предоставить х(о|у)ки
разрулить типизацию, чтобы типы х(о|у)ками доставлялись и это не приводило к циклическим зависимостям

Еще предлагаю сделать так, чтобы этот провайдер биндил чисто вьюхи, и вообще никак не занимался подключением асинхронных фич. Я тут описывал предложение как это сделать можно https://github.com/fullstack-development/react-redux-starter-kit/issues/76

fullstack-development / react-redux-starter-kit

Выпилить ContainersProvider #134