BrasilAPI / cep-promise

Busca por CEP integrado diretamente aos serviços dos Correios, ViaCEP e outros (Node.js e Browser)
MIT License
2.88k stars 316 forks source link

Adicionar função format #148

Open dmvvilela opened 4 years ago

dmvvilela commented 4 years ago

Exemplo: https://www.npmjs.com/package/cpf

Para fazer por exemplo cep.format(XXXXXXXX) e sair XX.XXX-XXX

Ia ser legal ter isso! :)

lucianopf commented 4 years ago

@dmvvilela a ideia é boa sim, apesar de ser mega simples de resolver isso no próprio client, só não sei mensurar o tamanho do benefício em relação a quebra de interface que isso vai causar na lib 🤔

Inicialmente pensando só consigo imaginar uma nova funcionalidade dessa surgindo se a gnt modificar o que exportamos do módulo pra ao invés de ser uma função ser um objeto contendo dois novos métodos, tipo:

{
  find: () => { ... },
  format: () => { ... },
}

Talvez assim a gnt consiga inclusive reutilizar as funções internas e exportar elas pra que clientes implementem novas soluções 🤔

@filipedeschamps , preciso da sua opinião aqui tendo em vista que isso implicaria em breaking changes

Função simples que já resolveria esse seu probleminha:

const formatAsCep = value => String(value).replace(/^(\d{5})(\d{0,3})(.*)/, "$1-$2")
dmvvilela commented 4 years ago

É coisa pequena mesmo, só uma idéia pra deixar a lib mais fera!

Seguindo a lib que coloquei ali, ela facilita muito por exemplo, já faz o strip do termo, retirando .,/ etc e faz a pesquisa independente da máscara.

Isso permite a gente no client utilizar qualquer máscara e pesquisar com facilidade sem ter que fazer o strip na mão.

É coisa bem pequena, mas que ajuda :)

filipedeschamps commented 4 years ago

Fala pessoal! Show essa idéia! Minha sugestão seria retornar isso no response para evitar breaking change, algo como:

cep('05010000')
  .then(console.log)

  // {
  //   "cep":  "05010000",
  //   "cepFormatted":  "05.010-000",
  //   "state":  "SP",
  //   "city":  "São Paulo",
  //   "street":  "Rua Caiubí",
  //   "neighborhood":  "Perdizes",
  // }

Um dos downsides que consigo pensar é se a pessoa só precisar formatar um cep sem querer fazer a consulta (apesar de que esse não é o use case da biblioteca).

O que acham?

vagnercardosoweb commented 4 years ago

Por padrão o viacep já vem com ele formatado e na aplicação é removido o - (hífen), acho que seria interessante a quem estiver usando a lib decidir formatar ou não, até porque a formatação pode ser diferente em alguns casos.

dansoliveira commented 4 years ago

Show!! Seguindo a linha do @vagnercardosoweb, seria interessante um campo de options. Exemplo:

cep('05010000', { format: true }).then(console.log)

E, para evitar breaking changes, poderíamos definir um valor default para este objeto de forma que continue com o funcionamento padrão.

Vejo que seria um bem mais trabalhoso de realizar a inclusão desse objeto nos services (até mesmo por conta das promises), principalmente se comparado com a solução proposta pelo ~Teló~ @filipedeschamps.

Inclusive, entre as ideias, entendo que teríamos de escolher entre:

ou

Realmente não sei até que ponto seria bom ambos os casos hahaha

PS: Estou muito animado com o rumo deste projeto. Espero contribuir e aprender muito. Está sendo a minha primeira participação num projeto Open Source 😍

SkyaTura commented 4 years ago

Eu gostaria de adicionar a discussão que existem duas possibilidades muito comuns de formatação CEP:

12.345-678 e 12345-678

filipedeschamps commented 4 years ago

Show! Decisão boa! Eu vejo a situação dessa forma (que é um cálculo de tradeoff)

Para o tradeoff acima, acho que o saldo é negativo, pois eu vejo o seguinte cenário:

Como a máscara pode ser aplicada localmente, não é necessário trafegar isso pela network inclusive, então o custo técnico é muito baixo.

Então ela, sem complicar a interface, pode retornar por padrão:

cep('05010000')
  .then(console.log)

  // {
  //   "cep":  "05010000",
  //   "cepMask1":  "05010-000",
  //   "cepMask2":  "05.010-000",
  //   "state":  "SP",
  //   "city":  "São Paulo",
  //   "street":  "Rua Caiubí",
  //   "neighborhood":  "Perdizes",
  // }

Como o @SkyaTura falou, essas são as possibilidades mais comuns e vejo que se a pessoa cair em um outro formato, ela deve estar fazendo uma coisa muito específica a aplicação dela e essa especificidade deveria ficar apenas na aplicação dela. Do contrário, não vale a pena pagar o custo técnico e de interface+documentação para atender a esse caso específico.

O que acham?

Fora isso, eu estou com um dilema sobre o BrasilAPI porque a idéia é usar o BrasilAPI como provider, mas ele utiliza internamente o cep-promise, ou seja, vai ficar em um loop. Uma das formas que vejo de evitar isso é, pela interface ou de alguma outra forma, definir quais providers ele quer consultar. Se vocês conseguirem pensar em formas de não alterar a interface e evitar esse loop, topam criar uma outra issue para discutir isso?

Abração turma!!! 👍

SkyaTura commented 4 years ago

Eu também acho essa solução mais interessante, @filipedeschamps , voto nela!

Sobre o seu dilema, eu adoraria participar dessa discussão também.

Na minha opinião, utilizar o cep-promise internamente na BrasilAPI é mais vantajoso do que utilizar a BrasilAPI como provider, pois uma das melhores características desse projeto é que ele busca em diversos fornecedores para garantir uma resposta, ao passo que se a BrasilAPI for utilizada como provedor, nós ficamos dependentes da disponibilidade dela.

lucianopf commented 4 years ago

Vai me bater se eu propor uma nova função no protótipo do cep que faz o setup dos providers usados? 😂

const cep = require('cep').config({
  providers: ['ViaCep', 'Correios'],
  masked: true,
})

É meio feio, mas não altera a interface do que já existe em nada 😬

filipedeschamps commented 4 years ago

Show @SkyaTura a idéia não é fazer o cep-promise ter o BrasilAPI como único provedor, e sim adicionar o BrasilAPI como mais um provider do cep-promise. Só que como de fato o BrasilAPI vai utilizar o cep-promise para o endpoint do cep, no fundo ele vai também fazer uma chamada para si de volta e vai endoidar num loop 😂

E @lucianopf adorei a sugestão!!!!! Realmente gostei! Eu tava com a cabeça presa na instância e não na factory! Eu só retornaria a máscara por default, mas a parte de configurar ali os providers matou a pau!!!!!

lucianopf commented 4 years ago

Na verdade a alteração da forma que comentei fica bem feia, acho melhor a sugestão do @dansoliveira mesmo 😬

Confesso que já coloquei o pé na água e ficou elegante 😬

cep(
  29090010,
  {
     providers: ['viacep']
  }
)
.then(console.log).catch(console.log)

Sendo que o segundo parâmetro é totalmente opcional 🤔 😬