5 ms·
I have very little React experience, but this seems a bit awkward to me: >Therefore every app should have an action component that could look like this: exp
by meowface 6y ago
I have very little React experience, but this seems a bit awkward to me:
>Therefore every app should have an action component that could look like this:
export const Action = ({ onClick, iconProps, buttonProps, dropdownItemProps, renderProp }) => {
if (iconProps) {
return <Icon onClick={onClick} {...iconProps} />
} else if (buttonProps) {
return <Button onClick={onClick} {...buttonProps} />
} else if (dropdownItemProps) {
return <Dropdown.Item onClick={onClick} {...dropdownItemProps} />
} else {
return renderProp({ onClick });
}
}
- mattmanser 6y agoYeah, was just looking at that and raising an eyebrow. I've only done smaller projects in React, but in 15 years of writing programs, that instinctively looks like a pretty severe code smell.
- fiddlerwoaroof 6y agoYeah, this is the sort of situation where you want to employ some sort of polymorphism rather than a static conditional, I’d think. In general, while some of the advice here could be useful, my experience has led me to very different opinions about structuring react.
- city41 6y agoReact is very flexible and tries to avoid strong opinions, so the result is React devs have built up lots of their own opinions over the years. React code bases are almost always wildly different from each other, a blessing and a curse. I personally wasn't a fan of any advice in this article. But again, it's all just opinion and personal experience. I didn't feel anything covered fell in the area of best practices.
- mattmanser 6y agoI've already commented on the GP, but that's not a good thing in my experience in the long run. Techs that quickly end up with a general "best practice" code style are a lot easier to work with, and a lot, lot, lot, lot easier to maintain.
- zdragnar 6y agoClojure and co are a similar vein of favoring small, focused practices and libraries over one-size-fits-all frameworks. YMMV, but having been on both sides of this equation at companies large and small, I find the library (I.e. react) approach favorable over the framework (I.e. angular or ember) approach.
- mattmanser 6y agoThat's got nothing to do with my comment though. At all, like whoosh, you've completely missed the point of it. My point was that it doesn't matter if it's a framework, library, language, configuration, or whatever. If the design of that thing means that the community coalesces around a particular style of usage, it's much better for everyone in the long run.
- slver 6y agoCase in point I literally only use JSX and ReactDOM.render(). Some would call this a blasphemy. Works great.
- bennyg 6y agoThis seems like a good way to accidentally turn every icon into a dropdown. It's really hard to see the benefit of this Action component instead of just rendering say an `<Icon>` where you want. So much simpler to read, and the explicitness of the render method is just in your face instead of behind a `iconProps` in the props interface of `<Action>`.
- bluedevil2k 6y agoI’ve seen people attempt to treat API calls like this as components and it just never quite works out well. How to populate the response to the Redux store? How to treat errors? Retries? More advanced API calls that might require some data manipulation? We’ve always found a separate API layer works best and is far easier to test.
- zarathustreal 6y agoAbsolutely.. it doesn't make sense to pick a tool like React (a DOM rendering library) to solve the problem of HTTP request state management, that's just not what "component" means in the React sense.
- merb 6y agowell the problem is, that most of the time you want: 1. show that you start a http call, by showing a loading spinner and blocking a button 2. do the http call 3. show the result to the user (even errors) 4. unblock the button and hide the spinner thus most often http requests are somehow required to be dependant on your ui. libraries like redux-saga help with that.
- bluedevil2k 6y agotry { setLoading(true); const responseData = await api.some.apiCall(); updateUI(responseData); } catch (err) { showUIError(); } finally { setLoading(false); }
- merb 6y agothat was the easy part. now what do you do if two independent parts of the ui need to do something if the responseData gets updated. you either bubble downards or you need a store.
- bluedevil2k 6y agoYes, you'd probably use Redux and wrap that API call in a dispatch(). I try to stick to the design pattern of "get a response from the API call if you need the data in that function, otherwise use Redux". I've created a library that really helps with this if you're interested - https://github.com/bluedevil2k/ruffle https://github.com/bluedevil2k/ruffle
- pigcat 6y agoI can't see any advantage of doing it this way You are already deciding that this component will take one of iconProps/buttonProps/dropdownItemProps, so you've already decided what type of component it is. You might as well just call <Icon />, <Button />, or <Dropdown.Item /> from the parent component and have more readable code.
- cam-perry 6y agoAgreed, I would have a component like <CommentDeleteButton /> render a normal <Button /> by default without these different iconProps/buttonProps/dropdownItemProps. Then give a renderButton prop to override the Button and use whatever <Icon />, <Dropdown.Item /> you want when you are using the <CommentDeleteButton />.
- skaushik92 6y agoThe crux of the problem here is overgeneralization to the point of losing meaning and scope. The most blatant signal for this is the else case here. The else case literally calls one of the props with on of its other props passed in. You could easily ask the question "Why couldn't the consumer of this component just do that instead?"—in this else case the Action component loses meaning by breaking the single responsibility principle. It becomes so overgeneralized that literally any component could be rendered through it.
- dcherman 6y agoAgreed, was typing a similar comment, however yours expressed that idea better than mine. Consider that this pattern contains N different properties that renders N different components (plus the default case), I don't know why you would want to put this into one component rather than having the consumer select the right one themselves. This even introduces the possibility of bugs - what happens if a consumer passes two sets of properties (which admittedly could be somewhat prevented by Typescript)? I don't really see the benefits of this approach for this particular example.
- wnevets 6y agoIt feels like this is a general problem with the entire React ecosystem.
- ZephyrBlu 6y agoI think the benefit of a component like this is that it is a well defined interface. As long as it doesn't become a monster super-component then it seems useful.
- tacotacotaco 6y agoNo. This person seems not understand composition. This pattern can be useful but is poorly employed in this example. If you are interested in good practices please read Eric Elliot's Composing Software https://medium.com/javascript-scene/composing-software-the-book-f31c77fc3ddc https://medium.com/javascript-scene/composing-software-the-b...
- zarathustreal 6y ago100% this person is misusing composition. Especially in the React Component sense. There seems to be some key details of React missing from the original posters' understanding, and I highly recommend anyone reading this to look into functional programming to understand what a React component is and does and how to build higher-level abstractions like the one(s) mentioned by this original poster the correct way. Higher-order functions and a real state management library are the obvious solutions to these problems
- ZephyrBlu 6y agoCan you elaborate on why this is a poor use of composition?
- zarathustreal 6y agoThis is not at all standard practice. It should be noted that just because you read something in a post on hacker news doesn't mean it's a good idea, common practice, or even well-reasoned. OP for example is making extensive use of Hooks, which are well-known not to be the best solution to the problem of state management within React. For example, useState leads to prop drilling, coupling, and cumbersome usage patterns. React is a declarative rendering library, it should handle: A. What DOM elements are created? B. How are those DOM elements structured? C. What styles and attributes do those DOM elements have?
- pizzeriafrida 6y ago> use of Hooks, which are well-known not to be the best solution to the problem of state management within React. I swear the introduction of Hooks threw such a monkey wrench into the ecosystem. They're just powerful enough to seem like an okay thing to use 100% of the time... I thought we all agreed that React's ultimate destination was obscurity while it quietly became the runtime target and developers just had to return dom structures as data from functions. But they got greedy, and didn't want to go down like that, and now we have a worse version of redux instead of building on top of react-redux. Perhaps my perception is wrong, in which case apologies and I'm happy to learn and listen.
- dragonwriter 6y ago> OP for example is making extensive use of Hooks, which are well-known not to be the best solution to the problem of state management within React. Hooks are fine. Using React’s built in Hooks [0] for nonlocal state management is a bad idea, but using, say, react-redux and useSelector/useDispatch is a different story. Well, if you use useState for something other than local component state, but that’s pretty expressly not using it correctly. [0] which are fine for local, intracomponent, state management, including within groups of components that are tightly logically coupled so that implementation coupling isn’t really adding an additional burden.