So, I’m working on the Drum Machine project and using React this time (the first time was with vanilla JS). Using only functional components (I feel more comfortable with them), I have created a Machine component, which renders nine different Pad components.
Now, I have managed to get everything to work relatively well so far, both mouse clicks and key presses, but I now have to add the “Label” or “Display” feature (display the name of the pad currently being played) - and It kind-of works with mouse clicks, but re-renders countless times (162 to be exact) whenever I press a key, and the display is incorrect.
What’s weird is, both mouse clicks and key presses call the same function (called “playSound”), so my guess is that there’s something wrong with the way the components are set, and/or with the way I use useEffect.
My aim is this - The Machine component (parent) has a state called “label” and has a “changeLabel” function which sets it. It passes down that label (ie the state) to the Display component (child) at the bottom of the page. As for the function itself, Machine passes it to Pad (a different child) via props.
The idea is that, whenever a pad is triggered (via click or key press), it calls the changeLabel function (via props) and passes to it the pad name as an argument. The parent components (Machine) then changes its state and passes the updated label to the Display component.
Now, the problem is that, while it works with mouse clicks (but renders 18 times, saw via console.log), it renders 162 times(!) with a key press, and the resulting label is always the very last one (“Snare”).
This seems a pretty straightforward setup and similar to examples I saw re useEffect. Could anyone help?
But the only difference I see between his code and mine is lines 27-28 in his code:
// extract the audio text for corresponding key, from props
const [requiredAudio] = audioClips.filter((audio) => audio.key === e.key.toLowerCase());
changeAudioText(requiredAudio.text);
While in my code, the function that handles the label (handed down via props) is simply provided the label it needs to show.
I’m sure I’m close but something is missing in my understanding of how exactly React re-renders the pads.
Yes, I understand that when a parent component updates state, it re-renders itself and all its children. That’s why it’s so difficult for me to understand why idea of passing a function to a child component is so common. Isn’t this how people do things in React?
The parent has a state (“label” in my case) whose change is dependent on data passed from the child.
The parent has a function that changes its state; passes it down to the child via props.
The child calls that function when needed, passing to it any relevant data
The function is called (in the parent), changes its state
The parent re-renders, along with all its children, including the one that called the state-changing function.
What if there’s no need for the child to re-render? In my case, all I want is to get the Pad component (specifically, the Pad instance that was clicked or whose key was pressed), to tell the Machine (parent) component which one it is, so the Machine component could tell the Display component what to display. That’s it. This is supposed to be a straight-forward setup, no?
And so, the problem here is - when I click a Pad, it does exactly what I’ve described above (calls the function passed to it via props; the Parent re-renders all its components). That’s why I see 18 logs on the console (9 pads * 2, which I guess is because of React Strict Mode(?)).
But when I pressa key, the whole thing renders 162 times – (9 pads, times 9 pads, times 2 for strict mode). Somehow the parent renders itself 9 times (each triggering 9 pads); or each pad (via the parent) triggers all the other pads. That’s why I only see the last label displayed.
It’s probably something with useEffect that I’m doing wrong. In the codepen I linked above (here), that state-changing function is called in the chid from inside useEffect. I don’t get why.
On a separate note, you should never be doing something like:
const pad = document.querySelector(`#${padId}`);
I appreciate that. I realized that React is supposed to replace such imperative statements, but I didn’t really see any other React way to select a DOM element. Do I need to map over all the Pad elements? Isn’t that redundant?
You don’t select DOM elements in React. You don’t deal with the DOM - React does that. You have the components respond to state and props and manipulate those. In rare cases. you can use refs to access things more directly. I seem to remember using a ref on this project, for handling the audio.
Thanks, will do, though I was under the impression that the basic useEffect and useState hooks would be enough for this project. I’ll look in to it, thanks.
Hey @kevinSmith , thanks for clarifying. Is that even though React is a library and allows you to mix declarative and imperative code?
And also, something I still find hard to understand on a conceptual level: isn’t directly selecting the element that you want (ie accessing the DOM directly) much more efficient than having to go through React, map over elements, having it re-render a component and all its children – that seems like so much extra work that the browser has to do, that it’s hard to see how or why React apps can be anything but far slower than traditional imperatively-coded apps.
If you win the lottery and hire the best money manager in the world, put her in charge of all your money and she says that all expenses, investments, incomes, etc. must go through her… if you start selling off assets and closing accounts and opening new ones and trading stock portfolios without telling her, how can she do her job?
If you want to micro-manage the DOM, then don’t use React. One (of several) advantages of React (and things like it) is that you don’t have to do that. You let React handle that.
Is it less efficient? Sure, maybe a little. But how important is efficiency. There is the old adage in coding that premature optimization is the root of all evil. People micro-obsess over all the wrong things.
Most apps (other than games) usually spends 99% of its time waiting for user input. Speed doesn’t even matter for most of the use. You want optimize it to save 0.001 seconds when humans don’t even notice a delay of less than 0.1 seconds? And what does that cost you? You loose a great tool that makes your code much, much more readable, easier to scale and reuse, easier to debug, etc. You’re giving up a lot of something that user may never even notice.
That’s not to say that there aren’t situations where that might become important. But they’re probably very rare.
much more efficient than having to go through React, map over elements, having it re-render a component and all its children – that seems like so much extra work that the browser has to do
In many cases, React will do as good or a better job than you deciding what to rerender and when. It doesn’t rerender every element on the screen - only the ones where props or state have changed. It doesn’t it’s checking in a virtual DOM so it doesn’t have to mess with the actual DOM until it needs to.
But if you want to manually control the DOM, go for it. But then don’t use React.
Hey @kevinSmith , thanks for the detailed response. I understand the React’s “state of mind” better now. My question was not so much about wanting to optimize everything to the maximum extent possible (we’re not talking about an anti-missile defense system or anything like that, after all).
My question was aimed more from the fact that, to me at least, React has quite a steep learning curve. To play by the React rules, I mean really have React take care of everything it could (since, to piggyback the financial advisor metaphor, it is the seasoned professional), I feel like I have to re-learn how to write software.
The thought is, “why bother with setting up such a complex system of components, basically re-train myself in the way I write software, in order to do something simple such as selecting a DOM element?” Maybe because I’m still learning React, but it is conceptually simpler to just directly select a DOM element than think about how to have React do it by the React rules. I’m sure the tradeoff is worthwhile (otherwise React wouldn’t be so popular), so I’m just sharing my thoughts in the meantime.
Regarding the re-renders, by the way, I have realized the problem - I used the useEffect hook to add event listeners in the Pad (child) element. So, when I had 9 pads, I had 9 event listeners for each key press. I moved that hook to the Machine (parent), and now I only have one event listener for the document. It seems to be working well, so no need for other hooks right now (though I’m sure it won’t be long before I have to research them).
Why learn to ride a bike when you can just walk? Or why learn to drive a car? That has a steep learning curve.
The thought is, “why bother with setting up such a complex system of components, basically re-train myself in the way I write software, in order to do something simple such as selecting a DOM element?”
Because this is an incredible powerful system that allows you to create very complex, large apps that would be very difficult to manage otherwise.
I mean, at some point you have to trust the judgement of tens of thousands of coders out that find that these libraries/frameworks are incredibly powerful and useful. Maybe we’re all wrong, but I doubt it.
Also, if your goal is to get a job, then maybe do a job search for “React” and see what you get. Search for “DOM manipulation”. Looking on linkedin, I get 98k for the former and 670 for the latter - and probably some of those are just asking to be familiar with DOM manipulation in addition to a library/framework.
This is just how the work is done now. If you want to be a baker, you have to learn how to use an industrial mixer. Sure, there might be a few jobs for artisinal shops where they mix everything in small batches by hand, but not many.
If you just want to be a hobbyist, then learn whatever you want. I usually assume that most people here are at least thinking about doing this professionally some day.
const pad = document.querySelector( #${padKey});
Yeah, except for the ReactDOM thing hooking into root of the doc, you should not be querying anything. Like I said, I think you might need a ref to access the audio element. You could do it with DOM manipulation but that would be hacky - if I saw that as sample code from a job applicant, I’d reject them based on that alone.
@kevinSmith thanks for the input. The goal is obviously to get to a professional level, and that includes using professional tools. I am not trying to disparage React, nor am I saying that I am right and everybody is wrong. I’m not saying I’m right at all - I’m just trying to understand where the benefits lie. If I can’t see them yet, that’s fine; my programs are still quite small after all (eg the drum machine). I have seen the wanted ads, and they all include some kind of JS framework, that’s for sure. That’s why I’m asking if this is just a natural phrase in learning this stuff, or whether I’m missing something altogether. I’m happy to listen to what seasoned pros have to say - that’s why I asked. It’s also my duty as a student to raise questions if I have any.
Regarding the query selector - yes, I’ve done some reading on useRef(), and it seems like that’s what I need, but there is a complication, as I am using functional child components, and these cannot be passed a “ref” prop (I get a console warning that says it won’t work). I know that there are workarounds but still need to look in to them. Will do so hopefully later today or tomorrow.
BTW, re this – Yeah, except for the ReactDOM thing hooking into root of the doc, you should not be querying anything.
What about adding and removing event listeners? I saw these are common, usually in useEffect. Aren’t these considered a direct “reaching in” to the DOM?
Right, the benefits will come later. If I were building a simple little one page app, I probably wouldn’t bother with React and redux. But as the app gets bigger and bigger, tools like those become super, super, super important. There is a big difference between baking some muffins and running a commercial bakery. The learner might wonder why the simple muffin pan and mixing bowl and small oven aren’t enough. That’s because they haven’t tried to make 1000 muffins, 400 croissants, 300 loaves of bread, and 2000 cookies a day. He’ll just have to take our word for it - he’s going to need to learn some new tools.
I have to say, I’m exploring createRef (which I understand is the way to go for functional child components), and it is creating a slew of new problems. I will create a separate thread (since this is now a separate issue), and will place the link here.
@kevinSmith OK ok, I think I got it. createRef() creates different refs each time, so I needed useRef() instead. Seems to all be ok now. Exciting!
I changed the direct DOM query to use useRef(). I even did it in the midi support feature (inside useEffect). I did “cost” me an additional iteration of the data (ie, I had to use find() to find the relevant pad I want to trigger), but there is no DOM manipulation now.
Here is the updated code, which I submitted as the solution: https://drummachine.idanlib.repl.co
Would appreciate feedback, if there is any.
I think the components folder belongs in the src folder. It’s still source code.
Looking in Machine.jsx
const padRefArr = new Array(9).fill(null).map(() => useRef(null));
This is going to create a new array on each render. I would memoize that or wrap it in a use effect with an empty dependency array.
Your keyboard support use effect is very big. In the real world, I would wrap that in a custom hook so I could get that logic out of the component. In the long run it makes testing easier, too.
const pads = sounds.map((soundObj, index) => {
Again, recreates on every render.
I would have also spread out the css materials, a file like Machine.css, Display.css, etc. Often I would put them in their own folder with the jsx, like a folder called src/components/Machine, with the folders in there. I might also have a file like Machine.hooks.js if I had specific hooks, or Machine.helpers.js for helper functions, maybe something for constants, I would put my test files associated with them all…
But still, it looks pretty good. Have fun on the next one.
Hey @kevinSmith , thanks for the comments! I really appreciate you taking the time to give feedback. I have some changes following your suggestions, as time permitted:
I think the components folder belongs in the src folder. It’s still source code.
Done
const padRefArr = new Array(9).fill(null).map(() => useRef(null));
This is going to create a new array on each render. I would memoize that or wrap it in a use effect with an empty dependency array.
I tried that, but wrapping useRef in useMemo makes React angry, as apparently it contradicts the “rules of hooks.” I get this warning. I might be able to work around this by creating a custom hook, though (haven’t experimented with that before).
Your keyboard support use effect is very big. In the real world, I would wrap that in a custom hook so I could get that logic out of the component.
It’s actually two different things in the same useEffect - keyboard support and midi support. I moved the midi support feature to its own useEffect, and added a cleanup function as well (some code duplication with the try-catch block though).
const pads = sounds.map((soundObj, index) => {
Again, recreates on every render.
Wrapped that in useMemo
Re CSS - I agree with what you say, and in general I have to find a way to organize my CSS better (not just in files but also in its classes etc). I just hate CSS so I usually try to spend the least amount of time on it (sometimes you have to, I’m sure). I’ll try to leverage at least some system in the next project.
