Feedback

[zapaiamarce]

Eze, Ahi le pegué una mirada y fuí anotando cosas que vi, hay un temita con los hooks y states (ahora lo vas a ver en mis notas), salvo eso lo veo bien de estructura y funcionalmente.

A nivel UX:

• Me parece que todo fluye y está bastante cercano al diseño propuesto. Estaría bueno que cada tanto puedas jugar con este proyecto y meterle mejoras librerías y que sea un poco tu playground para probar cosas de React. También porque va a quedar en tu perfil apx y está bueno que te quede bien cheto. Pero así como está lo veo bien!
• Agregale un mail de confirmación al final de la compra así me entero de que compré algo.

A nivel código:

• te recomiendo usar kebab case en vez de camelCase para archivos/carpetas. Eventualmente vas a tener problemas con camelCase.
• Veo que tenés una carpeta hooks con un mod hooks y otro local storage. Acá podés hacer dos cosas, o tener un archivo hooks en lib/ que tenga todos los hooks. O te haces una carpeta hooks con los hooks separados por entidad (user, auth, producs) y pones el hook de useToken ahí.
• El hook useToken no tiene mucho sentido, no interactuas nunca con el ciclo de vida del componente. Eso que hacés con los actions (setTokenAction) no es la forma más simple de interactuar con el token. Si es para jugar/probar está bien pero si no, con un par de funciones que te devuelvan el valor que tenés guardado y otras funciones para borrar etc estás super. Los hooks solo reservalos para poder interactuar con el ciclo de vida del componente y su estado.
• useGetSearch tenes un montón de estados para cada uno de los datitos, solo necesitás un state donde guardar todo el objeto del resultado de la búsqueda
• useEmail es raro. solo guarda el mail? usando useState en el componente loginSteps estás bien, para pedir el token podés hacer una función que pida el token (con email y code) y lo guarde en LS con alguna de las funciones de tu lib/local-storage (que es lo único que necesitas)
• todo lo de recibir data en ShowItem y meterlo en states no tiene mucho sentido. Acá te conviene tipar muy bien las props de este comp y usar la data que te llega asi como viene, solo usas estado del componente si querés guardarte data porque el componente va a mutar y no querés perderla, en este caso solo tenes que usar la data que te llega.
• En general hay muchisimos hooks y estados, yo lo revisaría todo en ese aspecto y reduciría los hooks y states al mínimo. Solo cuando son necesarios, lo demás con funciones clásicas lo podés resolver. La regla general sería: solo los hooks y states muy necesarios.
• Toda la capa UI la veo bastante bien menos el componente searcher que hace un montón de cosas y además hace un redirect y todo. Esta capa solo tiene que mostrar cosas y a lo sumo tener alguna pequeña lógica visual (barra de progreso o algo asi) pero nunca nunca interactuar con alguna api o hacer un redirect. Si tiene este tipo de acceso debería ir en la capa component/

Te lo devuelvo más que nada para que puedas pegarle una limpieza de hooks y states.

Es algo más de arquitectura y los hooks son complejos en si, asi que no te preocupes que lleva tiempo acostumbrarse y dominar el concepto.

Abrazo!

[EzequielVilla]

Buenas Marce! Gracias por la review!

Si, tuve problemas respecto al localStorage y la "data" de showItem cuando hacia el buildeo, me tiraba error siempre y supuse que era por el tema de que localstorage no existe en server, y la data llegaba "undefined" al componente, entonces recorde que useEffect solo se ejecuta del lado del cliente, y cuando lo meti ahi funciono. Pero claro, el manejo de localStorage se volvio una locura y lo otro tampoco nunca me cerro por tener que usar estados para data que no cambiaba, pero ahi pude arreglar ambas situaciones.

Gracias de nuevo por la review, le hago unos retoques mas y lo mando otra vez!

[zapaiamarce]

@EzequielVilla claro, en realidad ahí en el único caso donde vas a tener ese problema es si usas una función que invoque a la API directamente, desde un getStaticProps o algo así que corra en el server. Eso lo podés resolver con algo así:

