Support ESLint "--cache" option

[rondale-sc]

Inspired by a talk at Emberconf I though it might be a good idea to enable eslint caching. I expected this to be a pretty simple config option in eslintrc.js or likewise, but it seems that it is a bit more complicated

I think we can likely accomplish this by merging options in the lintTree func and passing those all the way down to eslint

https://github.com/ember-cli/ember-cli-eslint/blob/master/index.js#L34

https://github.com/ember-cli/broccoli-lint-eslint/blob/master/lib/index.js#L41

https://github.com/ember-cli/broccoli-lint-eslint/blob/master/lib/index.js#L72

https://github.com/eslint/eslint/blob/master/lib/cli-engine.js#L638

I'd like to work on this, but I really wanted to get some feedback before diving in any further. Is this a good idea? Are there potential problems or incompatibilities that would prevent us from doing this?

[dbashford]

I also came here to look into this because of the EmberConf talk. Big 👍 from me.

[Turbo87]

@rondale-sc tl;dr this will require deep changes in the implementation of broccoli-lint-eslint.

right now broccoli-lint-eslint works roughly like Array.map() in that it reads all *.js files and creates corresponding *.lint-test.js files by running each file through ESLint. to make use of --cache we should probably run ESLint only once but on all *.js files which will likely require a change in the base class used by broccoli-lint-eslint.

@stefanpenner @rwjblue @hjdivad please correct me if I'm wrong

[hjdivad]

What's the advantage of using eslint's --cache over the async caching from broccoli-persistent-filter?

[Turbo87]

What's the advantage of using eslint's --cache over the async caching from broccoli-persistent-filter?

1. currently when using ember serve ESLint is run once on startup and shows you all the existing linting issues on the console. once you fix an issue that file will be passed through to ESLint again, but none of the others since they haven't changed. that means that the remaining issues are also not printed to the console again. with eslint --cache the cached issues would be printed again.

2. it appears that even though we use broccoli-persistent-filter ESLint is run on all files whenever you startup ember serve. which in the current case might be useful, because otherwise you won't actually see any console output since no files had changed between runs.

[stefanpenner]

currently when using ember serve ESLint is run once on startup and shows you all the existing linting issues on the console. once you fix an issue that file will be passed through to ESLint again, but none of the others since they haven't changed. that means that the remaining issues are also not printed to the console again. with eslint --cache the cached issues would be printed again.

I thought we fixed this.

it appears that even though we use broccoli-persistent-filter ESLint is run on all files whenever you startup ember serve. which in the current case might be useful, because otherwise you won't actually see any console output since no files had changed between runs.

I believe it is intentional, and may the fix i thought fixed 1.

[stefanpenner]

Experimenting with eslints own caching may be a good idea. I'm curious as to the downsides.

[rondale-sc]

@Turbo87 So you think the changes need to occur in broccoli-lint-eslint then? If so, should I begin work there?

Also, I'm not entirely certain how to start with this. I suspect it will involve getting a broccoli pipeline in place and create a little repo that would facilitate easy testing? Would benchmarking be helpful?

[Turbo87]

So you think the changes need to occur in broccoli-lint-eslint then? If so, should I begin work there?

ember-cli-eslint should just be a very thin wrapper around broccoli-lint-eslint. essentially all it should do is provide it with the right options (e.g. choosing the correct test generator depending on whether ember-cli-mocha or ember-cli-qunit is used).

I suspect it will involve getting a broccoli pipeline in place and create a little repo that would facilitate easy testing?

ember-cli/broccoli-lint-eslint#90 should give you an idea on how to best test broccoli plugins

[rwjblue]

I thought we fixed this.

@stefanpenner - Yes, we cache the output of each file's linting and report it back to the console for a warm build.

it appears that even though we use broccoli-persistent-filter ESLint is run on all files whenever you startup ember serve.

This is what I am referring to above, we do not run eslint unless the cache has been busted. We do however print the same error/warning messages to the console as we emitted the last time the file was linted.

---

I think it is probably worth while to discuss what we want, and less about how to get there. For example, if what we want is that all warnings/errors print out (for all files even for those not changed) for every rebuild this should be pretty straight forward..

[Turbo87]

@rwjblue @stefanpenner it's possible that I misunderstood the current implementation. I thought only the generated test files were persisted between builds.

[rwjblue]

I thought only the generated test files were persisted between builds.

Nope. Checkout:

• Add postProcess method. broccolijs/broccoli-persistent-filter#50
• https://github.com/rwjblue/ember-cli-template-lint/pull/44
• Print errors properly when cached. rwjblue/broccoli-jshint#42
• Use broccoli-persistent-filter broccoli-lint-eslint#21 (migrates to broccoli-persistent-filter)

[rondale-sc]

@rwjblue So ostensibly this already happens and the only discussion is whether we make it more obvious that it is caching by showing only changed files lint output?

[Gaurav0]

I'd like to pass the --quiet option to eslint so as to not display warnings during CI builds. I hope a fix for this would be able to apply that as well.

[petermoore14]

Bumping this as it would be extremely nice to have. Unclear why the rest of broccoli-lint-eslint's options aren't supported either, should add those as well.