eslint-plugin-prettier is failing with a wrong path

[scottkidder]

https://eslint.org/docs/user-guide/migrating-to-5.0.0#-linting-nonexistent-files-from-the-command-line-is-now-a-fatal-error

After upgrading to ESLint 5 and running ember test in an addon and I have found EslintValidationFilter trying to lint addon/addon/adapters

 ~/dev/project-123 (master) > ember test
cleaning up...
Build failed.
Build Error (broccoli-persistent-filter:EslintValidationFilter) in addon/adapters/application.js

ENOENT: no such file or directory, scandir '/home/skidder/dev/project-123/addon/addon/adapters'


Stack Trace and Error Report: /tmp/error.dump.66d834709b71560c98ced28c7b8425d9.log

[Turbo87]

@scottkidder can you provide a reproduction repository?

[runspired]

This is failing in ember data when we run yarn install --no-lockfile --non-interactive

Example failure: https://travis-ci.org/emberjs/data/jobs/396554871

However, I am suspicious of the root cause being eslint itself. Both before and after the command is run this output the same, notice how in our case there is no eslint version 5 in play:

npm ls eslint
ember-data@3.3.0-canary /Users/cthoburn/Github/data
└─┬ ember-cli-eslint@4.2.3
  └─┬ broccoli-lint-eslint@4.2.1
    └── eslint@4.19.1 

[runspired]

The following is the diff of all the drift involved that leads to this, have not spotted anything super suspicious. https://gist.github.com/runspired/d1383ce39e37009daf584b6a7705eb92

[runspired]

Traced the issue to eslint-plugin-prettier, the patch version 2.6.1 introduces a config requirement that is either poorly thought out or which we need to fix our default config for.

[runspired]

PR that introduces the bug: prettier/eslint-plugin-prettier#92

[runspired]

Traced the issue down to how root = true in .editorconfig is being treated by prettier, removing it causes prettier to work. Isolating why.

[ezpuzz]

I couldn't fix my project by modifying .editorconfig, but reverting to eslint-plugin-prettier 2.6.0 fixes my build.

[runspired]

@rwjblue thinks (suspects, although he's known to be rather spot on in these matters) the issue is that we've been giving bad path data to the plugin all along and now it matters.

[Turbo87]

I can confirm, turning the prettier/prettier rule off seems to "fix" the issue for me. This seems unrelated to the ESLint 5 update.

[rwjblue]

Yep, definitely unrelated to Eslint version. Prior to the most recent eslint-plugin-prettier the editorconfig of the project was never used, now it is..

[pzuraq]

Dove into this a bit and found the underlying issue. The problem is that broccoli-lint-eslint passes the wrong file path to ESLint. For instance, in an addon, it passes {pathToProject}/addon/addon/file.js instead of {pathToProject/addon/file.js}.

This happens because broccoli-lint-eslint naively walks up broccoli nodes to find the original input node ({pathToProject}/addon), and doesn't account for the fact that along the way, various nodes can move around and shift the files. Somewhere along the way, we funnel the addon files:

new Funnel(addonFiles, {
  dest: 'addon'
});

So, when broccoli-lint-eslint finally begins transforming the files, it attempts to combine the original tree path, and the new relative path of the file.

This bug has been present for some time, but wasn't being triggered because no eslint plugins were using the file context before the recent change to eslint-plugin-prettier. I think ultimately we need to either pass in the actual original file path somehow, or we need to pass in the full path to the temp file we're actually operating on.

[buschtoens]

we need to either pass in the actual original file path somehow, or we need to pass in the full path to the temp file we're actually operating on.

What do you think makes more sense? If I understand correctly, we are actually linting the temp file and not the original source file? In this case passing the the path of the temp file probably makes more sense?

[buschtoens]

  - broccoliBuilderErrorStack: Error: ENOENT: no such file or directory, scandir '/Users/janbuschtoens/clark/application/client-packages/addons/ember-clark-sentry/app/app/initializers'
    at Object.readdirSync (fs.js:750:3)
    at traverseFolder (/Users/janbuschtoens/clark/application/node_modules/prettier/index.js:32564:18)
    at findRoot (/Users/janbuschtoens/clark/application/node_modules/prettier/index.js:32580:10)
    at maybeParse (/Users/janbuschtoens/clark/application/node_modules/prettier/index.js:32592:16)
    at editorconfigSyncNoCache (/Users/janbuschtoens/clark/application/node_modules/prettier/index.js:32605:35)
    at editorconfigSyncNoCache (/Users/janbuschtoens/clark/application/node_modules/prettier/index.js:28538:20)
    at /Users/janbuschtoens/clark/application/node_modules/prettier/index.js:34796:14
    at Array.map (<anonymous>)
    at _resolveConfig (/Users/janbuschtoens/clark/application/node_modules/prettier/index.js:34795:40)
    at Function.resolveConfig.sync (/Users/janbuschtoens/clark/application/node_modules/prettier/index.js:34831:12)
  - codeFrame: ENOENT: no such file or directory, scandir '/Users/janbuschtoens/clark/application/client-packages/addons/ember-clark-sentry/app/app/initializers'
  - errorMessage: Build Canceled: Broccoli Builder ran into an error with `broccoli-persistent-filter:EslintValidationFilter` plugin. 💥

Hm, judging by the stack trace the other way around might make more sense? Seems like Prettier tries to resolve some config files.

[rwjblue]

Using the temp path doesn't really satisfy what eslint / prettier is trying to do...

[rwjblue]

There just isn't a "correct" way to find a files original source path from within a broccoli tree. Thats what is biting us here. The current implementation in broccoli-lint-eslint is attempting to figure it out, but is doing it wrong (it isn't taking into account the various options to broccoli-funnel and therefore it includes app/ twice..)

[buschtoens]

So the "fix" is taking all options into account?

Or should we add this "feature" to broccoli to expose the original source path?

[rwjblue]

A little bit of both I think 😺...

However, I’m quite happy with the solution I mentioned above: remove ember-cli-eslint and use eslint directly. It speeds up build time, reduces cognitive overhead, and let’s us leverage industry standard tooling instead of reinventing our own config parsing and caching system...

[dfreeman]

Have you considered weighing in over on ember-cli/rfcs#121? I've been leaning more and more toward the same conclusion (for all the reasons you mention), but opinions seem to be all over the spectrum over there 🙂

[rwjblue]

ZOMG thanks @dfreeman! I had completely missed that RFC 😢

[sumeetattree]

So how are fixing this? Just ran into the same build error. Remove ember-cli-eslint completely?

[jeanduplessis]

@sumeetattree we've gone with the route of just removing ember-cli-eslint in favour of running eslint manually as part of our CI pipeline and also enforcing eslint rules in a pre-commit hook.