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

Alterar forma que estamos limpando os metadados para inserir na head #622

Closed
filipedeschamps opened this issue Aug 5, 2022 · 11 comments
Closed
Labels
back Envolve modificações no backend

Comments

@filipedeschamps
Copy link
Owner

filipedeschamps commented Aug 5, 2022

Contexto

@agjunior começou uma investigação do porque uma certa página no TabNews demorava para carregar através do profiling dela no frontend e descobriu que uma regex consumia grande parte do processamento (~83%). Em seguida, o @aprendendofelipe se aprofundou um pouco mais e entendeu com precisão o que estava acontecendo, ao ponto de conseguir elaborar um código markdown que travasse a página por completo.

Execução

Isto está acontecendo por conta de como o módulo remove-markdown funciona e nós usamos ele para fazer a limpeza do markdown dos conteúdos antes de inserir um valor limpo (sem formatação) nos metadados da página para alimentar melhor o SEO.

O @aprendendofelipe já está bolando uma proposta de como resolver isso.

@filipedeschamps filipedeschamps added the back Envolve modificações no backend label Aug 5, 2022
@aprendendofelipe
Copy link
Collaborator

@filipedeschamps eu enviei um PR que resolve esse problema de performance na biblioteca remove-markdown.

O que você prefere agora? Aguardar o desenrolar do PR, procurar outra biblioteca ou copiar o código corrigido para o repositório do TabNews enquanto não atualizam a biblioteca remove-markdown?

@coffeeispower
Copy link
Contributor

acho que da pra fazer upload do fork com um nome parecido no npm pra usar aqui e depois deletar quando for mergeado.

@coffeeispower
Copy link
Contributor

coffeeispower commented Aug 7, 2022

Mas tambem regex é uma coisa muito pesada de processar, é uma linguagem cheia de truques e simbolos e é uma dor pra implementar; o resultado: problemas de performance...

@filipedeschamps
Copy link
Owner Author

@aprendendofelipe que genial!!!

Topo total criarmos um model que retorne esse dado limpo! Ou se quiser, você pode fazer um fork do projeto e como a licença é MIT, você pode lançar seu próprio módulo no npm, por exemplo: fast-remove-markdown.

A última escolha dá bastante trabalho, mas é bastante recompensadora 🤝

@Rafatcb
Copy link
Collaborator

Rafatcb commented Aug 8, 2022

Tem a opção de usar o patch-package para modificar o código do remove-markdown no repositório do TabNews. Não sei se foi isso que o @aprendendofelipe quis dizer com "copiar o código corrigido para o repositório do TabNews".

@rodrigoKulb
Copy link
Contributor

rodrigoKulb commented Aug 8, 2022

@filipedeschamps, @aprendendofelipe, @coffee-is-power e @Rafatcb acho que tenho uma sugestão.

Hoje aplicamos removeMarkdown no corpo inteiro do texto. E no final só utilizamos 190 caracteres.

Podemos limitar uma quantidade inicial de caracteres exemplo 500 e depois fazer a limpeza o que acham?
Ponto positivo:

  • Teremos praticamente o mesmo tempo de limpeza, independente do tamanho do post

Ponto negativo

  • Existe a possibilidade de os 500 caracteres inicial serem todos tags, isso deixaria o head em branco. (raro).

Outra opção é gravar no banco os metadados, assim a limpeza vai acontecer no insert / update somente na inclusão ou modificação do post.

@aprendendofelipe
Copy link
Collaborator

@Rafatcb interessante o patch-package, mas acho que não vale a pena nesse caso, pois seria mais complexo do que criar uma função que faça o que precisamos.

@rodrigoKulb são sugestões interessantes para se pensar se vale a pena por outros motivos, mas não resolvem o problema atual, pois com bem menos de 500 caracteres é possível atingir o timeout das lambdas da Vercel com essa regex problemática.

@filipedeschamps
Copy link
Owner Author

Foi feito o merge do PR #627 e o @aprendendofelipe publicou uma comparação lá mostrando a diferença:

#627 (comment)

Nota que a versão original demorava 4.6 segundos enquanto a versão com o fix dele demora 100ms:

Função original Nova função
image image

@aprendendofelipe
Copy link
Collaborator

Nota que a versão original demorava 4.6 segundos enquanto a versão com o fix dele demora 100ms:

Na verdade a diferença é ainda mais absurda 😅

Foi de 4.6s para menos de 0.1ms... Esse de 100ms na direita é outro script

@filipedeschamps
Copy link
Owner Author

Nossa, é verdade! Sumiu qualquer Regex do topo da lista da direita 🎉

@filipedeschamps
Copy link
Owner Author

@aprendendofelipe eu vou encerrar essa issue e colocar no report dessa próxima segunda 🤝 E sobre cozinhar o resultado no getStaticProps, abrimos uma nova issue daí para resolver isso 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
back Envolve modificações no backend
Projects
None yet
Development

No branches or pull requests

5 participants