Let's Turn Beginner React Code Into Clean Code
Vložit
- čas přidán 12. 06. 2024
- Let's turn a beginner-level React component into a more advanced component. Of course, this is just my way of doing things! I think the refactored version is more readable and enjoyable to work on as your project grows.
BIG thanks to Rahul and everyone who sent in their repo to make it possible for us all to learn from!!
This component on Github: github.com/HLrahul/Cbill-fron...
-- my links
Discord: / discord
My GitHub: github.com/joschan21 - Věda a technologie
Adding eslint and some basic rules is one of the best things a beginner can do to improve
Just having eslint extension also makes it so much better
the extra strain it puts tho in the pc
@@8koi245wut
And the best thing someone reviewing code can do. Just set up ESLint to error bunch of important rules in CI and you will save yourself a lot of time while reviewing code.
@@8koi245 you gotta have the worst computer for it to slow down your computer at all
You added several new dependencies for the little functionality there was. Not sure if I'd call that a win unless the entire application is refactored (now or in the future) to make optimal use of those libraries. Maybe an idea for future videos like this, do a refactor without additional libraries. Then, take it a step further and use the libraries.
As far as refactoring goes, you really just rewrote everything, altering functionality in the process:
- toasts
- min/max username/password characters requirements (this could have consequences throughout the app as there were no such requirements before). Preventing empty fields could just be done by adding required to the input.
Regarding putting the login path in a config file, seems unnecessary to me as this doesn't typically ever change and is usually only used in one place, the login page.
I'm sorry if I sound nit-picky, but I've seen people refactor things in this way, introducing problems and bugs where there previously were none because they started changing things in the same way you did.
I mostly commented because you said refactoring, refactoring means improving the code without changing functionality and that wasn't the case here. Not sure if you used the term as a mistake, just felt I had to point this out as it might teach people the wrong things.
Just planned to write a similar comment about the term "refactoring" :)
I completely agree with you. If this is "refactoring", then I assume that the business logic and UI are provided by the designer
I saw in this video:
1) Renaming variables and comments (it's funny that the author at the beginning removes comments saying that they are not needed, and then adds it to dispatch saying that here it would be possible to write :D)
2) Renaming the UI text (you can't do that if you assume that the text has already been approved)
3) Breaking the scrolling logic (you can't do that if such a UX is approved by the designer)
4) Complains about the response from the data.access server (I believe that this is an approved contact from BE. You can't just take it and rename it if you are FE, because it will just break if you are not supported by BE)
5) adding new dependencies to the project
6) Rewriting to TS and complained that in JS you can't catch errors of code before launch
7) added validation just like that, well, because author wanted to validate (you can't do that if we believe that validation was not originally intended)
If tests were written on this original code, they would all be broken
I would call this video "how would I write a form"
Agree, only a few things he did were actual improvements. Mainly deleting comments and renaming stuff
completely agree, let me just import 10 new libraries, that should teach the noobs how to code!
I think he a junior this guy
Very informative video. Learning from other people's mistakes is a lot less damaging to both our businesses and our ego than having to learn from our own. More of these please!
Great video Josh! Need more such code review videos from you, because your content is really helpful and truly amazing! Thank you very much XD
There's a potential bug in this refactor (and the original): The onSuccess runs asynchronously and uses getValues(). The user might modify or delete the input field value if the login handler takes a long time to run, therefore the user will be logged in with the wrong username. Instead you want to get the username from the onSuccess parameters, which provide you access to the parameters that were used to invoke the mutation. This way the username will always be correct!
There's another bug as well as the password validation message doesn't match the actual parameter (8 versus 3).
This is very informative! Please make this into a series!👍
Thanks for creating this kind of videos Josh, I get to learn new things every time I watch your videos :D
Happy to hear that!
You need to do more of these videos! This was amazing. Never even thought about combining react-hook-form and usemutation together! You should do a video going into more detail on that
I didn't really know the importance of react-hook-form and zod but now that I saw the combination of those two and how amazing it is I'm going to be start using. Also I used to use react-toastify which is good but the toast from react-hot-toast is much cleaner (imo) so thanks for this really amazing educational video.
Would love to seem more of these videos in the future. Great stuff as always!
It reminds me a lot of the source codes I received at the time I was still buying templates in the envato. Thanks, Josh!
Thanks Josh! You have inspired me to create tutorials which I'm planning to do sooner.
Someone else also pointed out - Maybe an idea for future videos like this, do a refactor without additional libraries. Then, take it a step further and use the libraries.
I second this I think the best advice for that beginner would've been to show them form validation & good code structure from scratch without libraries, then use these libraries show them how they make things easier and what are the tradeoffs (controlled vs uncontrolled forms). Then they could try themselves to take it a step even further & create a Form store or HOCs and do everything through a custom component.
Another very helpful comment pointed out that in an actual project the UI/UX, data from backend would be already decided and approved, and if this is how they decided it your job is to make it clean and maintainable without changing those things, something to keep it mind which is why doing a few things from scratch first helps a lot in learning
Please do more of this! I learned alot in a single video.
Great assessment, can we have more code review videos like this.
Just subscribed you. I love the way you make in-depth videos
I think this type of content is good to add to the rotation😊
Great video Josh.
It is very informative video. I learned tons of information from this. I hope more code review videos from you.
Cheers, Josh! I learned at lot here and would appreciate similar videos in the future, perhaps one highlighting Typescript and / or security improvements
Loved the video. Pls make a series
Great video! I'm relatively new to react and this peer-review style video is EXTREMELY helpful. Would love to see more videos like this :))
But for a question -
In a project that is using redux, what considerations would you account for while choosing between rtk query and react query? Are there any major functionalities/benefits/performance gains for one over the other?
Don't mind me, I am commenting here because I want the answer aswell!
I really love this type of video.
Thank you very much. Here’s something I’m sure the whole community would love can we stimulate from beginning to end a production ready next js apllication that is feature rich, I’m sure I’m not the only one that gets stuck when asked questions about production.
Love it. Would like to see more videos like that
Learned a lot! I saw my repo at the start as well. Thanks for remembering me 😁 🙌✨
Ps: the todo one
Of course man, cheers
great inputs and video, thanks!
im 30 secs into the review and i already know it's gonna be a dank video 😂
AWESOME CONTENT DUDE!!!!!!!
Never add a max-char validation to a password. That is just counterproductive.
Pretty sure it’s normal to have one around 60-70 chars
Question, can you use Zod with standard html forms? I’m not a fan of adding unnecessary dependencies.
I believe that merely learning the syntax without gaining a deep understanding of how things work is a detrimental practice for beginners. Your advice is beneficial for experienced programmers; however, I think that writing the entire code and using as few tools and libraries as possible provides beginners with more knowledge. Simply copying and pasting boilerplate code without thinking may hinder their learning process.
Good point. It's always a split between the complexity of the video and the detail of implementation. This is what I would personally do in my own projects, so I share it here. Of course, you can build these yourself. But using these battle-tested and well-documented tools makes much more sense imo. If a video gets too detailed, most people will not listen because it takes too much focus. Building these from scratch once, not for productivity reasons, but simply for learning as you said, is a great idea.
I absolutely demand you to continue this type of videos. Next time I will definitely submit my project and let you check it out!
Also, try not to say "Why the hell...?" or "Why would it be...?", because it is a bit rude.
Understandable, of course never meant that way. Cheers man!
@@joshtriedcoding I disagree, just be yourself.
@@orvvro aha... i disagree to disagree..., because i can decide what he should do next :)
@@joshtriedcoding To be honest I like that the review is more natural, no need to try and be politically correct about everything. Yeah don't just shit on everything, but saying "what the hell is this" is totally valid and a natural thing to say. You can always say "I mean, yeah I used to do the same thing" to tone it down, but no need to sugar coat everything all the time :D
Very nice tips! The only thing that I do different is to use the combo Redux/RTK or Zustand/Tanstack Query
What about swr? Is tanstack batter than it 🤔
Yeah I would definitely use Jotai and RQ instead of Redux. The thought of what a complete ugly convoluted mess of boilerplate Redux is is making my eyebrows twitch.
@@owenwexler7214 Redux isn't what it used to be. Redux Toolkit makes thing cleaner and with less boilerplate. Redux is still popular for a reason.
It was very interesting to look at your refactoring
Is it possible to hope that you will advise something on my repository?
can you do common practice in styling using mui, my currenct project using wrapper for each component and write classed inside thes. dont using inline style, create another styled component if need for reuse. is this good practice?
Good tips. Proper naming of functions/variables may seem insignificant but is so important. Makes it so much easier to understand code by colleagues, AI assistants and yourself in the future.
I do have a complaint on package usage. For a login form, why would you bloat your JS with zod, hookform resolver, axios, react query, when without them you can have everything they're offering with the same amount of code? RHF could by itself handle the type-safety with validations, as well as provide a flag (isSubmitting) for monitoring the mutation (which could just be a native fetch).
For example, to me personally it's just nicer to work with RHF + yup/zod and axios. And you mention "for a login form...", but let's be real in an app you will never have just a single form, you will have many so why downgrade your experience with some homemade solution when you could use a battle tested library. I do like defining schemas in yup and then use them for react-hook-form validation because it's nicer to read and write (subjective opinion) and I can also generate a TS type from that schema and have my form data typed easily that way and I can use that type anywhere I want. And the best thing about that combo is that even junior developers can write really good forms because they don't have to go into some homemade solution that might be hard to understand and is not documented.
Also, I prefer axios because of interceptors and the ability to show upload progress (which native fetch cannot do). I mean, in theory you could always write everything from scratch, but there is no point in doing that.
I do agree with "isSubmitting" and it's something I've seen people ignore or not know about and do their custom "isFormSubmitting" useState all the time.
Josh can you please make a video that tells everything you should know to make a secure earning through freelancing. I am really starting to get demotivated because of no projects. Please tell which skills I should know in 2023 to be a top web developer.
"The monstrosity that no amount of alcohol could fix"😂😂
Awesome video
I've been using formik and Yup for validation. It's been great!! And I totally feel you about not being able to use Javascript. I'm a computer science student rn, and a friend of mine asked me to do some freelance development for him, and I straight up told him ki no, i can't do it in javascript. I'll use typescript. Take it or leave it.
Friends asking you to do something is already an instant no. Never goes well.
@@StiekemeHenk yeah true. Honestly, I usually say no out of principle yk, but sometimes you need the extra $
@@CrankyBarbar1an feel ya, had to make aa couple of projects for extra cash because payday is 1 week away T.T
What would you prefer between PHP and Next.js ???
Msg if you look at glance you will say it a food taste and not message
You did signup functionality in a login component???
Interesting video, I am unsure as to why you are setting min max messages for a user login. I have never seen that done, if anything the only time I have put a restriction on the username field is if it is to ensure that it is an email address otherwise it just seems redundant. if it was a register page sure why not.
thanks god i found your channel.
I'm waiting for the next roast!
This was great and it's what I do, but I admit I feel it does make things more complicated, i.e your approach (and mine) means you have to know how to use react-hook-form in-depth...I wouldn't consider it simple by any stretch, like conditional validation, triggering validation, watching inputs, using it with Selects, date/time etc. There you go, a video idea, react-hook-form, but more complicated examples.
what was the utility of typescript in all this exactly?
Not losing my sanity
@@joshtriedcoding hah I still don't get why, it was a honest question, because I never had a type related error in 6 years, so I'm trying to understand what am I missing each time I see it mentioned as a game changer
@@johndowland4623 bro had an awful answer for you here fr.
I feel like if anything, TS has proved to be a lot more painful than JS in some instances for me. Especially when I get some warning that looks horribly scary at first glance like "Type '(e: React.FormEvent) => Promise' is not assignable to type 'FormEventHandler'.
Types of parameters 'e' and 'event' are incompatible."
I think in the long run though, since TS will ultimately get transpiled into JS, it's more so a means for you to be confident that you understand the data being thrown around in the code, avoid type mismatches, and avoid unwanted type coercion while actively coding, especially on larger projects where it's harder to be sure what will happen when you change something in 1 file.
Seems like you just added complexity in order to get static typing?
I was starting tô developer onde Idea. But now Im waiting for the Josh Stak. When Will we try thia?
More videos like these!
what's the reason you use "type" instead of "interface"?
Will not be better to use nullish coalescing operator `??` instead of OR `||` (e.g., in `location.state?.from?.pathname || '/dashboard'`)? Great vid, thank you!
If you use the nullish coalescence, wouldn't you get redirected to the homepage if you came from there? Because an empty string would not be included in there, whereas with the or-operator it would
Will you do these again?
Do i have to send my code on discord?? Btw awesome video, thnx so much
I observed that React Query said that the usage of onSuccess will be removed. What would you use instead of this example?
iirc, they removed the onSuccess for useQuery and useQueries only
the onSuccess on useMutation will still be there
@@dynatle5450 thanks a lot!
I WISH I KNEW ABOUT THIS BEFORE
MY CODE SUCKS SO HARD AND I ALWAYS MAKE SOME HACKS
How do i sent my repo?
Why use React Query and not RTK Query since he's already using RTK?
Your password validation gives an error message for 8 characters, but the code only specifies "min(3, ..)", probably a copy & paste error from the username validation above. This proves that the old code was better 🤓
good catch. Most apps I know enforce a password with at least 8 characters, probably better than 3
akshually lol
"Nobody knows what the hell aT is". Funny enough I used that as a variable name for a url param and knew exactly what he meant.
Now I'll turn it into Solid.js code ;)
why?
Why would you tell in the login form how the password has to look like? This makes guessing or brute forcing passwords easier.
I wouldn't say that adding typescript and form library is a fix of mistakes. Those are tools, the same as styled-components. By shoving in ts you didn't improve code quality, you switch language practically for almost no reason, because for safety you could just have jsdoc comments.
Oh, and about form lib: you don't usually need it if you just have 1 form. Yes, it's good, declarative, many usable features, but for one form you can keep it simple.
Watched to the end of the video for entertainment purposes
and for such component the fetching fucntion could be easily extracted to a custom useLogin hook, which could make it even more clean!
great point, abstracting react query into a hook is super easy
should we do this for every query we make? or just those we use in multiple components?
@@imsuvasco7922 I'd say for those used in multiple perhaps u want to extract it, for one time use it's really up to u, if u feel like component gets dirty with this big query go ahead and put it in the file next to that
that useRef comment made me lol
Videos like this with SvelteKit!
Autofocus may be problematic in terms of accessibility
One tip, you don’t want to say the minimum let alone maximum length of a username let alone a password. It makes it easier to realize how to brute force it and even know how many iterations it users. Login errors need to be ambiguous: “login failed! Wrong username or password.”
You can enforce the fact that they both need to be non-blank but that’s it.
Also variable names, I don’t give a hoot. I am so old school that we tracked 16 but addressed and registers for variables. A good programmer can immediately see where what is used - say you did.
aT was more than clear to me.
And I find that for something that isn’t even OAuth that there are far too many libraries used, it makes it very heavy for something as trivial as two fields, that just need to validate if the values aren’t empty and send it to the server and return the server response or a valid token and send that on for authorization.
But that’s my gripe with web and Python devs in general, code is too bulky and too reliant on dozens of libraries for even the simplest things. I would only use an OAuth library library, because OAuth is pain to do and to do efficiently and safely, when you do not know the protocol. So those are my two docents as a former Identity and Access manager for big enterprises.
Other than that a nice video!
naming a variable aT instead of just accessToken is objectively shite, don't even try to defend that.
@@keemkorn Indeed I would likely call it aTkn and bTKn, no way I’ll call it accessToken. Clear enough for anyone who deals with access management. And a good language and AM implementation even has two different types for those that will clear things up even more.
My point is that developers these days thing that long descriptive names make software better readable. The thing is that it doesn’t often they gets so long that typos and reading errors occur especially for those that are very similar.
Us old guys learned to juggle single or two letter variables because memory was tight. And in assembly we juggled memory address as variables, until we got ab assembler and exen there we used small names because 20 chars or so on a line with indent was about what you had to write an instruction and remark.
So I’m fine seeing vars with one or two letters I read the code and occasionally check a type (which editors now just display very conveniently in case or Zig and Rust).
@@CallousCoder damn, you really ARE a callous coder. I hope I never have to work with someone like you in my life.
Never work under the assumption that someone who’ll look at your code will have the same amount of knowledge as you, that’s just unfair. accessToken is just fine; something like superSecretAccesTokenUsedForLoggingIn is not.
And all that “back in my day, we ate the leather off our boots” junk is so tired. Try to get away from that mindset bro, we aren’t coding in Perl anymore
Validation must be done on backend and frontend
At 2:42 you are saying that the window.scrollTo is a bad idea, but you don't explain why this piece of code is here for.
I consider this piece of code as valid but surely it is not in the right place.
When dealing with mobile devices, the adress bar of the browser can be really annoying if you are making a fullscreen page. Content at the bottom will be hidden if using 100vh as on mobile the viewport is sized without substracting the address bar height.
You have solutions to this, for example :
- Use `height: 100vh` on the body then use window.scrollTo to get rid of the address bar.
- Use `height: 100%` everywhere but you keep the address bar all the time (This one is mostly used everywhere)
However since end of May 2023 a new CSS unit is supported by latest versions of browsers on mobile (also on PC obv) : `dvh` solving this problem.
BUT. As it is not supported by older versions of browsers, it's not recommended to use it for now.
Nice one bruh
thanks man!
React-Hook-Form has validation, that's the main thing it does. I don't get why you need Zod in there.
So you only have to write the validation once. Outsourced to a separate file, you can use the validation for both server and client and only need to change it in one place when the requirements change.
@@ChristianKolbow Cool, but you can (and should) also outsource your validation and business logic into separated files using React-Hook-Form alone.
Can't speak about the server part, but since they are only functions and interfaces I asume it would also work?
Nice video for beginners. But I would not add validation to login form. There is no point for it. It is up to user to remember his credentials
Seeing else statements really scares me ever since i stopped using them years ago
Importing all of that for validating only two inputs does not seem great either
Thanks man! Learnt many things! By the way really embarrassing!
If I embarrassed you, then I apologise. You’ve done an awesome job man. Really appreciate the code you wrote and how much there is to learn from this. You can be very proud of your work.
@@joshtriedcoding 😉
When a client pays $30 an hour or $60 an hour.
should store the access token in a httpOnly cookie, not application memory
damn it I don't have discord to send any
As expected from you, Very precise review brother,
just some recommendatios,
adding too many external libraries for something that seems like a personal project isn't necessary. It basically adds a lot to your bundle size on top of a heavy library like react, and if you are a beginner and don't feel the need to use Typescript, native HTML form validations works fine.
Yeah, this definitely adds some complexity. This is how I‘d personally do this, and this also goes for any form in the entire app, not just this. Chances are you’re gonna have a bunch, so once you get the hang this approach pays big dividends
I'd greatly appreciate the opportunity to look at your modified code as a whole. While I know I can skip around your video, this is rather tedious and I may not even find what I'm looking for. Having the original code repo alone doesn't really help that much. :/
4:49 Why is the app yelling at me?
video is great!
but would be better to express less anger :D
yeah it seems I might have sounded a bit judgemental at some points. That is the exact opposite of what I want to communicate here. This dude did a great job that we can all learn from. There is no wrong code, so don't worry about it while writing. You can always go ahead and refactor later.
@@joshtriedcoding ahahah
f*ck it
you are a rock star!
You added too many dependencies for what the code was doing IMO
Hi Josh!!!!
waddup man appreciate your regular comments
@@joshtriedcoding waddup Josh appreciate your regular Cool videos! 🔥🔥
"I hate JavaScript" 😂😂😂👌👌❤️
Hey Josh,
I personally think NextJS is making things unclean day by day, it should not be the case, using that "use client" keyword or "use server" in middle, also we don't have any choice of creating dynamic routes without that folder convention, what you think let us know. TIA
Yeah the "use client" has been criticized for being a bit misleading. After all, your html is still prerendered - that's why seeing the page even works with javascript disabled. It's only hydrated on the client afterwards for the interactivity. Besides that and the caching issue with the app router I really like the new changes
Great vid, roast more people please :) learned a lot
Bro we need a instagram clonè
Limiting passwords to 20 characters is a bad design aswell. Password fields should accept at least 64 characters and all types of ASCII chars to avoid conflicts with generated secure passwords from password managers like KeePass
i just copy pasted this down and said this. yeah passwords generally tend to be limited to around 64 chars
3:34
Great video! However, If I was a bambi react developer Im not really sure If I could understand any of this.
The fact that this dude's code is in JS and you explain it in TS is a thing. Adding all that extra libraries for two inputs is kinda unnecessary. A simple validate function or a required attribute (for this project) would have been enough. For futures videos like this which I will watch for sure I would suggest going easy mode at first but still like a pro. Then like an "extra" you could take it to another level and write all kind of stuff.
I tried doing my fist hackaton a help some new devs, they didn't knew react and had fun however holy fuck I hate JS lmao
I don’t know, man. This component is still doing and knowing about too much. Classic react spaghetti. Why react-hook-form and zod? There are literally two inputs; browsers APIs can handle it without wasting kilobytes. Instead of zod, you could write your own validation functions they are pretty straightforward. Is react-query used only for the isLoading property? I mean, it’s not using the library’s feature - cache. It’s not updating its entities, nothing, just firing the success callback to update the redux store. Is good old axios not okay for it? Also, the only way to test this code is by running pretty pricey integration tests. Bottom line, the refactored code is questionable, if not worse. Happy hacking.
Roast .my code daddy 😅 I like to roast my code
🤨
Are you German ?
Bro hating on javascript and then using typescript like it's not the same thing but with limitations
Yeah... I'm really not a fan of this video. What you're saying isn't wrong, but the attitude is just... not fine.
Also, if you don't know how to use JavaScript as well as TypeScript, that just shows your inadequacy in understanding how JSDocs work. You can achieve the same IDE assistance by simply using correct type definitions.
too damm harsh bro ,you could have instead developed his idea and not completely fry the grand
🤣