Watch plugin for toggling --fix #53
Conversation
There was a problem hiding this comment.
This is awesome!
I agree configOverrides is weird. I think we really need jestjs/jest#4278, and make sure that watch plugins are able read it, and vice versa.
Probably some shared config spot for custom stuff so we don't pollute globalConfig, but still allow custom runners and plugins to communicate and have their own config without resorting to weird stateful modules and manual cosmiconfig
src/watchFixPlugin/index.js
Outdated
| }; | ||
| } | ||
|
|
||
| async _showLoading(fix) { |
There was a problem hiding this comment.
I added that loading indicator, because I felt that it provided a smoother experience (See the bottom part of the gif in the PR description) I'm happy to remove it you feel like it would be better without it.
There was a problem hiding this comment.
It looks slick, but does it do anything? Other than delay for a tiny bit
There was a problem hiding this comment.
The only reason was to provide an indicator and feedback of what is happening, but I guess you are right... none of the other Jest keys does that, I just removed it.
| @@ -24,7 +24,7 @@ const runESLintRunnerWithMockedEngine = options => { | |||
| }); | |||
| const runESLint = require('../runESLint'); | |||
|
|
|||
| return runESLint(options.runESLint); | |||
| return runESLint(Object.assign({ extraOptions: {} }, options.runESLint)); | |||
README.md
Outdated
| @@ -172,6 +193,8 @@ module.exports = { | |||
| } | |||
| ``` | |||
|
|
|||
| ### | |||
src/runner/runESLint.js
Outdated
| fix: getComputedFixValue(cliOptions), | ||
| }), | ||
| const { cliOptions: baseCliOptions } = getESLintOptions(config); | ||
| const cliOptions = Object.assign( |
|
|
||
| const runAndAwait = async plugin => { | ||
| const runPromise = plugin.run(); | ||
| await jest.runTimersToTime(700); |
There was a problem hiding this comment.
we have runAllPendingTimers and runAllTimers instead of hardcoding the values
There was a problem hiding this comment.
They also don't return promises, so you can drop the await
rogeliog
force-pushed
the
eslint-watch-plugin
branch
from
8ba5cb6 to
c64f70a
Compare
|
@ljharb Any thoughts? |
package.json
Outdated
| @@ -1,7 +1,7 @@ | |||
| { | |||
| "name": "jest-runner-eslint", | |||
| "version": "0.6.0", | |||
| "main": "build/index.js", | |||
| "main": "build/runner/index.js", | |||
There was a problem hiding this comment.
this can just be build/runner
src/watchFixPlugin/index.js
Outdated
| } | ||
|
|
||
| // eslint-disable-next-line class-methods-use-this | ||
| _getPrompt() { |
There was a problem hiding this comment.
why can’t this be a closed over function instead of a fake private method?
| @@ -15,23 +15,22 @@ class ESLintWatchFixPlugin { | |||
| } | |||
|
|
|||
| getUsageInfo() { | |||
| const getPrompt = () => { | |||
There was a problem hiding this comment.
if this is defined at module level, then it should be a bit faster.
|
(we should do #54 before releasing this, BTW) |
|
@SimenB Perfect! |
Fixes #43
I was originally thinking of a more fully featured watch plugin #43 (comment), but I was not happy with that UX.
Given that
--fixis the most common request, I think it makes sense to start with a--fixplugin, and if need be we can work on another one later.jest-runner-eslint/watch-fixinstead ofjest-runner-eslint/watchto leave room for more plugins.configOverrides, I would love to get some feedback on this. I'm not a fan of it, but we need a way that they can both access shared state... Other ideas are more than welcome.--fixis in the config. Given this, I decided to show a different message until the user enables the toggling. I think that the UX is still fine.