Skip to content

Allow multiple stateless components in a single file #575

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

Merged
merged 1 commit into from
Jan 7, 2016
Merged

Allow multiple stateless components in a single file #575

merged 1 commit into from
Jan 7, 2016

Conversation

SimenB
Copy link
Contributor

@SimenB SimenB commented Nov 10, 2015

IDK if you want it, but we had linting errors after upgrading eslint-plugin-react.

This change allows multiple stateless components in a single file.

Requires 3.80 of eslint-plugin-react
https://github.com/yannickcr/eslint-plugin-react/blob/master/CHANGELOG.md#380---2015-11-07

@mkscrg
Copy link

mkscrg commented Nov 20, 2015

👍

@ljharb
Copy link
Collaborator

ljharb commented Dec 27, 2015

This relates to #612.

What is your rationale for not putting each component in its own module?

@SimenB
Copy link
Contributor Author

SimenB commented Dec 27, 2015

Mostly it's because the stateless functions makes it really easy to have lots of really tiny components, that might not be reusable. Having them in their own files seems overkill when they are only used in a single place. As we use CSS Modules as well, that'd mean an extra stylus file in addition, for something that might be 3 lines of code.

@ljharb
Copy link
Collaborator

ljharb commented Dec 27, 2015

That seems reasonable to me. Let's get some more consensus (which might take a week or so, due to holidays) before proceeding.

@goatslacker
Copy link
Collaborator

👍 but we don't use stateless components yet so 😐

@ljharb
Copy link
Collaborator

ljharb commented Jan 7, 2016

Sounds like this is OK to merge, it just won't do us any good at Airbnb just yet :-)

@SimenB can you rebase this freshly on top of latest master, and also add a note to this effect in the react styleguide?

@SimenB
Copy link
Contributor Author

SimenB commented Jan 7, 2016

Will do later today!

@SimenB
Copy link
Contributor Author

SimenB commented Jan 7, 2016

@ljharb Updated. I don't know what text you actually want though. Is this fine?

@ljharb
Copy link
Collaborator

ljharb commented Jan 7, 2016

This looks great! perhaps a link inline to the eslint rule docs?

@SimenB
Copy link
Contributor Author

SimenB commented Jan 7, 2016

Like so?

ljharb added a commit that referenced this pull request Jan 7, 2016
[eslint config] [minor] Allow multiple stateless components in a single file
@ljharb ljharb merged commit ccef929 into airbnb:master Jan 7, 2016
@SimenB SimenB deleted the patch-1 branch January 7, 2016 18:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants