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.

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

SSR prod и dev режимы #59

Merged
merged 14 commits into from
Apr 11, 2023
Merged

SSR prod и dev режимы #59

merged 14 commits into from
Apr 11, 2023

Conversation

DNLHC
Copy link
Collaborator

@DNLHC DNLHC commented Apr 4, 2023

Сделана сборка для прод и дев режимов

Запуск ssr в дев режиме yarn dev:server
Запуск ssr в прод режиме yarn build && yarn build:ssr && yarn start

Чтобы корректно подгружать данные на сервере при рендере клиента, необходимо в роуте добавить необходимый запрос в loader
Переместил хранение выбранной пользователем цветовой темы в кукисы, чтобы сервер мог сразу отдавать HTML с нужным цветом.
Заменил eslint-config-node на eslint-config-n(форк) т.к первый больше не поддерживается
Заменил сборку сервера с commonjs на esm

packages/server/index.ts Show resolved Hide resolved
appType: 'custom',
});

app.use(vite!.middlewares);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Зачем тут восклицательный знак? По идее это важная ошибка и ее бы обрабатывать

Copy link
Collaborator Author

@DNLHC DNLHC Apr 6, 2023

Choose a reason for hiding this comment

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

Здесь нет ошибки. В режиме isDev у нас vite гарантированно есть (он определяется строкой выше), но TS об этом не знает, восклицательным знаком мы ему говорим напрямую, что здесь есть значение.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Я имел в виде, что важная ошибка, если его нет, и ее бы не игнорировать)
После инициализаци можно подмешать существование vite в значение isDev и избавиться здесь и дальше от игнорирование через восклицательный знак

Copy link
Collaborator Author

@DNLHC DNLHC Apr 9, 2023

Choose a reason for hiding this comment

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

Я имел в виде, что важная ошибка, если его нет, и ее бы не игнорировать)

Я выше писал, что vite не может не быть в isDev, т.к он в этом контексте и создаётся. Поэтому проверка внутри isDev избыточна.

После инициализаци можно подмешать существование

Приведёшь пример кода как это сделать?

Copy link
Collaborator

Choose a reason for hiding this comment

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

  1. Делаешь isDev let а не const (правда, по логике дальше создать бы другую переменную, что будет логичней)
  2. Когда инициализируешь vite, подмешиваешь ее в переменную выше isDev &&= !!vite
  3. Дальше делаешь проверки на эту новую переменную
  4. Если vite не проиницилизировался, но он он нужен в дев среде, то нужно отлавливать ошибку. Вероятность мала, но она может быть

packages/server/index.ts Show resolved Hide resolved
packages/server/index.ts Show resolved Hide resolved
packages/client/src/entry-client.tsx Outdated Show resolved Hide resolved
packages/client/src/components/client-only/client-only.tsx Outdated Show resolved Hide resolved
setHasMounted(true);
}, []);

if (!hasMounted) return null;
Copy link
Collaborator

Choose a reason for hiding this comment

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

А этот код разве не отрабатывает раньше, чем useEffect? Это метод работает корректно весь?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Отрабатывает раньше, в этом и задумка. Это обёртка для компонентов, которые не должны рендериться на сервере.

Copy link
Collaborator

Choose a reason for hiding this comment

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

И как он узнает, что он не на сервере, если у него всегда hasMounted === false? На клиенте он так же получит false, я не вижу никаких других зависимостей в компоненте, чтобы дойти до рендера компонентов

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

На клиенте сработает useEffect хук после первого монтирования, hasMounted станет true (7 строчка), компонент обновится и отрендерит children.
На сервере компонент не смонтируется и вернёт null.

Copy link
Collaborator

Choose a reason for hiding this comment

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

оки

@DNLHC DNLHC requested a review from EzhikTT April 6, 2023 14:41
appType: 'custom',
});

app.use(vite!.middlewares);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Я имел в виде, что важная ошибка, если его нет, и ее бы не игнорировать)
После инициализаци можно подмешать существование vite в значение isDev и избавиться здесь и дальше от игнорирование через восклицательный знак

packages/client/src/entry-client.tsx Outdated Show resolved Hide resolved
setHasMounted(true);
}, []);

if (!hasMounted) return null;
Copy link
Collaborator

Choose a reason for hiding this comment

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

И как он узнает, что он не на сервере, если у него всегда hasMounted === false? На клиенте он так же получит false, я не вижу никаких других зависимостей в компоненте, чтобы дойти до рендера компонентов

@DNLHC DNLHC requested a review from EzhikTT April 10, 2023 06:49
Copy link
Collaborator

@EzhikTT EzhikTT left a comment

Choose a reason for hiding this comment

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

Все ок, комменты не значительны для принятия

@DNLHC DNLHC merged commit 473dd6e into main Apr 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants