@kentcdodds mentioned he鈥檚 not using eslint-loader because of perf issues.
Nobody reported them to us, so naturally we don鈥檛 know about them.
What kind of perf issues exist with eslint-loader?
Can you tell me how to reproduce them?
Does enabling that cache option (new in 1.5.0) help, and should we do it?
cc @andreypopp who might have opinions
I don't use CRA with large codebases yet so I don't have perf issues with eslint-loader.
I assume that ESLint's cache option only helps initial linting because otherwise eslint-loader is marked as cacheable and so it lints only changed modules (when in watch mode).
Here's an example. I have experienced this myself on bigger projects. This is why I generally keep eslint out of my webpack stuff and have it as a separate script that I run as a pre-push hook.
@kentcdodds So the issue is in production builds, or initial npm start? This shouldn鈥檛 affect rebuild times in development as far as I understand.
cc @ericclemmons
@gaearon Unfortunately I can't provide the copy/paste metrics (they were lost in chat long ago, but I can give you near exact info)
webpack build 5x, and averaged the results.eslint-loader.cache option at the time, so this is _without it_)eslint-loader as a preLoader:npm start took ~12s (down from ~18s). This is because babel-loader is the 2nd most intensive process in the app.npm start takes ~8s (just tested it repeatedly), now that most of what babel-loader does it cached.As a result, we did the following:
eslint-loader from the compilation process. Caching may help, but personally we found that _stylistic errors shouldn't break the build_ while _syntax errors should_, but those are quite visible in the IDE anyway.Also, I originally implemented DLLPlugin to resolve speed issues. (You may remember me mentioning it in another CRA thread) Unfortunately, it simply required too much orchestration (especially as the app & deps grew) for the speed gains.
Knowing what I know now, for speeding up builds it would be:
babel usage. Meaning, if you're running Chrome/Node v6, _you can do an entire build without Babel, until you need JSX support_. (Of course there are exceptions, but the complete compile down to ES5 is unnecessary IMO for RAD)DLLPlugin, as this is the closest you'll get to having your app be entirely just ./src.I hope this helps! We _may_ try this again with cache: true in eslint-loader, but I just don't think it's worth it for me.
Released in 0.9.1/2.
Reopening since we reverted this in 0.9.3.
Waiting for https://github.com/MoOx/eslint-loader/pull/159 to get in.
Anybody wants to help me to write the tests and do code review? Currently it doesn't pass any of the tests T_T. I've published a fork here https://www.npmjs.com/package/eslint-loader-fs-cache if anybody wants to play with it and compare with the older implementation.
Can you post a more detailed note of what needs to be done? So that a person who wants to help knows where to start, what you tried, and what the desired outcome is. https://github.com/MoOx/eslint-loader/pull/159 is probably the best place to do this.
Can we punt this to 0.10 @gaearon? I don't want to risk additional breakages if we forget an edge case (even though using babel's module is a safe bet, but I don't want to rely on it without support from babel too [as in babel switches to the new module]). We might explore an entire webpack caching plugin in 0.10 which will nullify the need for this anyway.
Sure.
The latest release of eslint-loader is broken because its new dependency (https://github.com/viankakrisna/loader-fs-cache) uses mkdirp but doesn't declare it as a dependency: https://github.com/viankakrisna/loader-fs-cache/pull/2
It's fixed on 1.0.1. whew! broke 2 project release because I wanted to contribute to eslint caching... T_T
It happens to the best of us, @viankakrisna. 馃槃
@Timer shall we turn it on again in master? 馃槃
I learn so much in these mishaps though. It's both stressful and rewarding working on open source... :)
Also, i'm curious about the whole
We might explore an entire webpack caching plugin in 0.10 which will nullify the need for this anyway.
Is it dll? Or any other one?
We were looking at https://github.com/jsdf/cached-loader. As for re-enabling this, I am hesitant because I'm about to go on vacation and I don't want to release a build right before I leave, haha.
Woow, that's cool! Yeah, have a nice vacation! I agree if cached-loader can do it, it's better to have it rather than make caching scattered across loader.
Let's reevaluate later.
I tried putting this behind cache loader and it didn't give a speed improvement; we'll need to try the built in cache option now if it's stable. No harm in flipping the flag back on.
Going to close as stale.
Most helpful comment
It's fixed on 1.0.1. whew! broke 2 project release because I wanted to contribute to eslint caching... T_T