diegogamav / 5e-resource-tracker Goto Github PK

Fez o uso correto do hook!

Acho que seria interessante encontrar uma forma de suportar personagens multiclasse, onde o usuário escolheria o nível em cada classe do seu personagem e poderia ver suas habilidades todas juntas.

Tenta deixar mais claro como vai funcionar o elemento com mais interatividade na página. Bem como precisa avançar mais no HTML.

Eu não sei se teve um motivo específico para essa organização mas achei confuso a forma como tu separasse em pastas no frontend. Primeiro, não sei se era a melhor forma separar em /html e /javascript, ao menos a maioria dos projetos react não seguem essa estrutura. Segundo, dentro de /frontend/javascript possui um package.json que eu acho que não deveria estar ali (principalmente porque só possui react como dependência), talvez o melhor fosse em /javascript já fosse o código em react (no caso o que está dentro de /javascript/5e-resource-tracker). No final das contas, organização de pastas vai muito de pessoa pra pessoa mas acho que existem alguns padrões legais que facilitam a organização dos projetos :)

Vi que para checar se o ambiente é de produção ou de desenvolvimento está usando a função app.get('env'), que checa na NODE_ENV. Fiquei na dúvida se pretende mudar essa variável quando precisar configurá-la, mas de qualquer forma, recomenderaria utilizar um arquivo .env para mudar o ambiente, também porque pode ser útil quando chegar a hora de configurar o banco de dados. Dotenv pode ser uma boa.

Também queria sugerir botar um .env.example caso acate a primeira sugestão, assim os revisores podem rodar o código.

Talvez ter uma área dedicada a quem é novato no D&D e quer aprender mais. A ideia é deixar alguns links com materiais básicos ou até um breve texto explicando diferença de versões e coisas do tipo.

React.useEffect(() => {
   fetch("http://localhost:3001/classes/?id=0", {
     method: "GET",
     headers: {
       "Content-Type": "application/json",
     },
   })
     .then((res) => res.json())
     .then((response) => {
       setClassInfo(response[0]);
     })
     .catch((error) => console.log(error));
 }, [classInfo.id]);

Não entendi muito bem porque foi usado classInfo.id como dependência já que não é usado dentro do effect, acredito que poderia ter deixado as deps vazias por enquanto.

Porém analisando pela requisição, acredito que no futuro queria usar o id para fazer a requisição certo?
fetch("http://localhost:3001/classes/?id=${classInfo.id}

Dado isso, tomaria cuidado com a dependência que está colocando. Pois você usa o classInfo que é mudado dentro do effect. O ideal seria usar algum ID que você receberia por props invés do ID do objeto que é salvado pra salvar a resposta.

Também daria uma sugestão de melhorar essa chamada pra ficar mais claro que está sendo feito! É mais uma opinião minha, não se sinta obrigado a implementar:

React.useEffect(() => {
   async function fetchClassById(id) {
     const response = await fetch(`http://localhost:3001/classes/?id=${id}`)
     if (!response.ok) {
         console.error(error)
         return; // pode lançar exceção tbm ou exibir algum componente de erro
     }
     const classesInfo = await response.json()
     setClassInfo(classesInfo[0])
   }
  
   fetchClassById(id)
   }, [id]);

Olá 👋

Notei que para checar se algo está undefined, você faz da seguinte forma:

(if variavel === undefined) ou (typeof variavel === "undefined")

Um jeito mais direto e mais "javascript", como também cobre mais casos de valores que podem ser inválidos, é assim:

if(!variavel) {...}

// ou se quer checar se ela não é "inválida":

if(variavel) {...}

Fica então uma sugestão pra usar essa sintaxe se preferir

Configurar o uso de Context na aplicação

Tenho como sugestão colocar essa função dentro de um effect, por dois motivos:

1. Ela só deveria re-rodar quando o classAbilities mudasse, mas do jeito que está ela sempre roda quando acontece um render. No momento não tem muitos problemas porque seu componente não faz muita coisa, mas no futuro poderia chegar a ser problemático
2. O abilitiesLevel causa uma mudança no componente

useEffect(() => {
   separateAbilitiesByLevel(classInfo.abilities)
}, [classInfo.abilities, separateAbilitiesByLevel])

OBS: Existe alguns outros hooks mais ideias pra esse caso, como o useMemo, caso tenha curiosidade.

Seria legal criar um arquivo de useContext (até dentro da pasta utils) para realizar a propagação de estados na aplicação