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.4 Tanstack DeleteModalin ja EditModaliin | Antti/add-tanstack-library-delete-and-edit-modals #63

Merged
merged 43 commits into from
Feb 18, 2025

Conversation

anttiasmala
Copy link
Collaborator

Tanstackin lisääminen DeleteModal ja EditModaliin

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 Feb 18, 2025 10:29am

Base automatically changed from antti/add-tanstack-library-index-page to main January 22, 2025 11:52
handleErrorToast(handleError(e));
}
})}
onClick={() => void mutateAsync()}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Onko tämä ok? errorWrapper on Button-komponentissa jo ja mutateAsync() on ainoa funktio joka ajetaan onClickissä joten asynciä ei pitäisi tarvita 🤔

Copy link
Owner

@samuliasmala samuliasmala Jan 25, 2025

Choose a reason for hiding this comment

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

Koska Button-komponentissa on errorWrapper ei voidia tarvita, vaan tuon voisi laittaa vain onClick={() => mutateAsync()}. Mutta jos mutateAsync voi heittää virhee (kuten se voi) niin sitten on parempi käyttää try-catch blokkia kuten teit. errorWrapper on ikään kuin varmuuden vuoksi, sinne ei tietoisesti pitäisi laittaa virheitä kuten tässä kävisi esim. requestin epäonnistuttua.

Edit: Nyt kun lisäsit errorToastit errorWrapperiin, niin sitä voi halutessaan käyttää myös muuten kuin varmuuden vuoksi.

@@ -55,7 +56,7 @@ export function EditModal({ gift, closeModal }: EditModal) {
closeModal={closeModal}
title="Muokkaa lahjaideaa:"
>
<form onSubmit={(e) => void handleEdit(e)}>
<form onSubmit={(e) => errorWrapper(async () => await handleEdit(e))}>
Copy link
Collaborator Author

@anttiasmala anttiasmala Jan 22, 2025

Choose a reason for hiding this comment

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

Oon saattanut tämän kysyä jo useasti, mutta varmistan vielä jotten ymmärtänyt väärin 😄

Onko tämmöisessä tilanteessa tarpeellista pitää awaitia? Tämä on kuitenkin ainoa asia, mitä ajetaan onSubmit-funktiossa (esim. ei hoideta virheitä)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Kyseisen rivin voinee lyhentää seuraavasti, mutta menee omasta mielestä liian vaikea lukuiseksi verrattuna tuohon mikä nyt on 😄

Suggested change
<form onSubmit={(e) => errorWrapper(async () => await handleEdit(e))}>
<form onSubmit={errorWrapper(handleEdit)}>

Copy link
Owner

@samuliasmala samuliasmala Jan 25, 2025

Choose a reason for hiding this comment

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

Joo voi lyhentää noin jos haluaa. Vähän makuasia mistä tykkää, itse pidän ihan ok:na tuota lyhennettyä versiota.

Tässä <form onSubmit={(e) => errorWrapper(async () => await handleEdit(e))}> ei tarvitse awaitia. Edellinen on sama kuin

<form onSubmit={(e) => errorWrapper(async () => { return await handleEdit(e); } )}>

Se onko edellisessä await vai ei vaikutta missä virheet käsitellään, mutta koska tuossa yllä ei ole try-catch blokkia tässä tapauksessa eroa ei ole. Kannattaa lukea tämä await vs return vs return await artikkeli, se selventää.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Artikkeli oli hyvä, kiitos siitä! 👍

Comment on lines 78 to 79
onClick={() => closeModal()}
onClick={closeModal}
Copy link
Collaborator Author

@anttiasmala anttiasmala Jan 22, 2025

Choose a reason for hiding this comment

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

Lyhensin funktion näin. Onko tämä ok? Molemmat tavat sopivat itselle. Käytän lyhyempää tapaa vain funktioissa, jotka eivät ota argumentteja vastaan.

Esimerkiksi Tallenna-Buttonissa oleva onClick voitaisiin lyhentää
tähän

onClick={handleEdit}

alkuperäisestä

onClick={(e) => void handleEdit(e)}

Mutta jokseenkin tuon e:n näkeminen / sisäistäminen helpottaa

Copy link
Owner

Choose a reason for hiding this comment

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

Kannattaa tehdä itselle sopivalla tavalla. onClick-propsi on määritetty niin, että siihen annettua funktiota kutsutaan ja parametrina on event. Koska alemmassa tuo parametri välitetaan suoraan handleEdit funktiolle ovat nuo kaksi sun kirjoittamaa tapaa identtiset. Sen sijaan jos kirjoittaa

onClick={(e) => void handleEdit()}
onClick={() => void handleEdit()}

niin nyt event e ei välity handleEdit funktiolle.

Copy link
Owner

Choose a reason for hiding this comment

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

Eli sun ensimmäinen tapa toimii jos handleEdit saa pelkän eventin parametriksi. Joskus funktio voi saada muutakin, jolloin lyhyt tapa ei toimi vaan pitää kirjoittaa esim. näin:

onClick={(e) => void handleEdit(userId, e)}

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Joo. Nopealla testauksella helpompi ja huomattavasti nopeampi tapa lukea koodia itselle on se, että jos annetaan parametreja, "pidetään ne esillä":

onClick={(e) => void handleEdit(e)}

ja jos ei anneta parametreja voi silloin lyhentää

onClick={() => closeModal()}

->

onClick={closeModal}

Kiitos! 👍

Comment on lines 50 to 58
onClick={() => void mutateAsync()}
onClick={async () => {
try {
await mutateAsync();
} catch (e) {
handleErrorToast(handleError(e));
}
}}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Laitoin try/catch-blockin takaisin. Ilman tätä sivu ei toiminut

Copy link
Owner

@samuliasmala samuliasmala Jan 25, 2025

Choose a reason for hiding this comment

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

Sivun kyllä olisi pitänyt toimia, mutta virheiden tulla vain konsoliin ennen sun muutosta errorWrapper-funktioon. Nyt kun errorToastit on errorWrapperissä, niin halutessaan tästä voi poistaa try-catch blokin ja käsitellä virheet siellä (kunhan ei laita voidia, jos sitä käyttää niin promise "hukataan" eikä virheitä käsitellä).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Oot oikeassa! Syy oli voidissa! Palautan tähän tuon vanhan tavan takaisin, tosin ilman voidia.

Eli seuraava

onClick={() => mutateAsync()}

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.

👍

@anttiasmala anttiasmala merged commit aa2a283 into main Feb 18, 2025
5 checks passed
@anttiasmala anttiasmala deleted the antti/add-tanstack-library-delete-and-edit-modals branch February 18, 2025 10:31
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