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

Custom eslint rules #35

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

kooparse
Copy link

It is perhaps preferable to wait the plugin model (#30).

@kooparse kooparse changed the title Custom .eslintrc Custom eslint rules Oct 22, 2016
@@ -104,7 +104,7 @@ function generateServiceWorker() {
*
*/

const tasks = {
let tasks = {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You probably don't need to change this.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right 👍

@jchip
Copy link
Member

jchip commented Oct 24, 2016

Thanks for the PR. The general approach is good. If CWD/.eslintrc and CWD/test/.eslintrc exist, then use those instead of the ones in the archetype.

Eventually these are all the eslint check scripts

https://github.com/electrode-io/electrode-archetype-react-app/blob/master/arch-gulpfile.js#L189-L192

One of the things I was considering is to allow apps to override archetype config.

https://github.com/electrode-io/electrode-archetype-react-app/blob/master/config/archetype.js

@kooparse
Copy link
Author

kooparse commented Oct 25, 2016

In most cases, people are using only one linter config file (I think). But sure, if you want I could check for CWD/.eslintrc-react, CWD/.eslintrc-react-test, CWD/.eslintrc-node, CWD/.eslintrc-mocha-test custom files instead.
Yes, allowing apps to override the archetype config would be great, I will try to make a PR on that. :)

@@ -242,6 +242,18 @@ const tasks = {
"test-server-dev": () => shell.test("-d", "test/server") && exec(`mocha -c --opts ${config.mocha}/mocha.opts test/server`)
};

// Check if any .eslintrc file exists on the
// project root path.
const customLinterPath = Path.join(process.cwd(), ".eslintrc");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since eslint --init can generate files like .eslintrc.json, can this accept:

  1. no extension
  2. .js
  3. .json?

@citelao
Copy link
Contributor

citelao commented Nov 4, 2016

@kooparse +1 on separate configuration files, esp. since the archetype has the separation anyway.

However, ESLint doesn't support globbing/custom directories for .eslintrcs. It's in an issue right now. In order to get linting working in-editor (eg with Sublime-Linter), the only way is to place separate .eslintrcs in each directory:

  • / for the main/client-centric .eslintrc
  • /server/ for server-side
  • /test/client/ for client tests
  • /test/server/ for server tests

I've tested such placement (one .eslintrc in each folder as mentioned) and it works correctly in Sublime. Perhaps grab from these locations rather than by filename? IMO custom .eslintrc names are less helpful, since they can't work in editors (and won't be detected automatically in general).

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.

5 participants