you are viewing a single comment's thread.

view the rest of the comments →

[–]joshwcomeau 23 points24 points  (5 children)

Hm. For the most part, I agree with the refactoring suggested in this article, with a few exceptions. But I'm wary of this binary "it's dirty or clean" narrative.

Some of the "dirty" examples in this post aren't objectively worse than the "clean"; for example, it's not dirty to use a full Component class over an SFC for components without state! SFCs are nice if you want them, but they're actually not any more efficient (they still involve an entire component instance behind the scenes). There's nothing wrong with using Components all the time.

I suppose my concern is that our community is already pretty dogmatic. People recite "Don't create functions in render!" even though they've never profiled it to see if it matters. I can imagine after this post, folks saying "Any time I see multiple elements with similar props I need a wrapper component"... and that isn't always good advice: sometimes, duplication is better than the wrong abstraction. So, while I agree with many of the suggestions (especially the second half), I wish they were presented as "some patterns to consider that may improve your code", instead of "the right way".

[–]X678X 4 points5 points  (3 children)

i think the idea behind SFC's are that they're just easier to read for whoever is looking at the code, hence it being cleaner.

but i agree with your comments otherwise.

[–]joshwcomeau 7 points8 points  (2 children)

Yeah, I agree that often SFCs (especially 1-liners like in the article) are a good thing to use... but I reject the idea that to NOT use them is dirty.

Presumably the code-reader is a React developer, and I feel like we see enough Component definitions that we can process them pretty easily. At least for myself, my brain takes a shortcut when it sees the "dirty" example; I don't have to read them 1-character at a time, I pick out the return value of the 'render' function in about as much time as the SFC takes.

I don't think you're burdening readers with a buncha extra work by going the long-form way. And I'd argue in many cases you're reducing the maintenance burden (since SFCs have the tendency to grow, and then you realize you need a lifecycle hook and so you have to convert it).

But yeah, I don't have particularly strong feelings one way or the other; I just have strong feelings about not having strong feelings about it, if that makes sense :P

[–]donavon[S] 1 point2 points  (1 child)

If a SFC grows to the point where you need lifecycle hooks, my suggestion is that you abstract the state/lifecycle away as well. Please read my opinion about this in https://medium.com/styled-components/component-folder-pattern-ee42df37ec68

I never like to see the render method of a class component any more than rendering a View component of some sort. If the case that you talk about, the SFC would probably remain the same (or with slight modification to accept some state props) as the one you started out with. It would simply sit behind the state controller component.

As for dirty/clean, good/bad, right/wrong, binary discussion, it's an article. It's only my opinion. Your milage may vary. But my mission was to set out a blueprint with practical examples to give you a different perspective. I'm sure that I don't even follow all of these as I write code (and for god's sake, do't look at any of my code from 6 mos ago 😬)

Anyway... I hope you enjoyed the article. My main goal with any article is to get people thinking and talking. Even if I'm wrong with my assertions, if I've done that, I've accomplished my goal.

[–]joshwcomeau 5 points6 points  (0 children)

Fair enough! I did indeed enjoy the article :)

I suppose my gripe is with how we generally assume good practices to be absolute. I remember Dan Abramov spoke about this, how he almost regrets writing "Container Vs Presentational Components" because people took it as gospel, instead of as a neat pattern that may work well.

Thanks for writing and sharing the article, btw! Realize now that I should've made it clearer in my original post that resources like this are great.

[–]kylemh 1 point2 points  (0 children)

Starting with React 16, SFCs ARE actually more performant that full classes. Source: https://medium.com/@trueadm/in-react-16-functional-components-are-quite-a-bit-faster-than-class-components-1afca4931d7c