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

Habilitar CORS no serviço #9

Closed
lucianopf opened this issue Jan 31, 2020 · 10 comments
Closed

Habilitar CORS no serviço #9

lucianopf opened this issue Jan 31, 2020 · 10 comments

Comments

@lucianopf
Copy link
Member

Descrição

Precisamos explicitamente habilitar o CORS * no serviço para que outras pessoas consigam integrar sem problemas em seus sites

Screen Shot 2020-01-31 at 10 33 55

Provavelmente a melhor solução pra gente tratar esses headers de config padrão é usar o arquivo de config com as rotas definidas 🤔

https://zeit.co/docs/configuration#routes/headers

@filipedeschamps
Copy link
Member

Uhhh nice catch!

Outra alternativa é fazer pelo Nextjs usando um middleware: https://github.com/Timer/next.js/blob/master/packages/next/README.md#api-middlewares

image

Mas de fato, teria que fazer manualmente em todas as rotas e isso uma hora vai escapar. Se pelo Now dá para fazer um wildcard para todas as rotas seria sensacional!!!

@lucas-eduardo
Copy link

lucas-eduardo commented Feb 14, 2020

@lucianopf @filipedeschamps

Fiz uma implementação e já realizei o teste sobre essa issue...o PR aberto foi: https://github.com/filipedeschamps/BrasilAPI/pull/17

Fiz um custum server para essa configuração conforme a documentação do next fala. Quero estar contribuindo nesse projeto... aguardo por um feedback

@filipedeschamps
Copy link
Member

Show!!! Eu comentei algo importante lá no seu PR 👍

@filipedeschamps
Copy link
Member

Showww @OtavioCapila !! Eu não sei qual os padrões dos projetos nesse caso, sou super novo no framework, mas me parece mais previsível para evolução do framework fazer manualmente em cada endpoint ou nas configs 👍 Mas muito legal a sua sugestão e dentro de um ecossistema Node.js normal isso faz sentido 🤝 Vou lhe adicionar como contribuidor, pois querendo ou não, isso é colocar a mão na massa!

@lucas-eduardo
Copy link

@filipedeschamps @OtavioCapila

Estou em contato com o pessoal do next js e perguntei sobre o custom server ou alguma forma de colocar de modo global os middlewares...obtive a seguinte resposta:

Hi there

We don't have global middlewares because those are bad, we don't like the side effects of that, and they also don't work for serverless functions, every function is independant, same for static pages

And having a custom server is worse so try to avoid that

let's say you're using API routes for your API

if you want to add CORS to them

go to every API route and add it

there's nothing more to it

Ainda estou em contato com eles, pois fiz a implementação como esta na documentação deles do micro-cors e o mesmo não funcionou quando é gerado o build, só funcionando com o npm run dev...obtendo novidades, trago aqui galera.

@lucianopf
Copy link
Member Author

Li bastante das docs do Next e realmente parece que a forma que vc fez é a certa @lucas-eduardo !!

Lendo um pouco a lib do micro-cors da pra entender bem que ele na verdade é só uma high order function que injeta os headers no nosso futuro response 😬

Lí tbm sobre o custom server, mas acredito que não é necessário adicionar tanta complexidade só pra ter um comportamento global que podemos sempre ter importando o arquivo que vc gerou de config 😬

@filipedeschamps
Copy link
Member

E pra testar isso turma? Como vamos fazer? 😂 Acho que não vamos ter como escapar de fazer um E2E nervoso usando a URL de preview deles.

@lucianopf
Copy link
Member Author

Então, da pra fazer integration tbm, o que a lib faz é simplesmente injetar uns headers no primeiro parâmetro da função, a gnt consegue mockar essas chamadas passando uns objetos direto pra função como req e res 🤔

(Mas de qualquer forma nao descarto e2e tbm não hahaha)

@filipedeschamps
Copy link
Member

Tudo isso já foi feito, inclusive consegue ser testado e garantido pelos testes E2E desse repositório aqui: https://github.com/filipedeschamps/BrasilAPI/pull/29

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

No branches or pull requests

4 participants
@filipedeschamps @lucianopf @lucas-eduardo and others