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

Comply XO's "jsx-filename-extension" rule #739

Closed
wants to merge 3 commits into from

Conversation

davegomez
Copy link
Contributor

No description provided.

@davegomez davegomez changed the title Comply XO "jsx-filename-extension" rule Comply XO's "jsx-filename-extension" rule Sep 23, 2016
@davegomez
Copy link
Contributor Author

Any comment @ekmartin?

@ekmartin
Copy link
Contributor

Yes, sorry - here it is: I'm not sure if I agree with this. Dan Abramov explains it much better than I do though: airbnb/javascript#985 (comment)

@davegomez
Copy link
Contributor Author

I agree with that too but we are trying to comply with XO's rules #730, is not about what we think or what we like.

@ekmartin
Copy link
Contributor

Of course it is - for such a large project as this there's really not that much virtue in trying to comply with rules that we don't agree with, at least in my opinion. If the rule makes sense, definitely, but if not I don't see the huge win in bending the project in a way that feels unnatural.

I don't think a lot of people will go into developing HyperTerm with an expectation of how everything should look based on them knowing about xo. They'll run the linter, and fix what's being complained about - regardless of whether they agree with it or not. Thus I think some bikeshedding on issues like this is warranted, and not something to be afraid of.

@ekmartin
Copy link
Contributor

We shouldn't disable the rule nonetheless though, if we don't want .jsx we should set it to react/jsx-filename-extension: ["error", { "extensions": [".js"] }]

@davegomez
Copy link
Contributor Author

davegomez commented Sep 23, 2016

I don't fight with linters and I don't see any problem in renaming the files. I agree that use the rule react/jsx-filename-extension: ["error", { "extensions": [".js"] }] could be a solution, but I don't see any gain either way.

@matheuss
Copy link
Member

This rule was removed from eslint-config-xo-react 😅

@davegomez
Copy link
Contributor Author

davegomez commented Sep 23, 2016

@matheuss awesome, we should update the ruleset then

@matheuss
Copy link
Member

cc @sindresorhus

@ekmartin
Copy link
Contributor

Closing in favor of #741.

@ekmartin ekmartin closed this Sep 23, 2016
@davegomez davegomez deleted the filename-extension branch September 23, 2016 22:30
@sindresorhus
Copy link
Contributor

I already commented about it here: #723 (comment)

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

Successfully merging this pull request may close these issues.

4 participants