Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disable react/no-multi-comp? #154

Closed
dcousens opened this issue Jun 10, 2015 · 12 comments

Comments

@dcousens
Copy link
Member

commented Jun 10, 2015

The react/no-multi-comp rule disallows multiple react classes from being declared per JS file.
IMHO this actually encourages a more Java-like approach, and forces you to start nesting your file structures deeply rather than flat, as small components (a few lines at best) start reaching out into the file system instead.
Or worse, it seeds a habit of avoiding extracting them at all and instead in-lining them into the parents render function.

I personally think this is frustrating, and should be removed.

Thoughts?

@dcousens dcousens added the question label Jun 10, 2015

@jprichardson

This comment has been minimized.

Copy link
Member

commented Jun 10, 2015

Yep, I hate this and agree with @dcousens :p

@jprichardson

This comment has been minimized.

Copy link
Member

commented Jun 10, 2015

"hate" may have been a bit strong :) Actually, this may deserve a bit of a discussion. At first, it really bothered me. Eventually I've gotten use to it. I'm working on a pretty big React application and I could see how this will really bother me when I start refactoring my components. Overall, I still think that this rule should not be enforced i.e. multiple components should be allowed in one file.

@feross

This comment has been minimized.

Copy link
Member

commented Jun 10, 2015

I have no opinion on this since I don't use React. I changed the rule as you guys requested.

@dcousens

This comment has been minimized.

Copy link
Member Author

commented Jun 11, 2015

ping @cesarandreu [penny for your] thoughts?

@cesarandreu

This comment has been minimized.

Copy link
Contributor

commented Jun 11, 2015

I think it's fine to disable it, I don't have a strong opinion about it since I haven't built anything really large with React yet.

But I'd love to see an example of a cases where you think it's better to have multiple class definitions in the same file. So far when I've broken stuff up in a file I've just used static functions that take the expected props and return some DOM elements :D.

@dcousens

This comment has been minimized.

Copy link
Member Author

commented Jul 2, 2015

@cesarandreu

So far when I've broken stuff up in a file I've just used static functions that take the expected props and return some DOM elements :D.

That will fail if you want to use things like context without mixing concerns in the top level class.
Often, these small classes will be quickly extracted if they become generic or re-usable.

@winkler1

This comment has been minimized.

Copy link

commented Aug 10, 2015

From https://speakerdeck.com/vjeux/why-does-react-scale-jsconf-2014
image

Facebook recommends having a component per file, with the name matching the component name -- so there is no grepping for file contents, but a quick predictable mapping.

@dcousens

This comment has been minimized.

Copy link
Member Author

commented Aug 10, 2015

Facebook recommends having a component per file, with the name matching the component name -- so there is no grepping for file contents, but a quick predictable mapping.

I guess that is subjective to your IDE, I have no problems switching around between files, and often use stateless pure functions for mini-components in React. The 1-class-per-file idiom might be necessary if you have thousands of classes in a project, but, I personally work more modular than that, and this would be very frustrating to have enforced.

@winkler1 if you want to continue discussion on this, please open an issue in https://github.com/feross/eslint-config-standard-react.

@cesarandreu

This comment has been minimized.

Copy link
Contributor

commented Aug 10, 2015

FWIW, after using React for a while and trying to keep a single component per file... You realize how much of a terrible idea it is. Breaking up components in the same file just makes it easier to reason about what the overall thing represented in the file, even if it's broken up into multiple components. Otherwise you need to crawl through ten different files to understand where data is going.

@winkler1

This comment has been minimized.

Copy link

commented Aug 10, 2015

^ agree for quick personal stuff. For big work codebases, one component per file works a lot better IMO.

@cesarandreu

This comment has been minimized.

Copy link
Contributor

commented Aug 10, 2015

Even when the components are tightly coupled? Breaking up a bunch of tightly coupled components that aren't meant to be reusable doesn't really make sense. I break off components when they're reusable, but it helps to break up large components into smaller parts, and composing them while keeping em in the same file.

I've tried the "one component per file" approach and moved away from it because it made it too difficult to understand how anything worked. You either end up with huge complicated components, or you end up having to chase through ten different files.

@dcousens

This comment has been minimized.

Copy link
Member Author

commented Aug 11, 2015

^ agree for quick personal stuff. For big work codebases, one component per file works a lot better IMO.

Disagree, the entirety of my experience using React has been for now-in-production projects, and in that, I closely align with @cesarandreu, extraneous files for no reason leads to code bloat and monolithic style project management.
If you have re-usable code, extract it out. If its tightly coupled, let it stay that way, don't try and hide that fact by spreading complexity outwards.

@lock lock bot locked as resolved and limited conversation to collaborators May 11, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
5 participants
You can’t perform that action at this time.