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

14.3 Tanstackin lisääminen pääsivulle | Antti/add-tanstack-library-index-page #62

Merged
merged 38 commits into from
Jan 22, 2025

Conversation

anttiasmala
Copy link
Collaborator

Pääsivulle Tanstackin lisääminen. Sisältää lahjan lisäämisen ja uloskirjautumisen

…VG was taken to the credits_for_images.txt file
…tional rendering to make user experience better. Changes in index.tsx
…tional rendering to make user experience better
…. Make buttons same size and same paddings etc to look exact same
…Toast when catching error when logging out. It should be totally fine due to how Error Toasts are handled
Copy link

vercel bot commented Jan 17, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
lahjalista ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 20, 2025 4:22pm

pages/index.tsx Outdated Show resolved Hide resolved
pages/index.tsx Outdated Show resolved Hide resolved
pages/index.tsx Outdated Show resolved Hide resolved
pages/index.tsx Outdated Show resolved Hide resolved
@anttiasmala anttiasmala changed the title Antti/add-tanstack-library-index-page 14.3 Tanstackin lisääminen pääsivulle | Antti/add-tanstack-library-index-page Jan 17, 2025
… have an ansynchronous function inside onClick attribute
Comment on lines 4 to 38
/**
* A higher-order function that wraps a given function to handle both
* synchronous and asynchronous errors. Used to ensure that errors are
* caught in event handlers and other places where they would otherwise be
* uncaugh.
*
* @template A - The type of the arguments that the wrapped function accepts.
* @param {(...args: A) => Promise<void> | void} fn - The function to be
* wrapped. It can be either synchronous or asynchronous.
* @returns {(...args: A) => void} - A new function that wraps the original
* function and handles errors and returns a synchronous result.
*
* @example
* // Usage with an asynchronous function in click handler
* const handleClick = async () => { throw new Error('Asynchronous error'); };
*
* <Button onClick={errorWrapper(handleClick)}>Click me</Button>
*/
export function errorWrapper<A extends unknown[]>(
fn: (...args: A) => Promise<void> | void,
): (...args: A) => void {
return (...args: A) => {
try {
const p = fn(...args);
// Check if the function returns a promise to catch async errors
if (p instanceof Promise)
p.catch((error: unknown) => {
console.error('Error thrown asynchronously', error);
});
} catch (error) {
console.error('Error thrown synchronously', error);
}
};
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Kopioin tämän funktion Button.tsx:n sisään. Ymmärsinkö oikein? 😄

Copy link
Owner

@samuliasmala samuliasmala Jan 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Itse asiassa ajattelin että olisi kopioinut tämän funktion utils kansion johonkin tiedostoon, ja käyttänyt sitä Button-tiedostossa. Sitä voi käyttää Buttonin ulkopuolella kuten nyt teet, mutta jos käyttäisit sitä Button-komponentissa niin sitä ei tarvitsisi käyttää joka kerta kun käyttää async-funktiota onClick-handlerissä.

Eli perus-buttonissa onClick-propertyyn ei voi laittaa async-funktiota, mutta jos muuttaisi Button.tsx-tiedostoa seuraavasti niin sitten voisit poistaa errorWrapper funktion esim. index.tsx tiedostosta ja laittaa kaikkiin paikkoihin missä käytät Button-komponenttia async-funktion onClick handleriin.

import { ComponentPropsWithoutRef, MouseEvent } from 'react';
import { twMerge } from 'tailwind-merge';
import { errorWrapper } from '~utils/jokutiedosto';

type ButtonProps = ComponentPropsWithoutRef<'button'> & {
  onClick?: (
    event: MouseEvent<HTMLButtonElement, globalThis.MouseEvent>,
  ) => void | Promise<void>;
};

export function Button({ children, className, onClick, ...rest }: ButtonProps) {
  return (
    <button
      onClick={onClick === undefined ? undefined : errorWrapper(onClick)}
      className={twMerge(
        `mt-6 w-full rounded-md border border-lines bg-primary p-2 text-lg font-medium text-white disabled:bg-gray-300 disabled:text-gray-500`,
        className,
      )}
      {...rest}
    >
      {children}
    </button>
  );
}

Eli kun tekee näin, niin Button komponenttiin voi hyvillä mielin laittaa aina async-funktion eikä errorWrapper komponenttia tarvitse importata joka paikassa.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oliko muuten ComponentPropsWithoutRef<'button'> käytön syynä mitään erityistä? Mietin että olisiko tässä voinut käyttääButtonHTMLAttributes<HTMLButtonElement>?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Olisi voinut käyttää tuotakin. Suosin tuota käyttämääni siksi, ettei tarvitse muistaa tuota ButtonHTMLAttributes mikä on jokaiselle html-komponentille eri. Ja sitten lisäksi se on vähän lyhyempi. Julkaisin Notion-muistiinpanoni asiasta mitä kirjasin itseäni varten, jos kiinnostaa vilkaista.

pages/index.tsx Outdated Show resolved Hide resolved
TODO.md Outdated Show resolved Hide resolved
Copy link
Owner

@samuliasmala samuliasmala left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Muutama kommentti ja sitten taisi jäädä huomaamatta se ongelma missä edellisen käyttäjän lahjat näkyivät.

pages/index.tsx Outdated Show resolved Hide resolved
@anttiasmala
Copy link
Collaborator Author

Kommenttien lisäksi semmoinen huomio, että nyt jos kirjaudut ulos ja sen jälkeen sisään toisella käyttäjällä niin lahjalistassa näkyy edellisen käyttäjän lahjat -> Tanstack queryn cache pitää uloskirjautumisen yhteydessä tyhjentää ja varmuuden vuoksi kannattaa tyhjentää myös sisäänkirjautumisen yhteydessä. Tyhjennys pitäisi onnistua queryClient.clear()-funktiolla.

Tämä oli kanssa jäänyt huomaamatta. Erittäin hyvä löyty, kiitos! 👍

const { mutateAsync, isPending, error } = useMutation({
mutationKey: QueryKeys.LOGIN,
mutationFn: async (loginCredentials: UserLoginDetails) =>
await axios.post('/api/auth/login', loginCredentials),
onSuccess: () => router.push('/'),
onSuccess: () => {
queryClient.clear();
Copy link
Collaborator Author

@anttiasmala anttiasmala Jan 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lisäsin tyhjennysfunktion sekä kirjautumiseen että uloskirjautumiseen. Onko ok, että on molemmissa? Eipä tuo pitäisi paljoa "tehoa" viedä :P

Kirjautuessa tuo tarkistus pitää ainakin olla, koska jos käyttäjä uloskirjautuu sessionin vanhentumisen takia, ei cachea tyhjennetä

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Kannattaa olla molemmissa.

onSuccess: () => router.push('/'),
onSuccess: () => {
queryClient.clear();
router.push('/').catch((e) => console.error(e));
Copy link
Collaborator Author

@anttiasmala anttiasmala Jan 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Puhun router.push('/').catch((e) => console.error(e)); rivistä

Tuli vain tämmöinen skenaario mieleen, että mitä jos tämä käyttäjän siirtäminen paikkaan X jostain syystä ei onnistu. En tiedä onko tämä mahdollista, en keksinyt tapaa kokeilla tätä 😅 Olisiko parempi laittaa käyttäjälle jokin Toasti, esim "Uudelleenohjaus epäonnistui" kuin tuo console.error 🤔

Mietin kanssa, että pitäisikö toisessa branchissa tehdä jokin nappula joka ilmestyy jos siirto toiselle sivulle jostain syystä epäonnistuu. Nappia painamalla redirectausta voitaisiin kokeilla uudelleen 😄

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

En tiedä onko mahdollista, että siirto epäonnistuu. Jättäisin tämän näin, me ei olla D-alustassakaan potentiaalista virhetilannetta käsitelty. Aina voi F5 refreshata sivun jos meinaa jäädä jumiin :)

@anttiasmala
Copy link
Collaborator Author

Nyt pitäisi olla kaikkiin vastattu! Meikän moka, pahoittelut siitä! ✋

Copy link
Owner

@samuliasmala samuliasmala left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Valmis! Hyvää työtä taas! 💪

@anttiasmala anttiasmala merged commit 67dd887 into main Jan 22, 2025
5 checks passed
@anttiasmala anttiasmala deleted the antti/add-tanstack-library-index-page branch January 22, 2025 11:52
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

Successfully merging this pull request may close these issues.

2 participants