ember-cli / ember-cli-eslint

Ember CLI addon for linting Ember projects with ESLint
MIT License
116 stars 47 forks source link

warning File ignored by default. Use "--ignore-pattern '!node_modules/*'" to override #63

Open oligriffiths opened 8 years ago

oligriffiths commented 8 years ago

Hi

Since updating to ember 2.5 recently, I'm now getting this error:

/Users/oli/Projects/ember/node_modules/ember-freestyle/addon/modules/ember-freestyle/services/ember-freestyle.js
  0:0  warning  File ignored by default. Use "--ignore-pattern '!node_modules/*'" to override

Any ideas? This was working fine before.

Thanks

jcano commented 8 years ago

I'm also getting this error, but from upgrading ember-cli-eslint from 1.3 to 1.4. The previous release with a separate .eslintrc for test and dev, although painful to maintain, was working on Ember 2.5.

I'm not familiar with eslint configuration files but it seems that Ember or eslint doesn't like using two js config files with module.exports.

oligriffiths commented 8 years ago

@jcano How do you have separate .eslintrc files for test/dev?

jcano commented 8 years ago

@oligriffiths I don't know the implementation details, but the previous version of ember-cli-eslint had two .eslintrc files, one on the root of the project and one in the tests directory. When running ember test, the test runner would read tests/.eslintrc instead of the one at the root.

In the current release, there are two files too, but they are .eslintrc.js with modules.export inside. They are situated on the root of the project and the tests directory again, and I'm assuming that the tests one is trying to only change the environment and leave the rest of settings untouched as this is the only contents in tests/.eslintrc.js:

module.exports = {
  env: {
    "embertest": true
  }
};

For some reason, eslint or the Ember test runner is not picking up on this and thus, showing the message you wrote on your post.

BrianSipple commented 8 years ago

@oligriffiths @jcano Are your files still being linted despite the warnings? I've been looking into this, as it occurs for tests/.eslintrc.js as well, and while I still can't fathom why eslint is throwing warnings, any configuration I define there does seem to be getting used.

oligriffiths commented 8 years ago

Well, tests fail as a result. I have noticed that I don't have the .eslintrc.js but do have an .eslintrc in the root, but not in tests. Going to try re-installing the latest version.

oligriffiths commented 8 years ago

So, I reinstalled, and have both of those files now. I'm seeing the errors I posted above, plus tests/.eslintrc.js is show up, along with: .eslintrc.js: line 1, col 1, 'module' is not defined.

jcano commented 8 years ago

In my case, the files are still being linted but the tests spits a warning ("File ignored by default"). About the settings, I don't know what "env": { "embertest": true } is supposed to do but rules added on that file are actually used for linting, in addition to the ones defined on the root of the project.

The warning is probably referring to not linting .eslintrc.js, which would be expected but there should be a way of explicitly telling eslint not to even consider that file for linting.

BrianSipple commented 8 years ago

It looks like this traces back to a behavior in eslint where warning messages for default-ignored files were being output. But then it also appears that this behavior has been removed as of eslint@2.10.2. I'm thinking we might just need to update the version of eslint being used under the hood by broccoli-lint-eslint -- or configure it in a similar manner.

As for the file generation discrepancies, @oligriffiths, .eslintrc.js and tests/.eslintrc.js are now the intended starting points. I added a note to CHANGELOG.md to clarify that.

oligriffiths commented 8 years ago

Yeah linting is still working but those warnings cause the test suite to fail.

On 18 May 2016, at 07:51, Brian Sipple notifications@github.com wrote:

It looks like this traces back to a behavior in eslint where warning messages for default-ignored files were being output. But then it also appears that this behavior has been removed as of eslint@2.10.2. I'm thinking we might just need to update the version of eslint being used under the hood by broccoli-lint-eslint -- or configure it in a similar manner.

As for the file generation discrepancies, @oligriffiths, .eslintrc.js and tests/.eslintrc.js are now the intended starting points. I added a note to CHANGELOG.md to clarify that.

— You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub

sonnyt commented 8 years ago

I am experience the same with my ember 2.5 project. ESLint is trying to lint the ignored file but exiting with:

undefined:undefined  - File ignored because of a matching ignore pattern. Use "--no-ignore" to override. (undefined)

which is causing ember test to fail.

oligriffiths commented 8 years ago

What's involved in updating to eslint 2.10+ ?

BrianSipple commented 8 years ago

@oligriffiths I don't think 2.10+ would introduce anything beyond incremental minor updates from the current version we're using (2.4).

