Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Watch Mode Plugin: jest-runner-eslint/watch #43

Closed
azz opened this issue May 31, 2018 · 15 comments · Fixed by #53
Closed

Watch Mode Plugin: jest-runner-eslint/watch #43

azz opened this issue May 31, 2018 · 15 comments · Fixed by #53

Comments

@azz
Copy link
Contributor

azz commented May 31, 2018

Combining jestjs/jest#5212 and the new Jest Watch Plugins API, it would be really cool to have:

{
  "jest": {
    "runner": "jest-runner-eslint",
    "watchPlugins": [
      "jest-runner-eslint/watch"
    ]
  }
}

Which, when an ESLint error is reported, exposes an option in watch mode:

Press F to run eslint --fix

I think it makes sense for it to live in this repo, but it could equally be a standalone repo.

I'd be happy to work on a PR if it is desired here 😄

Related: #10

@ljharb
Copy link
Collaborator

ljharb commented May 31, 2018

I think it makes sense here.

@rogeliog
Copy link
Member

rogeliog commented May 31, 2018

Yes, this sounds really interesting! Currently, I don't think that there is a good way of sending that information from the plugin to the runner. It would be awesome find a way that could enable these type of features!

@rogeliog
Copy link
Member

I think I have some vague idea, I'll try to hack something today

@rogeliog
Copy link
Member

rogeliog commented Jun 1, 2018

I think having it in the same repo would make a lot of sense, given that it is a watch plugin really specific to this runner.

Here is a PoC:

There are still a couple of hiccups with the implementation, but I first wanted to be on the same page from a product perspective.

It is still pretty rough but I would love to get your thoughts 🙃@ljharb, @azz

  • I added single entrypoint for the watch plugin and then a list of options that could be "modified". I hope that this would allow the plugin to scale better. The UI is similar to https://github.com/rogeliog/jest-watch-select-projects.
  • I was wondering what would be other interesting options, that we would like to add to this?
    • --fix is implemented and working
    • --quiet is still mocked, and does not do anything
  • Given that they both live in the same repo, we can share state way easier, which is how the plugin and runner are communicating.
  • In order to achieve this there are some small changes that are needed in create-jest-runner(to pass extra options to the runner) and there are no changes required infacebook/jest.

watch-eslint

@SimenB any thoughts on this?

@SimenB
Copy link
Member

SimenB commented Jun 1, 2018

This is insanely awesome! I think it'd be cool to attach it to -u, but maybe not?

there are no changes required infacebook/jest

😍

@azz
Copy link
Contributor Author

azz commented Jun 2, 2018

Nice! I can't think of any other ESLint flags that I would want to change during a run.

@ljharb
Copy link
Collaborator

ljharb commented Jun 2, 2018

What about ext?

@ljharb
Copy link
Collaborator

ljharb commented Jun 2, 2018

I’d also see the value in being able to enable/disable/reconfigure rules during the run.

@azz
Copy link
Contributor Author

azz commented Jun 2, 2018

What about ext?

As in switching from all to just .js? What's the use case for doing that?

I’d also see the value in being able to enable/disable/reconfigure rules during the run.

That could be tricky if you want to persist back to disk. Would need to locate config file and patch it. Easy for JSON, YAML and standard module.exports = {} config files, but the JS files can be dynamic.

@ljharb
Copy link
Collaborator

ljharb commented Jun 2, 2018

Or from .js to .js,.jsx etc.

Why would you need to patch the config file? eslint on the CLI supports --rule

@azz
Copy link
Contributor Author

azz commented Jun 2, 2018

Oh I see what you mean - ephemeral overrides, like if you want to ignore no-console. I'd use that.

@rogeliog
Copy link
Member

rogeliog commented Jun 7, 2018

Here is a quick prototype with "ephemeral overrides", thoughts?

NOTE: My prototype is just a node script, which is why it says "Back to Jest and running tests..."

aaaaa

@ljharb
Copy link
Collaborator

ljharb commented Jun 7, 2018

That looks great for rules! what about other things like parserOptions, env, globals, "settings", things like that?

@rogeliog
Copy link
Member

rogeliog commented Jun 7, 2018

I think we could extend it as much as we want, I was thinking of choosing 2 or 3 things and polish them for the first release of the watcher(and add more later). I don't have a strong opinion about what these 2 or 3 things should be. cc: Any thoughts @ljharb @azz @SimenB?

@SimenB
Copy link
Member

SimenB commented Jun 7, 2018

I think messing with rules makes the most sense as a first iteration - parser and plugin stuff is more of a permanent change. Fine to revisit, of course, but a first iteration doesn't need it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants