CRA-template prototype review

[azinit]

По задаче #5 , на момент a3fb1ac

---

Для начала - спасибо большое, что смог заняться этим! Нанесешь непоправимую и мучительную пользу сообществу!

[azinit]

Начну с похвалы)

praise: классно, что так быстро забутстрапил темплейт, да и еще и для base и для ts!

Хотя щас возможно из-за этого придется в двух местах уточнять, но пока не суть 😄

[azinit]

praise: Здорово, что базово заполнил мета инфу!

[azinit]

praise: Классно, что получилось сделать небольшой по размеру темплейт

[azinit]

praise: Хорошо, что удалось перенять формат ридмишек, выглядит классно)

[azinit]

Теперь придирки ToImprove))

nitpick(critical): Не хватает в ридмихах упоминания Feature Sliced

Можно взять хотя бы как у эффектора

thoughts: Плюс в целом бы как-то еще пометить, что не просто теймплейт Feature Sliced, но и структура (с ссылкой на сайт офк)

[azinit]

nitpick(meta): Придирки по мета-инфе)

1. В keywords кажется можно добавить как минимум ["feature-sliced", "feature-based", "feature-slices"
2. В description лучше все же с большой буквы написать (Feature Sliced typescript template)
   • Или даже The offcial FeatureSliced template for Create React App. как у эффектора
3. А version я бы пока поставил 0.0.1 т.к. это все прототип пока

Аналогично также и с base темплейтом)

[azinit]

nitpick(meta): Ну и еще по мета инфе😄

4. В repository и bugs все же указал инфу о репозитории, как у эффектора

[azinit]

nitpick(fundamentals, public-api): Вот это прям вразрез с методологией идет)

Я бы добавил файлы реэкспорты в каждый компонент, и поправил импорты в shared/ui/index.js соответственно

В base и typescript соответственно 👀

[azinit]

thoughts: Да и раз мы делаем реэкспорт на весь shared/ui неплохо было бы комментарием добавить приписку о тришейкинге с ссылкой 🤔

https://feature-sliced.design/docs/concepts/public-api#%D0%B2%D0%BE%D0%B7%D0%BC%D0%BE%D0%B6%D0%BD%D1%8B%D0%B5-%D0%BF%D1%83%D1%82%D0%B8-%D1%80%D0%B5%D1%88%D0%B5%D0%BD%D0%B8%D1%8F

[azinit]

Тут тоже офк)

[azinit]

nitpick(fs, structure): Так нам бы или kebab-case или PascalCase в файловой структуре придерживаться)

Надо бы выбрать

JFYI, кортима активно приветствует именно kebab-case

[tednaaa]

я думал делать через PascalCase компоненты, а через kebab-case обычные модули :)

[azinit]

@tednaaa Надо бы что-то одно)
Иначе путанно, и будет раздрай

[azinit]

@feature-sliced/core что думаете?

[sergeysova]

Я за kebab-case все модули. Пусть структура определяет компонент это или нет, а не кейс символов.
Плюс избежим сразу огромного количества проблем с git и регистронезависимыми фс

[azinit]

Согл
+ кому оч надо, переведут на PascalCase новые модули
(потом в доке мб отдельно распишем проблематику эту)

[azinit]

nitpick(segments, non-blocking): Кмк тут как раз случай, когда необязательно все группировать в ui и хватило бы обычного

📁 pages/counter-page
     ui.jsx
     styles.module.css
     index.js

[azinit]

thoughts: Хотя не исключено, что меня просто зацепило излишнее дробление файлов в формате

pages/counter-page/ui/counter-page.jsx, когда можно было одним файлом обойтись

Так что некрит особо

[azinit]

nitpick(shared, routes): Задумка с роутами интересная вышла!

Но при всем уважении к этому, кажется в рамках темплейта так вовсе не обязательно упарываться)

issue(shared, routes):
Тем более, что вродь пока ни в чате, ни в кортиме пока не закреплялся активно вариант, когда в шейреде бы роуты хранились
(которые как бы завязаны на проект и роутинг)

suggestion(shared, routes): Поэтому я бы прямо в pages/routes.js роуты писал, как у кардбокса

• И сразу видно - что при обращении к роутам, придется лезть в уровень страниц - что осуждается по большей части

[tednaaa]

можно прямо в pages, согласен, но что делать, если к примеру внутри entities/foo нужно использовать history.push(routeNames.foo) либо что то связанное с конкретным роутом?

[azinit]

Не завязываться на конкретный роут :)

[azinit]

Никто же нам не мешает передать через пропсы/параметры нужный урл со страницы, а мы их обрабтаем как надо

И не будет размазывания ответственности

[tednaaa]

т.е роуты лучше держать внутри pages/router а не в shared/router ?

[azinit]

Именно

