update logic & jsx rule, add refactors directory, with logic and jsx refactor #84
Conversation
jamiekyle-eb
requested review from
miglesiasEB,
BenAtEventbrite,
kwelch-eb and
jonathancreamer-eb
| render() { | ||
| return ( | ||
| <div> | ||
| {cond && otherCond && ( |
|
|
||
| > Refactoring a large `render()` method with lots of long if-then conditional | ||
| > logic into separate components and inline JSX logic. (Also: Reasons why | ||
| > destructuring is bad for refactoring code). |
There was a problem hiding this comment.
This is a powerful. I once had a senior engineer admonish me that every new variable introduced is additional mental overhead and complexity for future devs. Using dot notation to reference object values provides clear context for where the value originates.
There was a problem hiding this comment.
I think these sorts of things are subjective and there are very good arguments on both sides.
If you need to break up a render into multiple components then multiple class components then having this.props or this.state is great. I've definitely had to do destructuring surgery.
However, I've also had cases where I migrated a chunk of render() into a separate component function and because I had destructured at the top, I already had the props for the new helper function. I didn't have to strip out this.props or this.state first.
And if you're just looking at markup and variables (instead of dot notation) some people find that easier to parse than others.
I think we waste to pushing what we find easier to understand on the masses when there isn't an objective benefit to either way.
There was a problem hiding this comment.
True, I have seen both approaches cause hard-to-understand code. I'm loving hook-based state management in my personal projects which makes this.props and this.state a thing of the past anyway. Very much looking forward to using them professionally.
There was a problem hiding this comment.
I've definitely had to do destructuring surgery.
I've been thinking about this all weekend because I've been considering removing destructuring from the programming language I'm designing.
I can't come up with a case where I think destructuring improves code or a place where it is necessary.
And if you're just looking at markup and variables (instead of dot notation) some people find that easier to parse than others.
It may be ever so slightly easier to read, but objectively speaking you do have less information when you write code this way.
At this point, I think I group together destructuring with other language features of JavaScript I never recommend using, even if you stylistically prefer it.
| return <SomeComponent kind="gah" foo={props.foo} baz={props.baz} />; | ||
| } | ||
|
|
||
| function Component2(props) { |
There was a problem hiding this comment.
was there supposed to be destructuring here?
There was a problem hiding this comment.
oops, no... I wanted it to be <SomeComponent kind="goop" bar={props.bar} baz={props.baz} />
|
|
||
| if (includeHeader) { | ||
| header = (<h2>Pagination</h2>); | ||
| // bad (hard to refactor into seperate components / moves JSX around) |
There was a problem hiding this comment.
I'm not sure this is what we agreed to. I think this is swinging the pendulum to the other side. It's exchanging one group's preferences for another IMO.
I think the spirit of the rule is to limit a whole bunch of logic in JSX. If you wanna destructure with conditionals or access this.props/this.state w/ expressions, it shouldn't really matter
There was a problem hiding this comment.
I think the spirit of the rule is to limit a whole bunch of logic in JSX.
Yeah, which is why I included additional instructions on how to break down excessive logic in JSX.
I'm not sure this is what we agreed to. I think this is swinging the pendulum to the other side. It's exchanging one group's preferences for another IMO.
I know, but I would rather push this right now. Considering it deeper, I don't think the middleground is actually okay. If people need further convincing, I would rather do that convincing and make this a hard rule.
|
@vincent-eb @jdawson-eb Hey this is super old, but explains a really good concept and pattern. Is this worth keeping around for education? If so where can we incorporate it. If not, I will close. |
From today's Frontend Guild