I am amazed I know that concept but don't know in so much depth that this can completely kill my app. omg I read about the same in kent c Dodds's blogs but did not know that this error can be undebuggable In large apps
I have definitely seen all 3 of these mistakes at my old job, no body seemed to know it's wrong when I told them they treated me like I was wrong so thank you for this video now I can share it with them.
This is an awesome video. I've worked in a codebase with a bunch of this "Component()" instead of "". I've always avoided to do it and thought it was a bit off, but never knew why, so I've never changed it nor talked to the person who wrote it. Now I have the technical base to do it. Thanks
another issue with nested components is that the inner component lose its state when outer component re-renders because each time it's a new reference so React treats it as a new component. So it unmounts old instance and mounts the new instance.
Your videos are pure gold Jack Herrington Sir , please keep going. If possible just write a book on React and Typescipt. You are the kind of teacher whole react community needs. simple and straight to point without jargons and over bombardment of information. Thank's onnce again !!! may god bless you
These videos are so good. Not clickbait. Legitimate issues we all see all the time / have made ourselves. Great in depth concise explanations, with real world examples not some contrived scenario that never happens. Amazing as always.
I've seen some of these mistakes in some codebases I've worked on. Git blame showed that it typically is a backend engineer dabbling in the FE and bringing along patterns they are used to. Great video and happy new year!
There are some mistakes that we know should be avoided no matter what but we don't really know why. This kind of videos with your gorgeous explanation let us go deeper and understand those mistakes. You're the G.O.A.T Jack!
Yes! I not only made those mistakes during my React learning curve but also now that I am a bit mature my eyes have opened and I am able to identify those mistakes during code reviews. I love your videos Mr. Herrington, very educational content.
I literally got the "Rendered more hooks than previous render" for the first time ever at work today doing something slightly different. Took me some googling to figure it out. I was rendering a component by invoking it as a function and any presence of a useState hook within the component spawned the error and took down my app. Jack made this video today because he was reading my mind, haha. I also for the life of me cannot work out why anyone would do mistake 3. I'd really love to know where they learned that from. Great video Jack, very timely!
Amazing content as always. Ive made #1 and now not only I know it is wrong but most important I know WHY it is wrong, which is what I love about this channel. Thanks my friend.
Thank you for making this video. It's one of those things in React development that I'm always finding myself un-teaching people. Now, I can just point them to this video!
Regarding mistake number 2. I do something similar to it but I am not sure if it´s falls exactly under the same umbrella (Looking forward to hear your opinion about it). But before I share the example I will tell you the "Why" we actually do this. We only do this in the Top level components (Like the root page component HomePage, EditPage, ..etc) Where in order to construct the page you have to add a lot of inputs, buttons, cards, and layout components grouped in some way or another (based on location within the page or visual hierarchy). These groups doesn´t seem to be a good use case to make a react component as they are not gonna be used outside of this page by any means. The resulting JSX of the page component will become so big in some Pages (usually the ones with many visual sections) where it is really hard to navigate through during code development or through code reviews. So What we opted for instead is the following. Instead of having the root page component rendered in this way: function MyProfileEditPage() { const state = useMyProfileEditPageViewModel() return .... Some other components needed to render the content of the homepage .... Which is gonna make this component JSX is too huge (400 - 500 lines and in some cases even bigger) } We do this instead: function MyProfileEditPage() { const state = useMyProfileEditPageViewModel() return {renderTopAppBar()} {renderBasicProfileCard()} {renderAdvancedProfileCard()} function renderTopAppBar(){ return } function renderBasicProfileCard(){ return ... some markup } function renderAdvancedProfileCard(){ return ... some markup } } What we are trying to achieve with this approach is the following. 1- Way better code reviews: The PR reader can see the top level overview of the page (How it´s structured in the exact order) and choose to navigate exactly to the section in the page where he is more interested in reviewing (using the function names renderXXX()) 2- Way easier code navigation within development: If there is a change that has to be done in a certain part in the page you can immediately tell where to go in the markup. In the example above if we happened to make changes in the Advanced Profile form and add a new inputs you can immediately navigate to it with the help of the names of these functions. 3- No more prop drilling and extra boilerplate: In the example component there is a state object. If we happened to extract these "locally defined" functions outside of the Page component and make them a React component (In the same file without exporting them because that´s the main motive which caused this code design "pattern/issue" to exist) we would either pass down the state object as-is (aka the lazy approach but less boilerplate) or pass only the properties used in these small sections of the page causing us to write the props interfaces for each one of them (A lot of boilerplate). While On the other hand. In the "locally defined stateless" functions. We get rid completely of this problem because the state object is accessible and causes no issues. 4- Finally and most importantly. Encapsulation of these (Group Components) which happened to exist only for the purpose which is constructing the different sections in a certain page. I hope in my comment that I addressed the "Why" behind this approach. But What I am honestly looking forward to hear from you is your opinion about it and what is the proposed alternatives (taking in mind the 4 points that we were trying to achieve).
I've always done this as a set of components, so there would be a TopBar, BasicProfileCard, AdvancedProfileCard, etc. components, in the same file, but maybe not exported, or exported only in dev mode for unit tests. Addressing your points: #1, not sure that really has an impact on code reviews per se. #2, components are also functions, so if navigating to functions is easy, then it's just as easy with broken out components. #3, sure there is no prop drilling, but that also means that you have one mega component that manages all the state and everything re-renders even when it doesn't need to. #4, to me components are just as good of an encapsulation. Lemme offer some downsides. First, renderAdvancedProfileCard is a function not a component function, so it can't have its own state. Second, this single function is going to get absolutely massive over time and all of the hooks will have to be at the top, it'll read like Fortran. Third, dev tooling goes out the window with this. All of the awesome work put into props, and context, memoization, minimizing re-renders, etc. is thrown out with this renderNNN() approach. Yeah, I just don't see this approach as making the best use of the framework, IMHO. It feels like you are fighting against how the framework was intended to be used. I've certainly seen render functions in the past in small doses, but I wouldn't architect around their use.
IMO, both creating new components and creating functions to decompose a big return statement are legit usecases. We use both for techniques for different reasons. I do not think any of them is wrong. Of course if things get massive, then we break down components and even create new files for them (I find this cleaner). When we need to give them their own state, again, new component is the way to go. If we want our dev-tools to treat those "fragments" as a new components, again, a new component is a way to go. Still, when our only goal is to break down a return statement into a more readable format, and not impact or complicate anything else, then function decomposing is a perfectly fine technique IMO.
@@alkismavridis1 You do you, but IMO, the intention of React is to encourage small components that are easily reused and composed into larger structures. More or less following Brad Frost's atomic model of composition. The less that you use components the less you are using the framework, and at a certain point you would be better served by a templating library because you won't be getting enough value from the 142Kb of React lib that you are dropping on the page.
You're the best, Jack! In terms of defining components within other component's module and/or function body - I saw it quite a few times but never got onto doing the same myself. Like you said, it bloats the component, makes it less performant (unless we use useCallback) and we're unable to reuse it/test it. I always stick to having these tiny functions as their own files even. I guess I started doing it way back in the beginning as it helped me understand React better.
Hi, I usually do the second "mistake" only when components need to be created with forwarding Refs (often necessary when using Material UI embedded together with your own components or other librairies). It seems to me more obvious and easier to maintain when the "forwarded ref component" is defined in the same package as the "base" component used in the forwarded one. Especially in typescript when you need to define the good types or interfaces for both "base" and "forwarded ref" components. (Sorry if my english not that clear ;-) )
@@michaelyabut5969 This. I think the biggest source of mistake #2 are the examples given by library creators - which mostly show "simple" usages. Combined with no or minimal knowledge in using React produces this "mistake". I've also encapsulated "private" components within the components myself at the beginning of my React learning path up until I fully understood how the "export" keyword is used and that I do not need to have one file per component (over-engineering).
Hi Jack. Im following you for a while now. Thanks a lot for all those awesome tipps. Im a principal web dev here in germany and I really have to stress the point that every react developer should drasticly reduce "logic" inside components in general. I highly recommand to "do nothing inside jsx"". Always go for jsx-compositons at last (codewise inside your Component definition). Just reference variable inside jsx. Even your eventHandlerFactories etc.. Just wast another well-named const to expain what going on, drop it inside jsx and get around most of the footshots. Cheers!
Woah the third mistake is thoroughly complicated by itself. Thanks for sharing these previous mistakes as well , I'm a newbie into the world of web development and It's much appreciated to share this sort of material. Thanks and greetings from ARG :)
i saw the first and second one in a tutorial on youtube building a react native app.. Use them until i ran into a bug some months back and unlearned them. Thank you for sharing this so others could see
Just watched your video on Brad's channel. You are the best example of "doing something so profesionally that you won't need marketing". I was astonished by your level of knowledge and the way you laid everything out for the viewers. As a medior react dev i still cannot believe i haven't found your channel earler. Keep making quality content which is unfortunately very rare nowadays.
I first learned React using class components, and (referring to mistake 3) is the idea of having render functions defined in the class, and calling them inside the main return(JSX). It took me a minute to get over this tendency once functional components became standard, and I'd never do that now.
This is what I was looking for. My senior is using function invoking & I don't know why. But I was pretty sure using JSX is the best way to declare components.
We have been doing mistake #2 in our project recently. We have used this pattern to replace some other things that we considered worse: - nested ”normal functions” that still return JSX - duplicated JSX elements - For example ”left wheel” and ”right wheel”, which need bunch of parent state, are tens of lines long, and their only difference is the direction. - JSX elements in a variable - not necessarily bad, but looks nicer in JSX and blends in with other custom components Of course the optimal solution would be to use context and top level independent components. But it would require a lot more refactoring (and would be more complex in some cases). Even after watching this excellent video, I feel that this pattern improved our codebase, since these mentioned problems also existed in the old pattern.
Hey, Joonas Krohn. I've found myself in that exact situation many times, too. Thankfully, it ALL disappeared when I started using "zustand" because it pulls the state out of the component, so then I can refactor the component into several small files that are so much easier to read, write, maintain, refactor… it's even beautiful. Perhaps you'd like to give it a try :D
I've never made those mistakes because at some point, someone told me to never make nested components and always use JSX as good practices. But now I know why I'm doing it this way. Pretty interesting video, keep up the good good stuff Mr. Harrington :)
I need to see all of your videos! Didnt make any of these mistakes though, but you explain well. Can you also make some advanced videos? Can you explain how to make a config file ? (do you use useContext?, do you just make a file? ) How do you manage your code? What is a “correct “ directory pattern? etc..😊
Thanks again for this channel, I did learn a lot form your videos. I have a question, concerning the first mistake. In reality I found myself invoking a component as a function but in this case because I found myself calling a switch statement to define the icon to display. In fact, the main component receives the type of info as prop (INFO, WARNING or ERROR) and inside, there is function that returns an icon component according to a switch based on this type.
That's fine, that's not a "component", that's a helper function that returns a React.createElement result. As long as your helper function doesn't use hooks, or act like a component, it's fine.
I’d say for the mistake #2 where is placed inside App.tsx, not within but together with , is quite reasonable to encapsulate re-render. Probably much proper example would be instead of that is used somewhere under the component tree. Then inside there’s an effect that subscribes to input changes that’ll only re-render instead of the whole One might say that it’d be better to create another file, but sometimes it’s quicker to just write at the same file. It can be refactored later.
I feel that for mistake #3 a lot of people continually are told to isolate anything that might be reused (heading components aren’t too uncommon) but they try doing this over-optimization stuff out the gate instead of first working towards the presentation they want, then breaking down reusable parts
I can relate to misstake where I create create functions within the function. I guess I got in to that pattern because I wanted to make the code easy to follow in the return Statement,but at the same time not create to many small files. Thanks for the video. Will definitely take this new knowledge with me.
I've seen mistake #2 happen because certain libraries make it almost impossible not to . They have a property on _their_ component that takes a component and the spec is that the component needs to change with the component state (for example, the label might be coming from a database). So in order to provide the component for the render prop for use by the library, people put the component definition inline. I think much of the time they're not even aware that is what they did, but even when you are aware, it can be a bit difficult to figure out how else to accomplish the goal.
Hey Jack, 1 place I did nest 1 component declaration inside another component was because my inner component was dependent directly to parent's prop, and I used useMemo to memoize the result so I don't hit performance problems. And the other reason was because the inner component was not used by itself anywhere so I did declare it inside my parent component.
About mistake #3. Is it the same as defining function outside the return statement and then calling it? Please consider this example: ``` return ( {(() => Example)()} ) ``` vs ``` const getHeadline = () => Example; return ( {getHeadline()} ) ``` Of course this is overcomplicating things in this particular case but sometimes this approach works really well (e.g. complex conditionals deeply nested inside the return statement - instead of chaining ternary multiple times I find it better to create IIFE and write if statements inside which I find way easier to read). I haven't noticed any difference in react-devtools components tab - it's not defining a component but a function which returns JSX. As far as I remember I've seen the second approach (defining get-jsx functions outside the return statement) taught by Kent C Dodds but I believe both of them are the same. Correct me if I'm wrong. Any thoughts are more than welcome!
It does work, that's true. And it is better than the example shown because the function is titled `getHeadline` (or often `renderHeadline`) which is helpful. And many people have commented about this if you go through the comments. Question is, why is this better than just having ``. If `getHeadline` needs state then those hooks would probably go in the parent component function and get passed as closure values, which, isn't really using React.
I really appreciate these videos, especially since its fairly difficult to find channels that dive deep in to React and its ecosystems. Besides the official docs, are there any other resources you can recommend to really, really master React similar to what this video attempts to do?
I don't know of any single canonical source. I just read a lot of articles, watch a bunch of videos, and in my own practice I tend to dig deep into why stuff fails when it fails.
Hi, thanks for the video Jack! Good examples and argumentation, however I am curious what would be the best solution for the scenario, that I have encountered today (regarding the first mistake). Statically rendered Next.js app with some headless CMS. We created an Icons file importing all (about 60) different icon components that would be used anywhere in the project and exporting them as members of one "Icons" object, i.e.: const Icons = {add: AddIcon, exit: ExitIcon}; where AddIcon and ExitIcon are the imported components. Most often called in JSX like . Let's say I have to render a particular icon based on some string (i.e. "iconName") received from CMS. My choice was writing it like this : {Icons.[`${iconName}`] && Icons.[`${iconName}`]()} Since those components do not carry any state and I am making sure the particular icon is not undefined, there should not be any serious effects of doing so. Nevertheless, after watching the video I wonder whether using the "top level API" like React.createElement would be better (I can see the benefit of controlling rerenders of the Icon component). Or maybe there is another, yet even better solution for implementing such a mechanism? Once again, thanks for the video as well as the answer in advance:)
@@jherr Thank You for such a fast answer. Yes, I can see that is a reasonable solution, however wouldn't it violate the very second rule of not creating components within components, as You mentioned in the video? Once again, thanks for the answer ! :)
In my previous team i have some seniors who used this type of code style and believe me components were huge, chunks of code in one file was a complete mess to do one simple change.
I've seen the third mistake, and judging by the context, I can guess the reasoning behind it: the overall style were components where _every_ computable value would be the output of a function. For example, instead of fullName = firstName + " " + lastName, you would have fullName = getFullName(), where the implementation of that function would be exactly as before. I observed that when functions were used more than once, they got a name, and when they weren't, they were inlined like your example. It might come from a belief that the lines are not executed unless there's a function call for them, maybe from people who are used to tracing all calls back to some 'main'. I have no proof, but what I observed was incredibly consistent in the repository.
I think this is exactly it. A lot of devs dont seem to understand Reacts component lifecycle and because they can get away with code like in the video they do.
3rd approach looks similar to `render props` pattern(look into react docs), which was popular in class components before hooks, probably people write this because its familiar? in class components you wouldn't need to care about renders because you would slap this `component` into class method, which do not get re-instanciated on every render
@@jherr ik, maybe someone just so used that put them everywhere). Good video idea indeed, i've wondered for a long time how Headless UI expose their state until i randomly stumbled upon this pattern
I've seen #1 + #2 different to your #3. Where one big component is created and contains all the state & data, smaller 'components' are created in that component scope that are just functions using data / changing state & returning JSX, that are then invoked in the big component return. I think it's to access data/state of the one big component, and not need to create a new component + file then deal with getting access to the data, state & state update functions (via prop drilling / context / a store). I'd be keen to understand how you handle handle bigger components that share data/state + update functions with their children.
+ if you put components in another components a new instance of the nested component is created every time the prop changes, bloating the browsers memory for no reason
I find the last mistake convenient I use it when needed. It comes in handy when there is some complex rendering logic, mostly when there are a lot of nested inline if statements, the condition ? condition2 ? do_this : do_that : condition3 ? : do_this : do_that. The third mistake helps me simplify such inline statements.
With all respect to my fellow coder, but might deeply nested inline if statements or nested ternary expressions be a code smell? Surely this signifies that the logic could be more clearly expressed, at least for human readability? I once heard that we don't just write code for computers, but for other humans.
Would love to see a video with nextjs13 and how to deal between server and client components when sometimes you need states after fetch fro exemple. Thanks really nice video, very helpful
When I first started learning react and have no idea what is it. People usually do that, inserting the component function in another component function. So it is like the norm in those days I guess. But I find it stupid and counterintuitive. To my surprise it still works even if move the function outside.
Actually I have already used the third one with switch case. Instead of using {something && } multiple time such as: {something && } {!something && } {something && something2 && render3/>} I think it's fine, it's my reference, because when using checking above, it's like although you already got the first render but it still check the another condition with nothing useful. so I use IIFE plus switch, for check condition then render only one that suits. If my way is bad or something wrong with it, I want to know.
So the third thing… I feel like it’s pretty common to map over something, with an anonymous closure for each element. Now, if that nested component gets complicated or particularly as as soon as it needs to deal with some internal state, then it seems necessary to factor it out, but how bad is it really otherwise? It seems like a first pass of getting the scaffolding together might often involve that, and there are probably cases where it’s unnecessary to change.
Gold content as always ♥...I had a discussion about this 1st mistake with one of my co-workers and a couple of days later, this video pops up in my feed..! One question I have though, in the 2nd mistake did you mean that EmailField should be exported to another file? because even though it was an isolated component it doesn't matter since App.jsx only exports App component, is that right?
If you want to be able to test EmailField in isolation then you can break it out into a different module, or export it as an additional named export from App. Your call. Just don't define EmailField inside the component. Thankfully this anti-pattern is now specifically called out in the new React docs.
I've seen people doing number 3 when you have a "complex" computed value that you don't want to repeat that gets used multiple times - they then pass it as an arg when calling the inlined function. Safe to say we have much better ways of doing that by using a memo or by separating into another component.
Thanks for sharing Jack! I hope I don't see people making this third mistake. It seems strange to create a self-executing function just for fun like this. Keep up the good work!
Wow, Jack, this is truly amazing. But after watching this, I want to ask if the following practice is a bad one? Imagine that in the returned JSX paragraph of code, I have something like this: {showComponent && renderComponent()}. I've seen a lot of people do that, me myself included. I'm not sure if it's better to actually memoize this renderComponent function using useCallback? I mean, clearly the return value of this function will be JSX, hence the question about useCallback being useful or not. Thank you.
There is a really good comment on this thread with someone advocating for extensive use on renderNNN() in a component as an architectural model and I give a lot of reasons why that isn't good. That being said, I think a simple renderNNN that's less than say, five lines, is probably fine.
13:25 - I've only used this with React Navigation. You can use it to add or modify props to the child component conditionally. Usually props passed are from React Navigation screen, because they instance components by string name as a prop "component='SomeComponent'" instationation... {(props) => { return ( ); }} In this case the wrapper needs navigation, before rendering the child.
Once, when I had no experience with React, I tried to write a switch case in JSX template, and it actually worked but looked a bit too dirty so I found another way to implement it. After watching your video I tried to reproduce it again "for (bad) exercise" and is something like this: {['a', 'b', 'c'].map((c) => ( setSelectedComponent(c)} /> {c} ))} {(() => { switch (selectedComponent) { case 'a': return ; case 'b': return ; case 'c': return ; } })()} What do you think about it? 😂😨
Yeah, I'd have a lookup map {a: ComponentA, b: ComponentB, ...} and then have const Comp = lookupMap[selectedComponent] up at the top and then down in the return.
10:00 There ain't really anything unclear or unexpected here. Consider this, what behavior would you expect if you first `return ` then on the next render `return ` then on the next render `return `, ... and so on? First React mounts a div, then next render it unmounts the div and creates a paragraph, then unmounts the paragraph and mounts a span. I don't think this surprises anyone. The only difference to your example is that we're switching between function components and we perceive them to have the same name and function body. But all that matters for react, the new component type !== the old component type. So unmount the old component, create and mount the new one. And this is not just a bad idea, it's broken. Besides the performance impact, these components can not hold a state, because how do you want to do that when you're unmounted and recreated on every render. It's like setting `key={Math.random()}`. 11:46 how about a different name: IIFE Immediately invoked function expressions, a very common pattern that is used in JS for decades. On this one I have to disagree with you. Just because it's a function and returns JSX doesn't make it a function component. How about calling it a render method, a partial, or a factory. What about this: `{items.map(item => ...)}` is that a function component? it takes an object as input and returns JSX ... so yes!? But I thought we're not supposed to invoke function components yet nobody criticizes mapping over an array. We can even spin this back to the first mistake. Why is the mistake here to invoke a function component? Why not, you can't use hooks in render functions? Who says that `EmailField` is a function component and not a render function, or even a hook? The dev-tools recognized it as a hook. My point here, it's hard to differentiate (and therefore more important than ever) when all your once different tools have become functions that return JSX. It all depends on how you use that function. That and something about hammer and nails... Imo. The only thing wrong with mistake 3 is the aesthetic or maybe (as in your example) unnecessary encapsulation. I'd probably also write it differently but that doesn't make it wrong.
When we need to render conditional components on the basis of more than 3 conditions, we can use the third mistake. I use it to manage different components in portrait and landscape mode. I don't feel any performance issue as well. Talking about React Native here.
I'm not saying that you should never use functions in your components. In that particular case it was unnecessary. Also there are non-function ways to switch between various components. ru-vid.comgvMpY48kf2w
And what about creating JSX and storing it as a value? I lot of time I saw code like this: const Component = () => { const header = ... const footer = .... return ( ) }
A couple of months ago I got an opportunity to work with a new startup, I was excited about the new role and moved there after one month of notice period. After a couple of weeks, it was clear to me that the startup was facing a lot of delivery issues, there were a lot of failed sprints and I was supposed to help with this, the problem is that the guy responsible for building the platform used every possible wrong practice to build the project, some like the ones in this video and your other videos about beginner mistakes in React. I saw a lot of React "crimes" that were causing a lot of the problems the startup had, I asked how and where did they learn it from and the answer was "Official Documentation" I begged the team to rebuild the project because it was just impossible to build on top of the current one which is evident by the failed sprints, impossible feature implementations, deadlines and delivery times and they will fail if this goes on, they've been building the project for one year now and it was just a couple of pages from over 50 thousand lines of code 🤯 I even did a full report about the codebase issues and offered excellent alternative solutions and talked it with the CEO and CTO. after two months, I got let go because I was too "Senior" for the job and I was told that my passion for clean code was not a good fit for the company. and now I'm jobless because of this.
Second mistake done few times. For example: - component container render a list of item component such that the first 3 items are rendered and of there are more than 1 left it will render a drop-down menu, otherwise it render the last item. - items receive some handlers cause they don't have logic - in the render of the container I have three places where I declare items and pass them the same props - what I did there, was using useCallback to define the item with the handlers inside the container. - nobody taught me that, I was just trying to DRY in the container component render Thanks for the great content and happy new year ( ╹▽╹ )
Thanks for sharing this! I had no idea about 1. I often use that paradigm if I need to render a component conditionally. So I'll make a resolveComponent() function in which I am doing something to the effect of if() {return ...} else {return ...}. What is the best practicy way of handling these conditionals?
@@doritoflake9375 Sure, but you can use a simple sub-component for that. Or just drop the logic at the start of the component and set a local variable with the component to either render, or the component rendered with createElement (through JSX). Or as the result of a memo, or ... lots of options.