Глянь как у кардбокса)
Там тоже люди читерят тем, чтоб их юзать из слоев ниже, но зато явно сразу видно - что мы лезем в чужую зону ответственности
Лучше хотя бы явно чтоб это было

https://github.com/cardbox/frontend/blob/master/src/pages/routes.ts
https://github.com/cardbox/frontend/blob/master/src/pages/paths.ts

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

[azinit]

nitpick(app, public-api): Не хватает файла реэкспорта)

Даже хоть используется это только в руте, надо соблюдать концепции))

Иначе в раздрай уйдет

[azinit]

nitpick(app, providers):

1. Некрит, но генерацию роутинга вынес бы все же на уровень pages
   У нас ведь там роуты хранятся с маппингом - там же поидее и саму генерацию в jsx роутинг остается сделать (см. как у кардбокса)
   Я бы понял, если бы и роуты мы на уровне app объявляли... Хотя тогда еще больше проблем было бы))
   Энивей, кажется знание про роутинг - это дело pages. А app только подключает все нужное

2. А тут я бы как раз заморочился и выделил бы отдельный HOC-провайдер withRouter
   Чтобы люди не лепили с самого начала всю логику инициализации в одном жирном компоненте, а разбивали ее, как на скрине

   Можно посмотреть реализацию тут или тут

   Называть все же лучше providers, т.к. у того же Vue нет HOC для этого, но есть концепция plugins, которые то же самое делают
   

[azinit]

thoughts(non-blocking, model, ui): Возможно прозвучит как бред, но есть идея вынести отдельно в pages/counter-page/model.js хук типа useCounter, который бы давал все хендлеры нужные для "тупого" ui

thoughts: Чтобы идея разделения модели и ui была сразу, и чтобы люди не гнушались туда хотя бы такую логику выносить
(если СТМ не завели, то хотя бы так разделять

[azinit]

И этот хук возвращал бы как раз полный объект в формате

{ counter, setCounter, increment, decrement }

И с юзколлбэками офк)

[azinit]

thoughts(non-blocking): Опасно))

Но в целом некрит

[tednaaa]

а как? :) отдельно каждый пропс?

[azinit]

Ну там же ты по-факту 2-3 параметра передаешь

У тебя нет цели полностью кнопку воспроизвести

А пропс дриллинг такой опасен малясь

[azinit]

suggestion(promote): Надо продвигать слайсы в том числе и лого кмк)))

Кажется можно сделать прям как у эффектора и заменить основное реактовское лого на слайсовое

[azinit]

Вот тут файлы все нужные)

Поменять бы в обоих темплейтах

https://drive.google.com/drive/folders/15qy6E7OOkJiUWdY3I1DeB1L_3YsDoE8z?usp=sharing

[azinit]

praise: Кста очень классно, что baseUrl сразу задал! Почему-то не заметил сначала)

[azinit]

suggestion(meta): Ну в манифесте, собсна как и у эффектора, сделал бы описание)

А то у нас пока оч дефолтный манифест

[azinit]

question: Я не тестил конечно, но точно ли там префикс base обязателен?
У эффектора вродь без него обошлось 🤔

https://github.com/effector/cra-template/blob/5792e4be311cd9d97e45cb5336f15d316b71cae7/base/README.md#L18

[tednaaa]

оу, по поводу этого точно не подскажу :)

[azinit]

nitpick(critical): Я не сторонник нанооптимизаций, но кмк прям отдельную зависимость в шаблон тянуть малясь излишне 🤔

Если получится без нее реализовать нормально кнопку, было бы здорово
(можно сделать костыльный дедовский кастомизатор класса, но приписать мол - можете заюзать classnames/clsx или аналоги)

[azinit]

Фух, ну вроде до каждого файла докопался все базово глянул)

@tednaaa спасиб тебе большое , что взялся - выглядит как оч мощный кандидат на прототип темплейта!

Думаю потом можно будет и полноценный забабахать, со всеми слоями 🚀

[azinit]

@tednaaa Энивей, тут не было цели набрасывать на твои старания

А офк - чуть подзапариться над концептом темплейта, чтоб не стыдно было публиковать, даже как 0.0.1

Тем более, не хочется, чтобы люди с самого начала вредные практики начали применять

[tednaaa]

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

[azinit]

@feature-sliced/core Мб вам еще будет что добавить, пишите)

[azinit]

thoughts: @tednaaa Кста кажется еще, что в ридмихах темплейтов было бы нелишним добавить, что все доп. слои (entities/features/widgets) можно смело добавлять - но эти темплейты максимально упрощенные

И тоже ссыль на статью со слоями закинуть 🤔

[tednaaa]

окей, тоже добавлю, но это наверно останется в след выходные 😄

[azinit]

Ага)

Либо можно сделать рили как у https://github.com/cristobalgvera/ez

Т.е. src/entities/.gitkeep