search

coskuntekin / bike-table-application

Nextjs demonstration project
0 stars 0 forks source link

Feedback #1

Open eknkc opened 5 months ago

eknkc commented 5 months ago

Biraz nitpick yapiyorum feedback olmasi icin :)

Iteration key olarak randomNumber kullanmak mantikli olabilir ama burada tum component icin tek number uretildiginden birden fazla bos item varsa ayni key ile table a ekleniyorlar. Bu durumda belki array index i falan kullanmak daha mantikli olabilir.

https://github.com/coskuntekin/bike-table-application/blob/7395be9e01b379807fc6603fd9d0a3a0653859cb/src/app/components/table.tsx#L35 https://github.com/coskuntekin/bike-table-application/blob/7395be9e01b379807fc6603fd9d0a3a0653859cb/src/app/components/table.tsx#L137

selectedBike in varligi zaten modal in gosterilmesi icin yeterli bilgi. Ekstra isModalVisible state e gerek yok.

https://github.com/coskuntekin/bike-table-application/blob/7395be9e01b379807fc6603fd9d0a3a0653859cb/src/app/components/table.tsx#L19-L20

Skeleton sayfa gecislerinde load edildiginde tum table i kapatmiyor, garip bir kayma oluyordu. Bu arada onemli degil ama skeleton da componentlari n kere repeat etmek yerine loop ile olusturmak daha mantikli. Final bundle da bunlarin her biri tekrar tekrar jsx constructor a gidecek. Bosuna js buyutuyor.

https://github.com/coskuntekin/bike-table-application/blob/7395be9e01b379807fc6603fd9d0a3a0653859cb/src/app/components/skeleton.tsx#L3

useSearchParams daha mantikli. Burada bir decoupling sorunu var, page.tsx de searchParams dan gelen parametreler buraya prop olarak passleniyor. Burada querystring e pushlaniyor o tekrar render demek ama bir tarafta next in querystring parser i bir tarafta URLSearchParams olunca birbirini tutmayip loop a girme ihtimali de var.

https://github.com/coskuntekin/bike-table-application/blob/7395be9e01b379807fc6603fd9d0a3a0653859cb/src/app/components/search.tsx#L17

Bura serverda calisiyor, timer tick de response u update ediyor ama client a bunu yansitacak bir durum yok. Tick bitince serverda bosuna datayi yeniden cekip kenara yaziyor ama client i update etmiyor. Buna "use client" bir component lazim.

https://github.com/coskuntekin/bike-table-application/blob/7395be9e01b379807fc6603fd9d0a3a0653859cb/src/app/page.tsx#L23-L30

Bir cesit retry logic olsa super olur. tanstack-query tarzi bir sey kullanarak otomatik retry yaptirmak mumkun (client side).

https://github.com/coskuntekin/bike-table-application/blob/7395be9e01b379807fc6603fd9d0a3a0653859cb/src/app/lib/observe.ts#L13-L15

coskuntekin commented 5 months ago

Geri bildirimler icin çok teşekkür ederim. ☺️