Turning bad React code into senior React code

Поділитися
Вставка
  • Опубліковано 18 лис 2024

КОМЕНТАРІ • 144

  • @novailoveyou
    @novailoveyou Рік тому +76

    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

  • @icoderdev
    @icoderdev Рік тому +47

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

    • @cosdensolutions
      @cosdensolutions  Рік тому +4

      You're most welcome ☺️

    • @kollpotato
      @kollpotato Рік тому +8

      Not always, because it's much easier to see other people's mistakes than your own.

    • @owenwexler7214
      @owenwexler7214 Рік тому +8

      Refactoring your own code from more than 6 months ago is essentially the same thing.

  • @vojinmilovic5787
    @vojinmilovic5787 Рік тому +138

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

    • @simonesquillace1624
      @simonesquillace1624 Рік тому +14

      Yea, he refactor bad react code… so yes looks good but the implementation still sucks. Honestly doesn’t look senior to me

    • @emanuellamba5828
      @emanuellamba5828 Рік тому +3

      I'm not a Web Dev but that was my first thought too

    • @weiss588
      @weiss588 Рік тому

      how should it be passed then?

    • @emanuellamba5828
      @emanuellamba5828 Рік тому

      @@weiss588 through POST calls to the server

    • @santiagobarrera2387
      @santiagobarrera2387 Рік тому

      @@weiss588An authorization header (bearer token) or cookies. And that can only be considered secure if the connection is made over HTTPS, otherwise there’s no (simple/elegant) secure way

  • @2gbeh
    @2gbeh Рік тому +60

    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-.."

    • @cosdensolutions
      @cosdensolutions  Рік тому +17

      Yes you're totally right! Realized this after I filmed the video 🥲

  • @hritikkumar2554
    @hritikkumar2554 Рік тому +5

    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.

  • @harag9
    @harag9 Рік тому +5

    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.

    • @cosdensolutions
      @cosdensolutions  Рік тому +1

      yep, you are totally correct! glad you enjoy the videos!

  • @amey4521
    @amey4521 Рік тому +5

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

    • @michaelczolko6093
      @michaelczolko6093 10 місяців тому

      100% agree, senior developers use composition instead of props in object array and later mapping with conditionals...

  • @Shaheer-xs5os
    @Shaheer-xs5os Рік тому +4

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

  • @saqibmuhammad7218
    @saqibmuhammad7218 Рік тому +1

    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
    @slavenDj Рік тому +4

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

  • @Alxndr0
    @Alxndr0 Рік тому +2

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

  • @zyph.
    @zyph. 8 місяців тому +1

    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

  • @Gilderbrant
    @Gilderbrant Рік тому +27

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

    • @cosdensolutions
      @cosdensolutions  Рік тому +9

      could be yes, but that's totally the wrong way to approach it

    • @muratcemyalin
      @muratcemyalin Рік тому +8

      you just could have memoize the function with usecallback and give it to useeffect dependency array

    • @Isaac-hd6vs
      @Isaac-hd6vs Рік тому +10

      @@cosdensolutionsyou sure? That’s how the new react doc suggests clean-ups do be done

    • @rosssharma542
      @rosssharma542 Рік тому +1

      @@cosdensolutions What's the right approach? This is the pattern suggested in the official React docs.

    • @inakiarias7465
      @inakiarias7465 Рік тому +2

      I think the point of Strict Mode running Effects twice is so that you can notice if your effects have unintended side effects if they run more than once. In this case it does, the fetch is running twice, so according to "good practices" you are making an error.
      The usual way to solve this "fetch inside useEffect" is to use the AbortController technique, where you pass a signal to the fetch call, and when you return from the effect, inside the cleanup function you abort with controller.abort.
      You can google it pretty easily, but basically each time the effect runs it will cancel the previous fetch, which should be "intended behaviour"

  • @pyyrr
    @pyyrr Рік тому

    code in the thumbnail is absolutely amazing

  • @dylanarmstrong9328
    @dylanarmstrong9328 Рік тому

    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. 😁

  • @irfansaeedkhan7242
    @irfansaeedkhan7242 3 місяці тому

    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

    • @cosdensolutions
      @cosdensolutions  3 місяці тому +1

      it's all the same tbh. Matter of preference at this point!

    • @irfansaeedkhan7242
      @irfansaeedkhan7242 3 місяці тому

      @@cosdensolutions ohh so there is no good practice bad practice in this regard, its just that how you prefer , or it looks cleaner to you

  • @irfansaeedkhan7242
    @irfansaeedkhan7242 3 місяці тому

    keep this series going sir

  • @AnPham-uz3td
    @AnPham-uz3td Рік тому +6

    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 !

    • @cosdensolutions
      @cosdensolutions  Рік тому

      Fair point, but this is a mature project and for its use cases this was an improvement. But it ultimately depends on the project. Not every project needs a complex routing structure

    • @siroccomask
      @siroccomask Рік тому +1

      @@cosdensolutions Nearly every routing file looks like the one you showed. It's already easy to read, and add a route. If I open the routing file and it has a loop in it just to reduce the lines in the file, I am not seeing how this is improving readability.

    • @cosdensolutions
      @cosdensolutions  Рік тому

      @@siroccomask it's making you not have to repeat all of the same components and auth router. If they're all the same, it's just easier to have a loop so you have one place to look at how everything is rendered

    • @m.syafaat
      @m.syafaat Рік тому +6

      ​@@cosdensolutions You seem to be misunderstanding the concept of "repetition" or you might heard of the DRY (Don't Repeat Yourself) principle. In this case, converting your component into a map doesn't actually improve your code, it could make it worse. It appears that you're introducing a new approach to rendering your routes, which doesn't necessarily align with good coding practices. If this were in my team, I would likely express my confusion with a "WTF", and then suggest, "Please, consider creating a new Route Component named AuthRoute and use it like a standard Route: .".
      Every time I see people doing something like this, I'm 100% sure that it's a sign of a junior developer.

    • @filipecovas
      @filipecovas Рік тому +1

      Completely agree, the map will increase the coupling and decrease de legibility and also some bad performance.

  • @muhammadashfaq913
    @muhammadashfaq913 Місяць тому

    Hi @Codesen
    Please create a detailed video on contributing to react-native open source and approach to solve a problem. Thanks. You are amazing.

  • @LePhenixGD
    @LePhenixGD Рік тому +4

    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

  • @sbabup8957
    @sbabup8957 Рік тому

    Never subscribed to anyone before, You are the first. Keep up the good work!! Also whats the font in your editor? I liked it alot..

    • @cosdensolutions
      @cosdensolutions  Рік тому

      damn, that means a lot! I use the menlo, monaco font. My next video is actually all about my custom vscode setup!

  • @rajendra222php
    @rajendra222php 10 місяців тому

    we need more video like this

  • @osamaakhtar6525
    @osamaakhtar6525 Рік тому +2

    4:15 Interestingly enough you could have placed the routes object outside the App component since it didn’t depend on any reactive variable.

    • @cosdensolutions
      @cosdensolutions  Рік тому

      Yeah you could've, but that's just personal preference at that point! Wouldn't affect performance in any way

    • @muratcemyalin
      @muratcemyalin Рік тому +2

      @@cosdensolutions i am using the same technic with just making a seperate routes.js file and exporting routes from there and importing to app just a little bit cleaner.

  • @tunoajohnson256
    @tunoajohnson256 Рік тому

    Good stuff! Cheers for sharing

  • @thecyberhobbit
    @thecyberhobbit Рік тому

    "Route" !== "Root" 🤣😂
    Jokes aside, nice video dude, keep it goin!

    • @thechaoslp2047
      @thechaoslp2047 2 місяці тому

      Its the correct british pronunciation

    • @thecyberhobbit
      @thecyberhobbit 2 місяці тому

      @@thechaoslp2047 right... that's why I said "jokes aside"

  • @sonhuynh570
    @sonhuynh570 Рік тому

    this process remind me the old time lmao btw great vid and keep it up bro

  • @devyb-cc
    @devyb-cc Рік тому

    i believe react router has component called RouterProvider to handle the first use case.

  • @DamilolaBada-x3x
    @DamilolaBada-x3x Рік тому

    Nice and informative video. Do you by chance have any video about organising files according to features and not types in nextjs?

    • @cosdensolutions
      @cosdensolutions  Рік тому

      I have a video on general folder structures in React, but nothing that specific unfortunately

  • @captsiro233
    @captsiro233 Рік тому +1

    Why do you have vim plugin if you are still using mouse... I don't have vim plugin and I use my mouse less.

  • @RiyazShaikh-o3v
    @RiyazShaikh-o3v Рік тому

    please make one javascript tutorial you are explaining fabulous

  • @zebapy
    @zebapy 4 місяці тому

    First thing I'd do is add and run Prettier.

  • @calefnicoft
    @calefnicoft 4 місяці тому

    Thaaanks! amazing 🥰

  • @tunamusic2314
    @tunamusic2314 4 місяці тому

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

  • @piotrzajas9014
    @piotrzajas9014 Рік тому

    4:04 - You can prolly delete the at the end of map func and make the a self closing tag?

  • @marcossequeira5433
    @marcossequeira5433 Рік тому

    It could be a better solution use an AuthGuard component and pass as childrens all the routes that you want to be protected?

    • @cosdensolutions
      @cosdensolutions  Рік тому

      depends how the component is structured. Maybe it doesn't allow this this way. But worth a try

  • @hritikkumar2554
    @hritikkumar2554 Рік тому

    And yes, you are awesome 👍

  • @groovebird812
    @groovebird812 Рік тому

    First you return generateURL, AuthToken and url and at the end end of the video only url? This was to quick for me

    • @cosdensolutions
      @cosdensolutions  Рік тому

      Yeah I thought initially we would need to return all 3, then we only needed the token so I changed it

  • @cezarygrzanka4229
    @cezarygrzanka4229 Рік тому +2

    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 🤔

    • @cosdensolutions
      @cosdensolutions  Рік тому

      Again, you can create as many hypotheticals as you want. I reviewed this for this scenario, which is valid

  • @nicolamigone8849
    @nicolamigone8849 Рік тому

    Are you going to do more of these? I'd love to have my code reviewed

    • @cosdensolutions
      @cosdensolutions  Рік тому

      Yes! Post it in the Discord there's a channel for that

  • @carlosmaralit8784
    @carlosmaralit8784 Рік тому

    this is so useful, can you also make like this with codeigniter? thanks

  • @WolfamStart
    @WolfamStart Рік тому +5

    Where are the tests, to prove you don't change the functionality?

  • @dgoenka1
    @dgoenka1 Рік тому

    1:55 how about a custom component and pass it to that?

  • @p1erceprc
    @p1erceprc Рік тому

    What theme is this? thanks

  • @aymenbachiri-yh2hd
    @aymenbachiri-yh2hd 6 місяців тому

    Thank You

  • @SacrificialGoat94
    @SacrificialGoat94 Рік тому

    Isn't sticking the access token in the search params a horrible idea?

    • @cosdensolutions
      @cosdensolutions  Рік тому

      you would usually put it in the headers of the request

    • @SacrificialGoat94
      @SacrificialGoat94 Рік тому +3

      @@cosdensolutions Really sherlock? Yet you did not mention that while creating "Senior level code" perhaps you're not quite ready to be making such claims.

  • @dhruv7827
    @dhruv7827 Рік тому

    What is that tool that is suggesting you the code? Is it an extension?

    • @harag9
      @harag9 Рік тому

      Co-Pilot. It costs per month to use, not sure how much.

  • @isaacjon
    @isaacjon Рік тому

    U said that we cant use state in a custom hook. What does that mean? U just used useEffect and useState hooks in this custom useGeneretaeUrl hook.

    • @cosdensolutions
      @cosdensolutions  Рік тому

      Hmm, where did I say that? You Def can use hooks in custom hooks!

    • @isaacjon
      @isaacjon Рік тому

      ​@@cosdensolutions9:45 can you check and explain it please

    • @samansaeedi1498
      @samansaeedi1498 Рік тому

      He says you CAN, and that's the beauty of it

    • @isaacjon
      @isaacjon Рік тому

      @@samansaeedi1498 ahh i got it. It sounded like (can't)

  • @imrahulkhatri
    @imrahulkhatri Рік тому

    Very helpful

  • @gustionusamba90
    @gustionusamba90 Рік тому

    What is the theme and font do you use?

  • @ooorayooonyoooo
    @ooorayooonyoooo Рік тому

    what is your color theme mate?

  • @Vagabondi404
    @Vagabondi404 Рік тому

    Very nice

  • @helios8567
    @helios8567 Рік тому +1

    you did the code better, but it's stil crap. You could do this code a lot more cleaner. Not a senior level imho.

  • @kirillvoloshin2065
    @kirillvoloshin2065 Рік тому

    got to propose to switch to TS

  • @navidghasemi9685
    @navidghasemi9685 Рік тому

    great that was helpful

  • @tauraamui
    @tauraamui Рік тому

    not related, but I've never seen someone using VIM or the VIM plugin on an editor but completely defeat the point by just using the mouse, it's painful to watch

    • @cosdensolutions
      @cosdensolutions  Рік тому

      Well, what can I say.. I'm sorry you feel this way 😅

  • @savire.ergheiz
    @savire.ergheiz Рік тому

    There is no such things as bad code. Any senior engineer always know that fact.
    There are only codes that carelessly being made due to many reasons.
    If you said its a bad code then how you justify those bugs that always goes into production no matter how good or big the companies behind?

  • @af2b
    @af2b Рік тому

    Amazing

  • @ask43242flight
    @ask43242flight Рік тому

    Do you have a tutorial that shows how to embed JavaScript code within HTML(i.e. JSX)? I don't have solid knowledge to know when I should use () and when I should use {}. Basically, I highly depend on VSCode to help finding issues for me while I am write JSX code. Thank you

    • @cosdensolutions
      @cosdensolutions  Рік тому +1

      Hmm not specifically, but in every video I kinda do that 😅 it just comes with time! So keep practicing

  • @icoderdev
    @icoderdev Рік тому

  • @jotasenator
    @jotasenator Рік тому

    8:04 🤕

  • @john_dren
    @john_dren Рік тому

    I found it funny how the refactoring takes you like 5 min tbut a junior like me half a day or a day :D