[HN Gopher] React best practices
___________________________________________________________________
React best practices
Author : onoufriosm
Score : 39 points
Date : 2021-05-03 20:59 UTC (2 hours ago)
(HTM) web link (onoufriosm.medium.com)
(TXT) w3m dump (onoufriosm.medium.com)
| franciscop wrote:
| I've been doing React for 4-5 years now. My opinion on these best
| practices:
|
| 1. Strongly agree. I'd add a note or two there about styled
| components or your favourite CSS library, and how thinking in
| components and the tree instead of html vs stylesheets is very
| useful.
|
| 2. Strongly disagree. This indeed seemed like a best practice
| when I was getting started, but that Action then keeps growing
| and growing until nobody knows exactly how it works or how to
| tame it. It's better to keep components small (point 1!) and
| focused, so let buttons be Buttons.
|
| 3. Cannot comment. I haven't worked on so many "just a CRUD app"
| so this might be useful for those. I can see the value of it if
| you have e.g. an admin panel or CMS editing a DB or similar. I
| would leave this repeating logic only to that though, not to the
| general logic of all the app.
|
| 4. Agree, but also use context to your advantage. e.g., instead
| of `useToggle()`, name it `useVisible()` or whatever is the
| appropriate name.
|
| 5. Strongly agree, it's very powerful to first write the high-
| level API you want by typing first the desired component tree,
| and then define the components themselves.
|
| 6. Yes we can try different things depending on our apps' needs,
| but then it's not a "React Best Practices", maybe just a useful
| tip? :)
| pigcat wrote:
| I disagree with 4 as well
|
| Maybe the author just used a poor example, but his useToggle is
| actually less readable to me.
|
| The initial example with just useState is super obvious to
| understand what's going on in that component.
|
| With his useToggle example, if I'm in this codebase I'm now
| asking myself - does toggle() fire some hidden side effect? Now
| I need to go check out the hook to make sure.
| franciscop wrote:
| Well that's the goal, that you should not really care (too
| much) how it's exactly implemented to use it, right? It could
| be very simple or very complex, it could be in Redux or a
| simple state hook. In the case of more serious side effects
| like if it persists through localStorage, you read it once
| (hopefully through some docs!) and then you know how to use
| it anywhere.
|
| I agree with you that in the author use case it's waaay too
| simple and the best case is to use it inline, but I believe
| what they meant to do (hopefully!) is use that as an example
| of a potentially more complex hook and how you can/should use
| custom hooks instead of shoving all the logic in the
| component.
| tshaddox wrote:
| > 2. Strongly disagree. This indeed seemed like a best practice
| when I was getting started, but that Action then keeps growing
| and growing until nobody knows exactly how it works or how to
| tame it. It's better to keep components small (point 1!) and
| focused, so let buttons be Buttons.
|
| I'll reiterate strong disagreement here. This approach is very
| tempting to reduce code duplication, but in my experience you
| end up essentially building a "JSX" component that takes an
| arcane combination of props and returns some specific bit of
| JSX. So you end up invoking <JSX type="button"
| buttonColor="blue" buttonLabel="Continue" />. Why not just
| invoke <Button color="blue">Continue</Button>?
| elanning wrote:
| The single best piece of React knowledge I've found is an old
| idea from functional programming and MVC. Separation of
| components into 2 groups. Pure "view" components that contain no
| state and are oriented around display. And "controller"
| components that put everything together. The controller
| components make http requests, manage state, and manage which sub
| view is currently active. So far it has worked out well. Dan
| Abramov had a similar idea with presentational vs container
| components but missed a lot of the nuance that functional
| programming had brought, and thus, value.
| activitypea wrote:
| What's gained from that?
| elanning wrote:
| Good question. Pure view components are incredibly easy to
| test and reason about. State management usually gets put into
| hooks that the controller component uses, which are also
| easier to test. You can have a nice set of unit tests for the
| hooks and the view components, and then a few "e2e" or
| integration type tests for the whole thing. Separating code
| into groups like this can also make it easier to find things.
| It also lends itself to nice reusable components.
| franciscop wrote:
| I have found better success personally with "presentational vs
| container" instead of "view vs controller" as you describe it.
|
| For example, a checkbox IMHO would be better considered a
| presentational component even if it manages the small bit of
| state it needs for it to work. Same with many of the state
| management that is around "visual" changes.
|
| The point where I normally decide to split it up like you say
| into sub-components is just when the component becomes too
| complex.
| elanning wrote:
| Yes, many have found good success with putting a minor bit of
| UI state into the "mostly pure" view components. It works
| well until some other part of the app needs to uncheck the
| checkbox due to some new business requirement. This is why I
| usually provide a nice default "useCheckbox" hook in the same
| file as the pure Checkbox component.
| cmwelsh wrote:
| Absolutely. The best thing I did for my React code base was to
| separate the entire view layer from the logic layer. That
| literally means I had components called e.g. UserProfileView
| that were stateless and unit tested, and another set of
| components called e.g. UserProfile that managed state.
|
| Push _all_ your callbacks into a separate component.
| ljm wrote:
| React has really turned into something else. Back in 2013, React
| was making waves but it was just the _view layer_. This was back
| when we were trying to replace jQuery with BackboneJS or Knockout
| JS or whatever. For whatever reason, MVC doesn 't work in the
| browser so prior art goes out of the window.
|
| The advice to take a react hook and turn it back into an HOC is
| bizarre. This is not a best practice.
|
| Any post proclaiming a best practice is suspect.
| lucasmullens wrote:
| It's a bit light on hooks and heavy on HOCs. For people starting
| new React projects, HOCs are probably worth avoiding in favor of
| hooks when possible. That's just my opinion, but I think it's a
| widely shared one.
|
| That whole User/UserRead/ReacHOC thing could be much more
| readable if there was just a useUser() hook instead.
| meowface wrote:
| 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 wrote:
| Yeah, 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 wrote:
| Yeah, 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 wrote:
| React 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 wrote:
| I'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 wrote:
| Clojure 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 wrote:
| That'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 wrote:
| Case in point I literally only use JSX and ReactDOM.render().
|
| Some would call this a blasphemy. Works great.
| seumars wrote:
| One react codebase being different from the next is pretty
| much the norm. I even doubt that the majority of react
| developers have fully transitioned from classes to hooks.
| bennyg wrote:
| This 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 wrote:
| I'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 wrote:
| Absolutely.. 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.
| pigcat wrote:
| I 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 wrote:
| Agreed, 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 wrote:
| The 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.
| wnevets wrote:
| It feels like this is a general problem with the entire React
| ecosystem.
| ZephyrBlu wrote:
| I 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.
| dcherman wrote:
| Agreed, 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.
| tacotacotaco wrote:
| No. 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-b...
| zarathustreal wrote:
| 100% 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 wrote:
| Can you elaborate on why this is a poor use of composition?
| zarathustreal wrote:
| This 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 wrote:
| > 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.
| activitypea wrote:
| Translation and i18n of text is really just a function - send
| text in, get text out. Why would you ever put it in a hook, let
| alone a component?
|
| > We managed to abstract away the logic for translation into the
| Translation component.
|
| A function call. You abstracted away a function call.
|
| > Now there is a centralized translation component where we can
| change any UI or business logic if needed.
|
| A plain old JS function still centralizes i18n functionality. As
| for the UI logic, all the component is doing is wrapping text,
| therefore your component needs to support anything anyone would
| ever want to do a p tag. Or an h1 tag. Or a li tag. Or every
| other tag that can contain a text node. At some point you're just
| reimplementing the DOM API. What specifically is gained in going
| from `<p>i18n('Hello!')</p>` to `<Translation text='Hello!'/>`?
|
| > It's now a lot easier to test files that use translation.
| Before refactoring we would have to mock the useTranslation
| somehow because we don't want to test its implementation. With
| the refactored code we only need to check that we render the
| Translation component and that we pass the correct props.
|
| I can't criticize this part because I genuinely don't understand
| where its coming from. "We only need to check that we render the
| Translation component" implies that the Translation component is
| rendered in the test. And for the Translation component to
| render, the `useTranslation` hook still has to be called, so we
| still have to mock it, right?
| zarathustreal wrote:
| I think this poster is missing the key insight that React
| components are indeed nothing more than functions. If anyone is
| reading this right now go console.log the output of your
| component, it's an object that describes the DOM output. Logic
| should not live in components, React is a rendering library.
| Make use of higher-order functions. Make use of a proper state
| management solution. Don't over-use hooks. It's really that
| simple.
| eliseumds wrote:
| They're definitely more than simple functions if you look at
| it from the the entire application point-of-view. With
| components you have access to context and lifecycle. Doing:
| import someFunc from 'someFunc'; // ...
| return someFunc();
|
| is not the same as: import SomeComponent from
| 'SomeComponent'; // ... return
| <SomeComponent />
| tshaddox wrote:
| I agree that I don't see a huge advantage to using a
| Translation component over a useTranslation hook, but I think
| the useTranslation hook can sometimes make sense over a plain
| old JS function, because you might want that hook to grab stuff
| from React context or even fetch data from the server using
| useEffect. It might not be feasible to have a plain old JS
| function with all internationalization data for all supported
| languages.
| activitypea wrote:
| I guess I should clarify that I am specifically discussing i18n
| of text. I understand that proper i18n of apps is a much more
| complex topic.
| eliseumds wrote:
| I beg your pardon? Even translation of pure text (and not
| numbers, date, currency) relies on locale data, like plurals
| and genders. It's just way more elegant to have this data
| inside a React context instead of importing a global
| translator or passing in locale data whenever needed
| (dangerous repetition).
| aniforprez wrote:
| I genuinely don't understand this comment or the reason for
| writing it as a hook or a separate component in the article
|
| Why wouldn't you just write it as a function with data and
| context being passed through as arguments? Isn't it much
| simpler to write a pure function and test that function
| than render a component and test it? If you simply write a
| function that accepts the locale, the string and context,
| does the translator magic and returns the translated text,
| would you not then just need to write a unit test simply to
| make sure you're getting the expected output? Why the
| abstraction into hooks and then further into a separate
| component that you then have to render?
| eliseumds wrote:
| It doesn't matter what your code is doing inside that
| component, you can still make it easily testable by
| having the core translation pipeline as pure functions.
|
| See https://news.ycombinator.com/item?id=27031847 for
| more reasons.
| activitypea wrote:
| What's "elegant" about putting it in a React Context?
| Outside of initial configuration, it's a stateless
| function. Just chuck it in a separate module and have some
| initialisation code. Nothing about it demands that it live
| in the component tree.
| eliseumds wrote:
| It is not that simple. You could use the exact module in
| different environments: browser, NodeJS, React Native. If
| you don't use context, you either need to do some very
| ugly module replacement at build time or duplicate your
| code. Also, you lose lifecycle hooks (not that important
| in this case though).
| activitypea wrote:
| Coupling it to React Context makes it easier to reuse in
| Node? I beg your pardon? :)
|
| If you plan to reuse it in different environments, just
| give the user a `setConfiguration` and a
| `translateFunction` function. I prefer module-level
| variables for holding the i18n system state, but it could
| also be a class-based singleton, if that's your cup of
| tea. I still don't see how React plays into this.
| jakelazaroff wrote:
| _> Translation and i18n of text is really just a function -
| send text in, get text out. Why would you ever put it in a
| hook, let alone a component?_
|
| One concrete reason: if the language is user-configurable, you
| can avoid an additional argument at the call site by putting
| that data in a context.
| fastball wrote:
| Just want to mention that the section on "wishful thinking" is
| usually called "optimistic updates" (if you want to learn more
| and googling "wishful thinking" isn't giving you great results).
| eliseumds wrote:
| I guess you misunderstood it. It has nothing to do with
| optimistic updates. Optimistic updates are about rendering
| results that you hope will happen, like clicking on a "Like"
| button and immediately making it bold and disabled even before
| the request has finished. In case it errors, you simply revert
| to the previous state and display some feedback to the user.
|
| Now, wishful thinking is about making it work even if not
| perfect, production-ready. It could be about rendering a hard-
| coded "Loading..." or "Success" or "Error" string before you
| hand it over to your translation team. Or just calling your
| param names `a` and `b` before you talk to your domain experts
| about the proper terms. This comes with experience.
|
| This is not something the author invented, it's a well-known
| technique.
| activitypea wrote:
| Atrocious advice, I hope I never have to work in a codebase that
| follows these practices
| willio58 wrote:
| Specifically what do you not like? Some of it seems iffy to me
| but some of it also seems obvious e.g. "use components".
| purplerabbit wrote:
| I find almost every section to be extremely distasteful.
|
| for instance, points 2 and 3 basically suggest throwing YAGNI
| out the window in favor of premature abstraction. Combine it
| with the highly-decoupled style in point 1, and you have a
| nice spaghetti recipe.
|
| I could keep going, but it looks like everyone is piling on
| this poor guy already.
| activitypea wrote:
| <3
| activitypea wrote:
| I don't think there's a single paragraph in the article I
| agree with. In another comment, I picked on the parts that
| are the easiest to criticize in a shortish comment. The whole
| article screams bad OOP-style cargo cult programming, which I
| found particularly annoying :)
| eliseumds wrote:
| It might be obvious to you. Hiding concrete implementations
| behind an elegant, easy-to-use interface, is not something we
| see every day. I've noticed countless codebases importing
| HTTP clients globally, mocking a ton of modules for testing,
| imperatively translating messages and so on. Wrapping
| implementations with components/hooks allows you to future-
| proof your application in case you need, for ex, to access
| some context data, to develop for a new environment (mobile,
| desktop), or for easier testing as already mentioned,
| including snapshots. I specially like that the author talks
| about wrapping third-party libraries.
|
| Another benefit of components is lifecycle. Imagine a
| "RelativeTime" component; you could have called library
| functions directly to render "20s ago", but by making it a
| component you could tell it to update periodically. Cool
| stuff.
|
| Now, I understand that having such granular components can
| lead to some performance overhead if you're working on a
| massive application. If only there was an easy way to do AST
| transformations that would hoist small stateless
| components/hooks. Damn, AST transformations could even hoist
| stateful ones! I'll keep dreaming about it :)
|
| In regards to examples such as `Fetch`, `UserRead` and
| `EntityActionDelete` from the post, I'm kinda on the fence
| because, at the same time they are uglier than hooks, they're
| also more performant because updates would only re-render
| itself and its children, and not the entire parent component.
| There's no right or wrong here.
| chrisweekly wrote:
| > "For best results we should delegate any business logic into a
| nested component if possible. The deeper in the components tree
| the logic lives the better."
|
| Not to pile it on, but how did this make the front page?
| ZephyrBlu wrote:
| In terms of rendering this is actually correct. If you keep all
| your logic super high level, then your entire component tree
| will re-render often.
|
| Also, I think the author means that logic should as deep as it
| makes sense in the component tree which makes sense because
| then you can have better separation of concerns like he showed
| in his example.
| zarathustreal wrote:
| >If you keep all your logic super high level, then your
| entire component tree will re-render often.
|
| This is not true, especially if you're following _actual_
| best practices and using e.g. React.memo() and immutable
| state updates. React is explicitly built around the idea that
| you don't need to re-render every part of the DOM on every
| state update.
| activitypea wrote:
| Right, but you do have to re-render the React DOM, and for
| huge apps that can get expensive, at least in theory. I've
| yet to work on an app big enough where we have to reach for
| memo/PureComponent except for very specific cases e.g. a
| stock price ticker
| activitypea wrote:
| This is going against the actual established best practice of
| avoiding premature optimisations. React renders are crazy
| cheap, and when they start getting expensive, you can just
| chuck a PureComponent in there and `cut off` a whole subtree
| from unnecessary rerendering.
| ZephyrBlu wrote:
| There are definitely ways to mitigate excessive re-renders
| (Though they don't solve the problem of what happens when
| it actually needs to re-render), but there are also other
| reasons for wanting to push logic down the tree as far as
| possible like the one the author was conveying.
| johnghanks wrote:
| I've been looking for a best practices guide for React for a
| while... and this is definitely not it. I was optimistic after
| reading the first item (an actually good point) but the entire
| thing went completely off the rails after that.
|
| Anyone have any examples of _actually_ good best practices for
| React?
| seumars wrote:
| _Generic Action Component_ sounds like a premature optimization
| nightmare
| eliseumds wrote:
| It is indeed premature optimisation.
___________________________________________________________________
(page generated 2021-05-03 23:01 UTC)