Turning bad React code into senior React code

Welcome to Code Review!

This is a series of videos where I review code that you send me or that I find online. I review the code as I would when I work with my clients. You will see how a senior developer looks and thinks about code in a variety of scenarios, learn about best practices and how to do things the right way, and learn how to become a better developer overall.

Enjoy!
Darius

[icoderdev]

Restructuring another Developer's code is one of the highest skill to possess. Thanks cosden solutions

[novailoveyou]

The first example has more to learn! Take the routes variable outside of the App component and put it on the same scope as App. That way we're saving ourselves a bit of performance and improving reliability. Also const variables like routes we could name ROUTES in all caps to indicate that that's something constant and within our frontend App

[vojinmilovic]

I don’t think anyone mentioned that the access token was passed through url which is as insecure as it can be

[gbeh]

Perfect, but one more thing, your custom hook is supposed to accept the api endpoint as a parameter because its actually different in both places used from the original code. That is, "survey/link/pd-.." and "survey/link/energy-.."

[hritikkumar]

Sol 1: we can also fetch the path text when it gets clicked and we define the onClick method in context and then we can set the path by string concatenation.

This will help us to get rid of the array that you just created.

[harag]

Just found your channel, and watching some of your stuff (and subbed). the useEffect actually runs twice and you should code it as such, it runs twice in DEV mode, but when live it will run once. React 18 is when I believed this changed. Because you're calling an API in the useEffect, you will also need an Abortcontroller, and a cleanup in the useEffect to abort the first run. It's a good habit to get into. So far liking your channel.

[zyph.]

Nice video that shows your thought process. I don’t agree that turning the routes into an array is making the code simpler / more readable. It makes it less flexible. Imo a better solution would be to create e.g. a AppRoute component that hides a lot of the boilerplate for you. Feels more React-ty

[amey]

A more efficient way of handling protected routes would be to create an protected route component to wrap unprotected routes.

[AnPham-uztd]

At 1:50 I completely disagree with you, in a fast pace code changing webapp, I usually see people rewriting/enhancing the root routing of the app (like adding a new login OAuth handler, add logic to track user behavior on specific route of the app with utm, etc). Rewriting it to an array and .map does make the code smaller, yet it makes the code coupling (please google the term "coupling") into your own logic, hence it may gets in other developer's way if they want to add new feature, which is bad for new feature and bad for the business.

Actually, the more time I live inside the coding world, the more I see that the best code is the most simple one !

[Gilderbrant]

7:25 I think this pattern is used to prevent double call of useEffect in the strict mode (react 18).

[Shaheer-xsos]

Man you are one of the best teacher ever, I have watched your react-hook videos and they're so good, respect you 💕

[Alxndr]

Nowadays everybody with good speech in a relaxed environment can be seniors 😂

[saqibmuhammad]

One of your well wisher and Keep it Up you are doing amazing work. I learn a lot of things from your videos and shorts, ❤

[slavenDj]

Great video. I use shortcut F2 to rename variables, saves a lot of time, and it's safer.

[cezarygrzanka]

4:10 and now... you just receive mail that some routes has to be wrapped in other component, that some of them may be exact, and for some reason to one of them you have to pass params. I think you just unnecesary complicated quite simple part od code just to reduce couple LOC 🤔

[irfansaeedkhan]

as senior developer once told me that do minimum code in jsx so i would recommend do the map outside jsx and just use the variable with complete list in jsx

[LePhenixGD]

6:20 I believe you should've used the .set() method of the .searchParams setter instead of directly assigning a new URLSearchParams object, if you wanted to assign the parameters your way you should use the .search setter instead

[tunamusic]

how can you select multiple same word and update name, which extension bro ?

[pyyrr]

code in the thumbnail is absolutely amazing

[dylanarmstrong]

As someone who uses the nextjs framework watching you go through the route code was really funny to me. Great solutions though because of dry principle. 😁