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

GP-9: deploy on Netlify #4

Merged
merged 3 commits into from
Oct 7, 2020
Merged

GP-9: deploy on Netlify #4

merged 3 commits into from
Oct 7, 2020

Conversation

voronin-ivan
Copy link
Contributor

@voronin-ivan voronin-ivan commented Sep 27, 2020

В рамках следующего pr перенесу активацию деплоя на уровень gh-actions и добавлю проверки вроде линтера

@voronin-ivan voronin-ivan marked this pull request as draft September 27, 2020 08:36
@voronin-ivan
Copy link
Contributor Author

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

.eslintrc Outdated
@@ -5,6 +5,7 @@
},
"rules": {
"react/react-in-jsx-scope": "off",
"jsx-a11y/anchor-is-valid": "off"
"jsx-a11y/anchor-is-valid": "off",
"react/prop-types": "off"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

правило невалидно с тс работает

@voronin-ivan
Copy link
Contributor Author

https://5f723bf4151d840007c46347--growth-points.netlify.app/ - деплой этой ветки. После перехода на gh-actions будет автоматический коммент с адресом приходить

@voronin-ivan voronin-ivan marked this pull request as ready for review September 28, 2020 19:44
if (!req) return {};

try {
const url = getOriginFromRequest(req) + endpoints.api.hackathons;
const res = await axios.get<HackathonsApi>(url);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

почему пришлось переделать: в доке некст не рекомендуют обращаться к своим же ручкам, все из-за того, что нет нормального способа получить собственный хост, особенно в лямбда-функциях. Ну и поведение req на разных платформах отличается - в vercel, например, даже referer нет, не говоря про host

PavelNen
PavelNen previously approved these changes Sep 28, 2020
Copy link
Member

@PavelNen PavelNen left a comment

Choose a reason for hiding this comment

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

ок

Copy link
Contributor Author

@voronin-ivan voronin-ivan left a comment

Choose a reason for hiding this comment

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

@@ -28,6 +30,8 @@ module.exports = withBundleAnalyzer(withPrefresh({
config.externals.push(
/^(preact|preact-render-to-string|preact-context-provider)([\\/]|$)/,
);
} else {
config.plugins.push(new IgnorePlugin(/firebase/));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

т.к. взаимодействуем с firebase теперь внутри getInitialProps, нужно явно исключать из бандла (несмотря на проверку req и res), иначе на этапе сборке клиента получаем ошибку (firebase-admin не может работать на клиенте)

"start": "next start",
"start-netlify": "netlify dev",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

это просто для локального тестирования

export const getFirebaseAdminInstance = () => firebaseAdmin.initializeApp(
adminInitialConfig,
nanoid(),
);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

на каждый запрос возвращаем свой инстанс (а иногда и два разных), т.к. используем лямбда-функции, постоянно нельзя держать соединение - firebase/firebase-admin-node#929

};

const login = async ({ email, password }: LoginParams) => {
// with common instance we get a timeout on netlify
Copy link
Contributor Author

Choose a reason for hiding this comment

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

причем это касается только ручек /api, getUser.ts норм работает и с одним инстансом, т.к. используется только на самой странице и не является ручкой

@voronin-ivan voronin-ivan marked this pull request as ready for review October 6, 2020 11:16
- move logic from /api to /utils/firebase
- fix some auth-bugs
- exclude firebase from client-js
- unique firebase-instance for each request
Copy link
Member

@PavelNen PavelNen left a comment

Choose a reason for hiding this comment

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

Посмотрел, симпатично выглядит

@voronin-ivan voronin-ivan merged commit 2f82670 into master Oct 7, 2020
@voronin-ivan voronin-ivan deleted the GP-9 branch October 7, 2020 09:53
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.

2 participants