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 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'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.
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.
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
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 think pattern number 3 might come from certain libraries that use higher order components that take a function as a child (I think pre-hooks formik would do this). It was a way to pass shared context into the child component. Though slightly different than your example, I could see newbies getting confused
Are you actually defining ArticleEdit in the component, or selecting between ArticleEdit and another component? If it's just selecting between components that's cool. It's defining a new component that has issues.
Awesome video as usual Jack. What are your thoughts on using component functions if you don't need state and just need need to render some markup? I admit I've done this a few times thinking that it looked clean and readable, wasn't aware that it was a faux-pas until now.
I think it's setting yourself up for an issue down the road because other devs might look at that component and say, "Oh, it's a component, lemme add some hooks..." And then ... potentially ... boom.
If I put it behind a channel membership would that change your mind? I'm thinking about creating a channel membership for some small amount and some content would be behind that so that I can fund the channel with the proceeds.
Depends on their size. Probably yes it's bad practice, IMHO, because if you can group the JSX as a logical unit then it should probably just be a component.
I have seen the nested component declaration pattern in AngularJS...everything sharing $scope...the file is 1000+ lines long and really hard to read and understand. I get maybe they were trying to just do something quick because everything was a priority...well if everything is a priority then nothing is a priority.
i use Arc too, please please please how can i deactivate debugging mode when dev tools is open, right now i cant use arc when working anymore cuz almost everytime i refresh something invokes the debugger and blocks execution please help
I never saw this taught by anyone, but I created the third pattern independently by myself :) it's no worse than {[0].map(() => My App)} and you do that so you can have scope local to that piece of JSX, like {show && (() => {const x = computation(); return {x})()}
Yeah, you're right. Personally I would call x = computation() up at the front, but within a loop I can see where that might be tricky. And that would be a clue to me I should just be making another component since the logic is somewhat tricky.
And what is wrong with the third point? Does react consider a simple function created inside of component as a hook if it doesn't contain any other hooks called inside it?
In this particular case it was unnecessary, and I've seen that on several occasions. There are definitely uses for function calls in component though. Or using functions as renderProps. Though there are some pitfalls with that though, you need to control the references to functions to avoid tripping memo or dependency array references, and to do that you can use useCallback to stabilize the function reference between renders.
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.
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
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
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!
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.
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.
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!
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.
+ 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
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.
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!
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.
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!
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 ( ) }
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 ( ) }
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.
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!
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.
Ok.. Where is your react course? I’m working my hardest to be a principle engineer. I aim to master react and everything JS, I believe you’re the person I should go to.
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, 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 ! :)
Is there a good pattern for unit testing components that are not exported from a module? Because, AFAIK, you'd still be unable to unit test them with the proposed solution
If you really, really needed to hide them then you could have the components conditionally exported only if NODE_ENV isn't production. And then you can unit test the exported functions, but they are only exported when in dev or test.
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).
something that really scarme me right now if that i never seen those erros before and i could use them until this day because i know them, you know that each day i try to learn new things, and your brain mix good and wrong information, i would rather have not seen this video, what i've done
I don't think that's necessarily a bad thing. It's fun to try out stuff that potentially crashes the framework you are using to test and understand its limitations better.
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.
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.
Second Problem: If we have conditional components then moving focus from one element to next element in flow does not work on click of tab because it rerender and lost the focus. CONDITION COMPONENTS I means like selecting from drop-down and then displaying input field and in this case if drop-down is render like problem two then navigation from tab will not move focus from drop-down to next input field.
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 ( ╹▽╹ )
Because we don't identify hooks by name. There is no name associated with `useState()` just the variables we assign them to, which React doesn't know about. So all React has to go on is the call order and index. So if you call a different number of hooks between each render then what happened? Did you remove one in the middle? Or add one at the start? Or drop one at the end? It's impossible to know. So... you always have to call the same number of hooks (except for the "use" hook).
Hi Jack, your videos are such a great resource! One thing I've been struggling with is hooks using other hooks data as arguments. I don't know what the best practice is for dealing with hooks return values possibly being undefined.
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'm using 3 in cases i need switch case. For example you have multi-step form. Outer component has state with step number, then in jsx {(() => { switch (step) { case 1: return case2:... } })()} this is analogue of having long sequence of {step === 1 && } {step === 2 ....
So the basic message is: Don't use hooks inside a render function. It's one of the first things you're taught about hooks - that they're only meant be used inside a component or a custom hook, so I thought this was very well known. Function components may be just functions, but they're invoked by React.createElement(). And React.createElement(Component, props) wouldn't have existed if it was exactly the same as Component(props).
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.
I can't think of why anyone would try to make these mistakes in the first place. Maybe because my thinking is more object-oriented than functional, that I can't conceive doing it myself. Lol
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.
![DDG & G Herbo - Nosey [Official Video]](http://i.ytimg.com/vi/I9Ra0Zc2NlQ/mqdefault.jpg)
so far this is by far your best video of the year
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
Honestly
You deserve a sub after this video. It’s explained so well and isn’t over explained. It’s the perfect medium.
HI Sir l here is a senior guy who actually told us that this a good way to define components inside host components now i send you video to him
Cool, please send any articles he has posted to me in a comment. :)
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'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.
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.
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
Yeah, and in the case of render-props that's totally fine. This was just a div though. RenderProps might make a good video though.
@@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 learned so much from this video, i usually make all the three.
12:40 i replayed like 100x haha, thanks Jack!
very cool and detailed explanation 💯
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.
These are the most bizarre mistakes i've ever seen, never have I come across anything like this xO
I think pattern number 3 might come from certain libraries that use higher order components that take a function as a child (I think pre-hooks formik would do this).
It was a way to pass shared context into the child component.
Though slightly different than your example, I could see newbies getting confused
Great video as always. Super helpful
I sometimes have declared a component inside a component when it's a conditional part of a component eg. ArticleEdit
Are you actually defining ArticleEdit in the component, or selecting between ArticleEdit and another component? If it's just selecting between components that's cool. It's defining a new component that has issues.
Very good one here tyvm
great explanations! thank you a lot!
Awesome video as usual Jack. What are your thoughts on using component functions if you don't need state and just need need to render some markup?
I admit I've done this a few times thinking that it looked clean and readable, wasn't aware that it was a faux-pas until now.
I think it's setting yourself up for an issue down the road because other devs might look at that component and say, "Oh, it's a component, lemme add some hooks..." And then ... potentially ... boom.
Please Sir ! Make a full Course on Next.js with Typescript ...
We really need it and it is not available on youtube ...Pleease sir.
If I put it behind a channel membership would that change your mind? I'm thinking about creating a channel membership for some small amount and some content would be behind that so that I can fund the channel with the proceeds.
Great as usual 👍👍
Good stuff
No way people actually make the 3rd mistake. Mind blown!
Would you say it's also bad practice to have nested functions within your components that are only returning JSX?
Depends on their size. Probably yes it's bad practice, IMHO, because if you can group the JSX as a logical unit then it should probably just be a component.
I have seen the nested component declaration pattern in AngularJS...everything sharing $scope...the file is 1000+ lines long and really hard to read and understand. I get maybe they were trying to just do something quick because everything was a priority...well if everything is a priority then nothing is a priority.
Have you all met that developer that overcomplicates his code for no reason?
i use Arc too, please please please how can i deactivate debugging mode when dev tools is open, right now i cant use arc when working anymore cuz almost everytime i refresh something invokes the debugger and blocks execution
please help
I never saw this taught by anyone, but I created the third pattern independently by myself :) it's no worse than {[0].map(() => My App)} and you do that so you can have scope local to that piece of JSX, like {show && (() => {const x = computation(); return {x})()}
not advocating for it or anything, just wanted to clarify why you would do that :)
Yeah, you're right. Personally I would call x = computation() up at the front, but within a loop I can see where that might be tricky. And that would be a clue to me I should just be making another component since the logic is somewhat tricky.
And what is wrong with the third point? Does react consider a simple function created inside of component as a hook if it doesn't contain any other hooks called inside it?
In this particular case it was unnecessary, and I've seen that on several occasions. There are definitely uses for function calls in component though. Or using functions as renderProps. Though there are some pitfalls with that though, you need to control the references to functions to avoid tripping memo or dependency array references, and to do that you can use useCallback to stabilize the function reference between renders.
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.
and they will see your comment
I can relate to mistake #1 and #2, but I never thought about #3.
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
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
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!
obviously
But who EVER would use a Component as function?! 😲 And defining components inside components?! What developers are out there🤨
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.
Good to see you again 🎉
Great to be back. The holidays were cool. But rollin' videos feels real good.
This video is really helpful. Thanks for posting.Could u post a video for react redux hooks into webcomponent?
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.
I did the first thing literally today. So thanks Jack and thanks RUclips algorithm. Great timing.
First comment.. huge fan of yours 😉
Happy new years Jack. Looks like you got your ears lowerd (my grandfather way of saying haircut)
I've never seen any of these in the wild, and I'm glad, because if I had I might have thrown up in my mouth a bit 😅
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!
Luckily haven't done or come across these myself, but glad to see you spread awareness since it seems like some are taught this
you can write some complex logic with the 3rd mistake. maybe that's the only benefit
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.
There we go ....
here's Jack with another gr8 content!
Thank you Sir! 🙂
it's hard to imagine why someone would do the mistake #3, it's so crazy! ;)
:shrug: I've seen all kindsa weird stuff.
I see these implementations all the time and I have no idea, where they are taught. I have to break other developers out of using those practices
1 & 2 are relatable and understandable but for 3, I don't know why someone would do that.
+ 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
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.
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!
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.
biggest mistake here is not using TypeScript lol
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!
i did components within components, no one taught me that, i did a lot of oop before and doing that kinda of encapsulation was out of habit
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 (
)
}
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 (
)
}
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.
I think they are great. Though I do often find them hard to consume and know that I'm using them the way that the author intends.
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!
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.
Thanks for honestly reflecting your thoughts on this approach.
I will share your feedback with my team and see how we can improve from there.
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.
Always with the high quality contents you are like my own senior-dev at my job Thank you
Ok..
Where is your react course?
I’m working my hardest to be a principle engineer.
I aim to master react and everything JS, I believe you’re the person I should go to.
ruclips.net/p/PLNqp92_EXZBJs6rKouX5U8-tWJgTLaeKv
Thanks for the great video. I will check my code in regard of the second mistake. By the way. What is special with the browser you used. Its was ARC ?
Yep, Arc.
for some old lib i have to do the 2nd method. they don't have a component export
12:49 Oh my god, why?, There are people that code like that?
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, 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:)
const IconComponent = Icons.[`${iconName}`];
@@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 ! :)
@@stachbial No, because you aren't creating a new component, you are just selecting an existing component.
@@jherr Oh, I get it now - the const just points to a reference, which is not called untill in JSX. Thank You very much for Your time!
Is there a good pattern for unit testing components that are not exported from a module? Because, AFAIK, you'd still be unable to unit test them with the proposed solution
If you really, really needed to hide them then you could have the components conditionally exported only if NODE_ENV isn't production. And then you can unit test the exported functions, but they are only exported when in dev or test.
Just want to tell you that your videos are 💯!
Way to go!!
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 ;-) )
No need to apologize for your English skills mate, your English is good and perfectly understandable. 😉
@@Stoney_Eagle thks
Yes maybe I am not understanding the docs, but MUI and Bootstrap do #2 in their examples/components often
@@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).
something that really scarme me right now if that i never seen those erros before and i could use them until this day because i know them, you know that each day i try to learn new things, and your brain mix good and wrong information, i would rather have not seen this video, what i've done
I don't think that's necessarily a bad thing. It's fun to try out stuff that potentially crashes the framework you are using to test and understand its limitations better.
Hey what's the vs code extension that showed the warning about more hooks than the previous render right in the IDE?
Console Ninja
That last one can’t be real, that’s made up for content right?
Nooooope.
@@jherr 😭
I never see these three mistakes. these all feelled awkward to me.
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.
Thanks. So simple and effective
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.
@@jherr Good point!
Amazingly helpfull. Thanks.
Second Problem: If we have conditional components then moving focus from one element to next element in flow does not work on click of tab because it rerender and lost the focus.
CONDITION COMPONENTS I means like selecting from drop-down and then displaying input field and in this case if drop-down is render like problem two then navigation from tab will not move focus from drop-down to next input field.
Incredible video ! Explained very nicely!
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 ( ╹▽╹ )
Why doesn’t react allow different number of hooks to be called between each render?
Because we don't identify hooks by name. There is no name associated with `useState()` just the variables we assign them to, which React doesn't know about. So all React has to go on is the call order and index.
So if you call a different number of hooks between each render then what happened? Did you remove one in the middle? Or add one at the start? Or drop one at the end? It's impossible to know. So... you always have to call the same number of hooks (except for the "use" hook).
Hi Jack, your videos are such a great resource!
One thing I've been struggling with is hooks using other hooks data as arguments. I don't know what the best practice is for dealing with hooks return values possibly being undefined.
These videos are highly addictive I must admit 🎊 😅 🎉
I think I saw nested definitions of components somewhere in the Material UI documentation.
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.
@@jherr Thanks for your answer.
Another great video Jack! Thanks for diving into advanced topics that no one else cares to talk about.
The fact that 3 works is really cool tbh
I'm using 3 in cases i need switch case.
For example you have multi-step form.
Outer component has state with step number, then in jsx
{(() => {
switch (step) {
case 1:
return
case2:...
}
})()}
this is analogue of having long sequence of
{step === 1 && }
{step === 2 ....
very clear explanation
great video
So the basic message is: Don't use hooks inside a render function. It's one of the first things you're taught about hooks - that they're only meant be used inside a component or a custom hook, so I thought this was very well known.
Function components may be just functions, but they're invoked by React.createElement(). And React.createElement(Component, props) wouldn't have existed if it was exactly the same as Component(props).
Thanks so much! Now I understand why I blow up my react app when it gets bigger 😂
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.
Amazing Video! I can't believe I never made any of those mistakes. Hoowwwwwww!!!!!!
I can't think of why anyone would try to make these mistakes in the first place. Maybe because my thinking is more object-oriented than functional, that I can't conceive doing it myself. Lol
How is the "Rendered more hooks than during the previous render." error displayed in vscode? Is it a vscode extension?
Yes, Console Ninja. It's free.
@@jherr Thanks! This is great!
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.