That said, after digging around a bit into how we're currently handling ignored messages, I was able to implement a solution locally (adding to/modifying the RegExp check that's going on there) that prevents the warnings (including the one that you're seeing, @sonnyt) -- and I'm working on pushing it out soon. It definitely seems like this approach can stand to be more robust going forward, but it should solve the immediate issue.

oligriffiths commented 8 years ago

Great. Is that on a branch?

Regarding the version, can we bump that anyway? Currently 6 point releases behind.

On 18 May 2016, at 19:02, Brian Sipple notifications@github.com wrote:

@oligriffiths I don't think 2.10+ would introduce anything beyond incremental minor updates from the current version we're using (2.4).

That said, after digging around a bit into how we're currently handling ignored messages, I was able to implement a solution locally (adding to/modifying the RegExp check that's going on there) that prevents the warnings (including the one that you're seeing, @sonnyt) -- and I'm working on pushing it out soon. It definitely seems like this approach can stand to be more robust going forward, but it should solve the immediate issue.

— You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub

BrianSipple commented 8 years ago

https://github.com/ember-cli/broccoli-lint-eslint/pull/37 was just merged in broccoli-lint-eslint, and I bumped its version to 2.3.0.

@oligriffiths @sonnyt Reinstalling ember-cli-eslint will update your broccoli-lint-eslint dependency. I've done this in my projects and I'm no longer seeing ignore warnings; could you confirm that this is fixed for you as well?

oligriffiths commented 8 years ago

Awesome. Will check out an confirm asap

On 19 May 2016, at 07:14, Brian Sipple notifications@github.com wrote:

ember-cli/broccoli-lint-eslint#37 was just merged in broccoli-lint-eslint, and I bumped its version to 2.3.0.

@oligriffiths @sonnyt Reinstalling ember-cli-eslint will update your broccoli-lint-eslint dependency. I've done this in my projects and I'm no longer seeing ignore warnings; could you confirm that this is fixed for you as well?

— You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub

sonnyt commented 8 years ago

@BrianSipple the fix worked for me! Thank you 👍

maoueh commented 8 years ago

I don't get the warning anymore when doing ember test. However, inside Atom, when using the linter-eslint plugin, the warning is still present (for the two .eslintrc.js and tests/.eslintrc.js files). The broccoli-lint-eslint is at 2.3.0 here.

Not sure what makes Atom behave differently but ready to investigate if you have clues or directions I can follow.

oligriffiths commented 8 years ago

I too am seeing those warnings when running ember serve.

On 21 May 2016, at 06:25, Matthieu Vachon notifications@github.com wrote:

I don't get the warning anymore when doing ember test. However, inside Atom, when using the linter-eslint plugin, the warning is still present (for the two .eslintrc.js and tests/.eslintrc.js files). The broccoli-lint-eslint is at 2.3.0 here.

Not sure what makes Atom behave differently but ready to investigate if you have clues or directions I can follow.

— You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub

BrianSipple commented 8 years ago

https://github.com/ember-cli/ember-cli-eslint/issues/48 also touches on mysterious behavior in Atom.

I just did a fresh install of linter-eslint in Atom, generated a new Ember app with ember-cli @ 2.6.0-beta.2, ran ember install ember-cli-eslint, and then served... no ignore warnings. Hardly conclusive in a lot of ways -- but @maoueh & @oligriffiths, are you on the latest linter-eslint plugin? Furthermore, could one of you screenshot the alert as it appears in Atom?

maoueh commented 8 years ago

@BrianSipple I was probably using the latest linter-eslint at time of writing because it was a fresh install at that time. But it might have been updated since then. Note that doing ember serve is not giving me any warning, only in atom.

As for #48, I guess new user won't hit the issue anymore as the default generated config file is not referencing eslint-config-ember anymore. However, I hit a similar issue when I switched to eslint in my project as I wanted to use eslint-config-airbnb and in atom, it was complaining that it couldn't find the config module. The problem was only seen in atom, on the command line, the module was found correctly. I don't understand the implication yet, but I simply added eslint to my dev dependencies in package.json and it fixes the problem.

I will try to get you a screenshot tonight as I'm on another computer right now, otherwise, it might slip until tomorrow night. Also, I'm sure I will be able to add more information about the not found module.

maoueh commented 8 years ago

Here the image in question:

image

Also, just did this steps and got the problem:

cd /tmp
ember new simple-ignore-bug
cd simple-ignore-bug
ember install ember-cli-eslint
atom .
# Deleted all .jshintrc files
npm uninstall --save-dev ember-cli-jshint

And once I opened .eslintrc.js file, hit the warning above. Here the various bits of information:

If you cannot reproduce with the step above, it might be platform specific maybe Windows problem due to different separator or something else.

BrianSipple commented 8 years ago

@maoueh Yep, I’m seeing this too.

This is a tough one. It appears to be related to an issue that's affecting all users of eslint at the moment, and I’m not quite sure it’s something we can address here (But I'd love to hear if anyone has ideas 😀).

To elaborate, briefly: Behind the scenes, ember-cli-eslint runs eslint by way of broccoli-lint-eslint -- which explicitly takes measures to prevent this from occurring. With atom’s linter-eslint, however, your linting pipeline is actually linter-eslint —> eslint, and thus it’s completely outside of anything ember-cli-eslint is doing.

(Worth noting, though — especially for anyone just now getting to this issue: since the error is localized to Atom, it shouldn’t cause builds to break, as that process does still go through broccoli-lint-eslint.)