Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Feat/mobile app #47

Open
wants to merge 20 commits into
base: master
Choose a base branch
from
Open

Feat/mobile app #47

wants to merge 20 commits into from

Conversation

aliceinapple
Copy link
Member

@aliceinapple aliceinapple commented Jan 30, 2025 •
edited
Loading

Задача #46

  • собрала базовую структуру проекта
  • добавила заготовки для экранов
  • выяснила, что не могу использовать vanilla/extract для react native, добавила компонент Box с использованием @emotion/native

@aliceinapple aliceinapple self-assigned this Jan 30, 2025
@aliceinapple aliceinapple requested a review from Nelfimov February 5, 2025 16:42
@aliceinapple aliceinapple marked this pull request as ready for review February 5, 2025 16:42
Nelfimov
Nelfimov requested changes Feb 6, 2025
View reviewed changes
Copy link
Member

@Nelfimov Nelfimov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ну и работаем как с монорепой

mobile/app/.expo/devices.json Outdated
@@ -0,0 +1,3 @@
{
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

папка .expo вроде не нужна в гите, проверь

mobile/app/.idea/.gitignore Outdated
@@ -0,0 +1,3 @@
# Default ignored files
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

аналогично - гитигнор

mobile/app/.yarnrc.yml
@@ -0,0 +1,3 @@
enableGlobalCache: true
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

добавь путь до .yarn либо куда корень указывает

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

не сделано

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

не сделано

Вроде же добавила
yarnPath: ../../.yarn/releases/yarn-remote.cjs или это не то?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

комментарий к кэшу

@aliceinapple aliceinapple requested a review from Nelfimov February 7, 2025 12:22
Nelfimov
Nelfimov requested changes Feb 10, 2025
View reviewed changes
Copy link
Member

@Nelfimov Nelfimov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

не используешь монорепу

@aliceinapple
Copy link
Member Author

не используешь монорепу

Ты имеешь ввиду, добавить mobile в workspaces?

@Nelfimov
Copy link
Member

не используешь монорепу

Ты имеешь ввиду, добавить mobile в workspaces?

да и вынести ui и все остальные в отдельные пакеты

@aliceinapple
Copy link
Member Author

не используешь монорепу

Ты имеешь ввиду, добавить mobile в workspaces?

да и вынести ui и все остальные в отдельные пакеты

В mobile я использую node_modules, организовала внутри mobile отдельные пакеты screens и ui, где также использую node_modules. Или лучше вынести пакеты на уровень с mobile? Просто для screens например, логичнее было бы оставить его внутри mobile

@Nelfimov
Copy link
Member

См. как сделаны наши другие репы, примеры и как надо по fsd

@aliceinapple
Copy link
Member Author

См. как сделаны наши другие репы, примеры и как надо по fsd

Ну вот в примерах, demo-react-native - тут все пакеты внутри папки с мобильным приложением, используются node_modules, тоже самое в dating, только там все еще внутри папки src и импорты написаны так
import { Box } from '../../ui/layout'

С учетом того, что нам для mobile придется делать свой ui, я подумала, что логично будет организовать все так, что пакеты ui, screen и прочие связанные с mobile будут внутри mobile

@Nelfimov
Copy link
Member

Ну вот в примерах, demo-react-native - тут все пакеты внутри папки с мобильным приложением, используются node_modules, тоже самое в dating, только там все еще внутри папки src и импорты написаны так

В первом примере это по прежнему монорепа

import { Box } from '../../ui/layout'

С учетом того, что нам для mobile придется делать свой ui, я подумала, что логично будет организовать все так, что пакеты ui, screen и прочие связанные с mobile будут внутри mobile

А почему нужно делать свой ui? Vanilla не работает?

@aliceinapple
Copy link
Member Author

Ну вот в примерах, demo-react-native - тут все пакеты внутри папки с мобильным приложением, используются node_modules, тоже самое в dating, только там все еще внутри папки src и импорты написаны так

В первом примере это по прежнему монорепа

import { Box } from '../../ui/layout'
С учетом того, что нам для mobile придется делать свой ui, я подумала, что логично будет организовать все так, что пакеты ui, screen и прочие связанные с mobile будут внутри mobile

А почему нужно делать свой ui? Vanilla не работает?

Да, уже писала, что vanilla не работает

@aliceinapple
Copy link
Member Author

В первом примере это по прежнему монорепа

В первом примере используются node_modules везде, в нашем случае я использую node_modules только внутри mobile, потому что metro использует node_modules и не умеет работать с pnp. Если я и вынесу все пакеты отдельно, то мне нужно будет все используемые пакеты устанавливать еще и внутри mobile, потому что поиск нужных зависимостей будет происходить в node_modules

@Nelfimov
Copy link
Member

Но если у тебя цепочка

Энтрипоинт - экран - фрагмент - компонент

то будто зависимости работают, нет?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Nelfimov Nelfimov Nelfimov requested changes

Requested changes must be addressed to merge this pull request.

Assignees

@aliceinapple aliceinapple

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@aliceinapple @Nelfimov