Reputation: 884
This is two React components source code coding styles (put your attention on onSubmit
):
Version 1:
...
const ContactsEditPage = ({ match }) => {
...
const [updateContact] = useMutation(
gql`mutation updateContactById ($id: UUID!, $input: ContactPatch!) {
updateContactById(input: {id: $id, contactPatch: $input}) {
clientMutationId
}
}`
);
return (
...
<Formik
initialValues={data.contactById}
onSubmit={(values, actions) => {
updateContact({
variables: {
id: match.params.contactId,
input: values
}
})
.then(() => history.push('./'));
}}
>
<Form>
<FieldGroup>
<FieldRow>
<FieldLabel>Email:</FieldLabel>
<FieldInput
as={Field}
type='email'
name='email'
placeholder='Email'
/>
...
);
};
Version 2:
...
const ContactsEditPage = ({ match }) => {
...
const [updateContact] = useMutation(
gql`mutation updateContactById ($id: UUID!, $input: ContactPatch!) {
updateContactById(input: {id: $id, contactPatch: $input}) {
clientMutationId
}
}`
);
function handleContactSubmit(values) {
updateContact({
variables: {
id: match.params.contactId,
input: values
}
})
.then(() => history.push('./'))
}
return (
...
<Formik
initialValues={data.contactById}
onSubmit={handleContactSubmit}
>
<Form>
<FieldGroup>
<FieldRow>
<FieldLabel>Email:</FieldLabel>
<FieldInput
as={Field}
type='email'
name='email'
placeholder='Email'
/>
...
);
};
The differences:
onSubmit
handleContactSubmit
function instead inline Javascript codeIn the first part of my coding life, I preferred to use handler function (version 2).
Now I have a preference for inline code (version 1) when:
Why: I prefer inline event handler code to avoid indirection in code to improve human readability (for coding review process).
There is also a cost to use a function (inderection). When a new reader encounters this code, they need to jump between many function definitions in many files. This non-linear reading process requires more mental focus than reading linear code.
I meet a majority of ReactJS coders who prefer to use handler function (produce indirection in code).
My question:
Note:
Best regards,
Stéphane
Upvotes: 2
Views: 595
Reputation: 28
TL;DR when not obvious, agree with your team on a scope basis.
Some good answers have already been posted here and the consensus seems to be that it depends on the preferences of the team as well as "objective" factors such as:
But in your example, the handler is neither of the abov, does multiple things and is a bit more complex than a single state update. One could argue that this complexity justifies using a handler, or this non-reusability justifies a in-line style; it is difficult to draw the line as readability is quite personal and depends on the developer's background, personal preferences, IDE etc. as explained with detail in yuyu5's answer
Your team may not agree on what complex-enough-for-a-handler mean, but could agree on drawing the line according to the scope of the function. For example: does it stay within the component (state, form validation, UI-changes) or does it goes beyond (localStorage, user log in, and in this case, programmatic navigation)?
A scope-based agreement (whether you pick the one I described or another) is easier to enforce and would age better than a complexity-based one, as IDEs and preferences change, coding styles should remain the same.
Upvotes: 1
Reputation: 161
My philosophy is to write code optimized for readibility that's why I try to keep the render / return as abstract as possible, it's easier to come back later (for debugging, new features...) and know quickly from the render / return what the function does from its name.
What seems to be a problem of the indirection is a false problem, because IMO:
For your questions:
Am I part of a coder minority who prefer indirection in code?
From what I've seen, in React world version 2 is the preferred and most used one.
Do I have to accept majority preference even if my non-linear reading process requires more mental focus than reading linear code? And add handler function rule use in the ReactJS style guide in my project?
IMHO, the most important thing is the team and what makes sense to its velocity. To me it's not a religion so my best answer is to choose the one that is more accurate to your/your team efficiency.
Upvotes: 0
Reputation: 474
TL;DR It depends on the situation and personal preference.
Using an arrow function in render creates a new function each time the component renders, which may break optimizations based on strict identity comparison.
This only applies to class components and is not relevant for functional components. In a functional component, the function is re-defined on every render regardless of if it's defined as function
, const myFunc = ...
, or onClick={() => ...}
because it's scoped to your component-function (this could be solved with memo
, but that's out of scope of your question).
avoid indirection in code to improve human readability
IMO, I would say this depends. Sometimes, especially when e.g. reviewing a PR or returning to code I haven't seen in a while, I like to see a function name in the component's prop like onClick={doSomething}
because then I can skim past the content I am not concerned with and focus solely on what I need to do for the task at hand. But, more importantly, it gives context to what is happening through the function name.
Yes, you're right that the dev has to scroll down to see where the functions are used and then scroll back up to see what the function does, but if I saw a component with multiple buttons, each with different handlers, something like below, I would have to actually read every single handler until I found the function I wanted to alter. If those were instead replaced with function names, I could easily just understand "Oh this button does this, whereas this other one does that" without having to scroll up to find the function declaration. This is especially true when the function is large or complex; re-reading a 30 line function that has no name just to try to remember what it does/why is very painful when you return to your code 6 months later.
return (
<div>
<button onClick={() => ...}>Some text</button>
<button onClick={() => ...}>Some other text</button>
</div>
);
Another thing to think about is levels of nesting. If you have a large render with many indentations, and then you add a complex function inline to props, suddenly you have to scroll left/right in your editor which essentially re-introduces the original issue you tried to solve.
Am I part of a coder minority?
No, there are use cases for both inline and separately-declared handlers. I write both in my repositories, both in and out of work. My teammates do the same.
If the handler is small and it takes little effort to understand what it does (e.g. onClick={() => location.hash = "/redirectedPage"}
), then it's totally reasonable, and probably better for readability, to use inline functions (why declare a new function redirectToPage
when it's a one-line function?). For more complex functions, or deeply nested components, it is often helpful to declare a function name.
There are other factors that could come into play here, like maintaining consistency within a code base, lint rules, etc., but these are some of the main ones that I think apply outside of specific tunings like those.
Upvotes: 2