function getToken() {
  return typeof window == "undefined"
    ? null
    : localStorage.getItem("auth_token");
}

En los casos que utilices SWR no vas a tener ningún problema porque internamente usan useEffect

[zapaiamarce]

Muchísimo mejor! Te voy a pedir un par de ajustes más porque te va a servir para esto y para la vida :)

Anoté algunas cosas de unos componentes pero creo que hay que pegarle una mirada general para ver si hay más hooks/states de más.

• Todos los states que usas en components/show-search pueden ser uno solo objeto. No necesitás un state pare cada dato, aunque técnicamente no está mal.

• useUpdateProfile usa un hook para exportar un state y un setter (setInputData) para hacer toda una vuelta rara, esto es totalmente innecesario. No estás usando el state para conservar un dato que el componente necesita, ni tampoco usas useEffect o algo asi. En este caso lo que podés que hacer es:

  import {updateUserData} from "lib/api";
  const onSubmitHandler = (data: any) => {
  setIsLoading(true);
  const res = await updateUserData(data);
  
  // todo esto de los states y effect de setInputData no te sirve para nada
  // setInputData(data);
  };

• Otro problema de usar los hooks como los usas es que el setState es async por lo que si haces algo como esto:

 const onSubmitEmail = (data: any) => {
    setEmail(data.email);
    setUserEmail(data.email);
    // clean the input for the code.
    reset();
  };

No tenés ninguna seguridad que setUserEmail se ejecute después de setEmail (quizás lo termina ordenando raro y lo hace antes).

• Con esto:

  // log-in-steps
  const { email, setEmail } = useSendCodeToEmail();
  const { setCode, setUserEmail } = useCode();

Mismo caso que lo del ejemplo de updateUserData() no necesitás usar useEffect y estos states para hacer todo eso, solo necesitás exportar funciones comunes que manden la data a la API. A lo sumo tenés un state interno para guardar el email antes de recibir el code y mandarlo. De nuevo, técnicamente no es incorrecto pero es tan complejo hacerlo asi que podes tener efectos indeseados por todos lados. CAda vez que haces un setState o un useEffect se dispara, React vuelve a recalcular el componente y todos los sub componentes a ver si algo cambio en consecuencia de ese useState o useEffect.

En resumen, hay un exceso de hooks y states, la mayoría de estos pueden ser funciones comunes que se disparan cuando un evento los llama (submit, click), los states dejalos solo para cuando no te queda otra. Los custom hooks tipo useUpdateProfile pensalos como "lógica reutilizable para componentes" no hace falta que dentro de este uses useState o useEffect para todo. Podés crear un custom hook que sea asi por ejemplo:

import { getToken } from "lib/api"
export function useLogin(){
  return {
    login: async (email,code) => {
      const token = await getToken(email, code)
      saveToken(token)
      // por las dudas devuelvo el token
      return token;
    }
  }
}

Que no usa state ni effect porque solo le pasa una función al componente, (eventualmente podría llegar a usar un state si tuviera data que ir acumulando o algo asi)

Una alternativa totalmente válida en este caso de ejemplo es que el componente use directamente getToken (o alguna función que te loguee) desde "lib/api"

Repito, el tema de los hooks/states es algo que lleva tiempo, no te preocupes, estamos hilando fino para que no te traiga problemas a futuro.

[zapaiamarce]

@EzequielVilla haceme los ajutes, avisame y aprobamos 🚀 🚀 🚀

[EzequielVilla]

Marceeee, gracias nuevamente por la review que hiciste! Esta ultima me sirve un poco mas todavia, estaba necesitando un empujon con el tema de los hooks porque no le estaba enganchando la vuelta (justo justo estoy leyendo articulos al respecto de como/por que/cuando usarlos), porque crei que eran como funciones auxiliares. Me estaba pasando que tantos useEffect o logica dentro de un componente estaba siendo un choclo o no se lograba entender, y meter useEffect en funciones no sirve, asi que implemente los custom hooks porque queda mas claro y expresado que hace cada cosa, aunque se ve que no lo estoy implementando del todo bien, pero todo esto me viene de 10. Voy a ir asimilando y metiendo mano en todo esto!