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

max-classes-per-file enabled in code but not in docs #2143

Open
SeanOfTheHillPeople opened this issue Dec 9, 2019 · 9 comments
Open

max-classes-per-file enabled in code but not in docs #2143

SeanOfTheHillPeople opened this issue Dec 9, 2019 · 9 comments

Comments

@SeanOfTheHillPeople
Copy link

@SeanOfTheHillPeople SeanOfTheHillPeople commented Dec 9, 2019

The max-classes-per-file rule is enabled in rules/best-practices.js but there is no mention of this requirement in the docs. If this is a best practice, I'd like to know why.

ljharb referenced this issue Dec 9, 2019
@ljharb
Copy link
Collaborator

@ljharb ljharb commented Dec 9, 2019

The same rationale that applies to https://github.com/airbnb/javascript/tree/master/react#basic-rules (but also appears not to be documented): the whole point of modules is to make them as small and atomic and reusable as possible, which means splitting things up as much as possible.

Is there a specific use case where you think this rule shouldn't apply?

@SeanOfTheHillPeople
Copy link
Author

@SeanOfTheHillPeople SeanOfTheHillPeople commented Dec 9, 2019

Thanks!
I'll have a look at our offending modules with the team to see if there's a case for keeping them together.

@SteveThorpe
Copy link

@SteveThorpe SteveThorpe commented Dec 9, 2019

I would advocate for the use of a single module for model and controller classes for the same 'component' (in non-React ES6)

@ljharb
Copy link
Collaborator

@ljharb ljharb commented Dec 9, 2019

@SteveThorpe that presumes the MVC pattern is an ideal one to be following for the frontend, react or otherwise :-)

@SteveThorpe
Copy link

@SteveThorpe SteveThorpe commented Dec 9, 2019

@ljharb MVC is far from ideal (IMO), even for our codebase. But if you are starting out with a codebase that is 'sort-of' MVC and, have a directive to impose AirBnb style on it, then the max-classes-per-file rule makes things worse (again, IMO)

@SteveThorpe
Copy link

@SteveThorpe SteveThorpe commented Dec 9, 2019

@ljharb So would you agree that if you are stuck with the MVC pattern then you could be justified in turning off the max-classes-per-file rule? ;)

@ljharb
Copy link
Collaborator

@ljharb ljharb commented Dec 9, 2019

You can always turn off any rules that aren't a good fit for your use case - the Airbnb config assumes you're using Babel, targeting both web and node, using React, etc.

@ghost

This comment was marked as off-topic.

@ljharb

This comment was marked as off-topic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.