refactor(Function): use cloud function 2gen #107
Conversation
There was a problem hiding this comment.
Pra formalizar o que comentamos, a princípio mantemos a função na v1 e duplicamos criando outra na v2, deve ficar algo como:
exports[functionName] = functions.https.onRequest(server)
exports[`${functionName}v2`] = functionsV2.https.onRequest(server)Aí pegamos o URL da nova função e substituímos em uma segunda PR, antes disso onde faz referência ao ...cloudfunctions.net/... segue como tá (no workflow de deploy, env...)
scripts/firebase-deploy.js
Outdated
| } | ||
|
|
||
| client.functions.config.set(config, { project }) |
There was a problem hiding this comment.
Uai mas por que parou de enviar o config no deploy da função?
There was a problem hiding this comment.
Não funciona mais? Não mudou só sintaxe?
| @@ -36,12 +36,13 @@ | |||
| "homepage": "https://github.com/ecomplus/application-starter#readme", | |||
| "dependencies": { | |||
| "dotenv": "^10.0.0", | |||
| "firebase-tools": "^8.20.0", | |||
| "firebase-admin": "^10.3.0", | |||
| "firebase-tools": "^11.0.1", | |||
There was a problem hiding this comment.
Por que firebase-admin aqui?
O package da function tá na pasta functions, lá que precisa (e sempre teve) do firebase-admin nas dependências...
There was a problem hiding this comment.
Nesse packages.json no root são só dev deps pra deploy, lint, etc...
There was a problem hiding this comment.
E inclusive isso aqui tá gerando um conflito na PR no package-lock.json (aka não tem como dar merge 🙃 )
scripts/firebase-deploy.js
Outdated
| const envDir = path.resolve(__dirname, '../functions') | ||
| fs.writeFileSync(`${envDir}/.env`, env.join('\n'), 'utf-8') |
There was a problem hiding this comment.
Esse negócio ficou meio gambs né 🙃
functions/routes/ecom/webhook.js
Outdated
|
|
||
| const documentRef = admin.firestore().doc(`running/${storeId}`) | ||
| documentRef.get() | ||
| const documentRef = admin.firestore().doc(`running/${storeId}`) |
There was a problem hiding this comment.
Essa collection running era um workaround pra esse problema da (falta de) concorrência mesmo, ideal é até remover esse troço agora...
functions/index.js
Outdated
| const bodyParser = require('body-parser') | ||
| const bodyParser = express |
There was a problem hiding this comment.
Entendi essa mudança aqui não viu, acho que vai dar pau aí tentar parsear JSON ...
functions/__env.js
Outdated
| const { pkg, server } = require('firebase-functions').config() | ||
| const functionName = server.functionName || 'app' | ||
|
|
||
| require('dotenv').config() | ||
|
|
||
| const pkg = { | ||
| name: process.env.NAME, | ||
| version: process.env.VERSION, | ||
| } | ||
|
|
||
| const server = { | ||
| operator_token: process.env.SERVER_OPERATOR_TOKEN, | ||
| } | ||
|
|
||
| if(process.env.FUNCTION_NAME){ | ||
| server.functionName = process.env.FUNCTION_NAME | ||
| } | ||
|
|
||
| const functionName = server && server.functionName ? server.functionName : 'app' |
There was a problem hiding this comment.
#107 (comment)
Então , ainda tô nessa aí 😬
Por que trocar o config nativo por .env?
There was a problem hiding this comment.
https://firebase.google.com/docs/functions/beta/config-env
Aqui fala que o functions.config() foi removido no firebase 2gen e que ele foi substituído por variáveis de ambiente.
There was a problem hiding this comment.
Certo, mas nesse caso (já que .env passou a ter suporte nativo) não precisamos do require('dotenv').config(), precisamos? Pela doc aí parece não precisar, parece que o próprio Firebase já faz isso runtime...
There was a problem hiding this comment.
Fato. Realmente não precisar.
| @@ -36,12 +36,13 @@ | |||
| "homepage": "https://github.com/ecomplus/application-starter#readme", | |||
| "dependencies": { | |||
| "dotenv": "^10.0.0", | |||
| "firebase-tools": "^8.20.0", | |||
| "firebase-admin": "^10.3.0", | |||
| "firebase-tools": "^11.0.1", | |||
| @@ -36,12 +36,13 @@ | |||
| "homepage": "https://github.com/ecomplus/application-starter#readme", | |||
| "dependencies": { | |||
| "dotenv": "^10.0.0", | |||
| "firebase-tools": "^8.20.0", | |||
| "firebase-admin": "^10.3.0", | |||
| "firebase-tools": "^11.0.1", | |||
There was a problem hiding this comment.
E inclusive isso aqui tá gerando um conflito na PR no package-lock.json (aka não tem como dar merge 🙃 )
There was a problem hiding this comment.
Fora a diferença config/env, as alterações no webhook também deveriam ser aplicadas só no server na function v2, certo? Não é melhor a gente duplicar os arquivos necessários e editar, mantendo os originais pra function em produção?
Outra opção é você fazer um outro deploy do app (em outro app do Firebase), com o source proposto já pra v2, faz o deploy dele normalmente e testamos primeiro em uma loja teste, depois damos merge aqui ainda mantendo a function v1, pegamos o URL da v2 e depois removemos a v1 em produção...
[skip ci]
[skip ci]
[skip ci]
[skip ci]
[skip ci]
[skip ci]
[skip ci]
[skip ci]
Referente ao issue [#106]