Add typeahead to watch mode
@cpojer I found #2251 really interesting, so I started working on it just to get a bit more familiar with the internals of Jest, of which I know nothing.
This is a really primitive prototype, which still has several issues that need to be address, as well as testing.
This is the typeahead running in the [React](https://github.com/facebook/react) repo
In the issue you mentioned that there is some refactor that will make this easier to accomplish, so I guess that this might not be the right way of implementing it:
> may be easier to accomplish once we do the following:
> * Split jest's watch feature into a separate package and refactor it.
Assuming that we want to move forward with the feature here are a couple of things that I'm still missing
- [x] A better understanding of Jest's architecture to figure out if that is the right place to put that logic.
- [x] Figure out the appropriate user experience for it.
- [x] Handling error cases and some keys that break it.
- [x] Handle pluralization(file vs files)
- [x] Handle screen resizing
- [x] Add tests to the typeahead.
- [x] Some weird cursor positioning
- [x] How expensive is `Runtime.createHasteContext`? Is it something that I should be caching?
Lol I literally just saw your comment on the issue and there is already a PR. Oh man. Let me see…
Ok, so this is really cool! You did point out the question about how expensive creating an instance of `jest-haste-map` is (through `Runtime.createHasteContext`) and this is really what this comes down to and why I recommended the refactor: In our internal codebases this takes upwards of 1-3 seconds, which is crazy long.
However, we implemented file-watching in `jest-haste-map` (using a watch argument) and we can actually change our watch mode here to use that, which will make the watch mode a ton faster for any mid to large project and will help us massively at FB.
Originally I was thinking the watch mode should be its own separate package but I don't think we need to go that far. If we split out the watch mode into a separate file within `jest-cli`, I'm fine with it too. You were able to figure out how it works really well but the current implementation is quite a mess and all the CLI code is just mixed in this one `jest.js` file. I don't have a strong opinion whether it should be its own package or not but the potential for it is that other test frameworks could potentially use the `jest-watch` package and get the same watch mode that we have, which would be pretty awesome!
So, to lay out the steps:
* Refactor this watch mode into its own file.
* Currently jest.js has its own watch feature and creates a haste context object inside of `runJest`. We need to invert this to create the haste context based on the configuration, set the watch flag to true in jest-haste-map and then use the change event to re-run Jest. This will also allow us to remove a bunch of duplicated code in jest.js.
* Once we inverted this, running tests after a file change will be instant because we won't have to wait for jest-haste-map to reconcile with the file system. Basically instead of re-creating a haste context for each run, we'll have one in memory and get an updated version of `hasteFS` and `moduleMap` and a list of changed files with every file change event.
See https://github.com/facebook/react-native/blob/master/packager/react-packager/src/node-haste/index.js#L211-L216 for how the watch feature is used in react-native's packager.
After that we can consider:
* How do we polish this typeahead? It may even make sense to get rid of the "usage" or condense the usage information, which can get pretty long and annoying.
* How do we test all of this? Currently it is all manual; in the past we were thinking we could potentially use snapshot tests for this.
* I think it would be pretty cool if we could highlight the matched parts in the test names with color, what do you think?
Again, I'd like to invite you to our discord channel ( http://facebook.github.io/jest/support.html ) and I'm also happy to have a chat over google hangouts or something to walk you through in person.
Let me know what you think about this plan! I love that you started on this and this refactor is going to make Jest a ton faster for engineers at FB :)
## [Current coverage](https://codecov.io/gh/facebook/jest/pull/2324?src=pr) is 89.25% (diff: 100%)
> Merging [#2324](https://codecov.io/gh/facebook/jest/pull/2324?src=pr) into [master](https://codecov.io/gh/facebook/jest/branch/master?src=pr) will not change coverage
@@ master #2324 diff @@
Files 39 39
Lines 1471 1471
Methods 0 0
Messages 0 0
Branches 0 0
Hits 1313 1313
Misses 158 158
Partials 0 0
> Powered by [Codecov](https://codecov.io?src=pr). Last update [6501ca0...ee7aabb](https://codecov.io/gh/facebook/jest/compare/6501ca01b31dbe005545e0e50913da0ae17c837f...ee7aabb2b9e07fd984c47bb833461e618f25e58d?src=pr)
Thanks a lot, this feedback is super helpful, I'll definitely reach out if needed :)
Alright, now that watch mode was rewritten, I believe we can pretty easily make this work with a great experience!
* I'd recommend against a `testPatternTypeahead` option for now.
* Can we highlight the matched parts in the typeahead? @thymikee is an expert on pretty cli printing now. Any suggestions?
* cc @gaearon and @tomocchino who are UX experts on this. Do you have any thoughts about what this should feel like?
I'm a big fan of highlighting search phrases. I believe yellow bg with dark text would be more than enough here, just like with Ctrl+F on Chrome
That may work well, yeah! The other option I can think of is dim vs reset for matches, which is more subtle.
Here are some gifs of how each of the highlighting options could look :)
👍 for the highlighting option. The dimming is clean but a bit too subtle I think. Also not sure how it would look on a light terminal background
I would personally prefer the less aggressive dimming.
(I got a bit confused by the Twitter poll and what everybody referred to as “highlighting”.)
Chrome does highlighting because you need to find these words *between other words* on the page. In this case, all results are displayed below, and you don’t really need to interact with them anyway. I’m not convinced we need to use a bright color here but YMMV.
I agree with @gaearon. I think dimming is fine for this.
I'm also happier with dimming/highlighting text instead of background highlight 🙂
Alright, @rogeliog let's go with the dimming for now and see how people react to the next release.
Please ensure that
are enabled on:
Generated by :no_entry_sign:
Alright I think this looks good. The one problem I noticed is when my terminal output pushes me down to the bottom of the screen because of some failures. Typing feels a bit odd then, not sure if we can fix that? Shall we wipe away everything on the screen when entering pattern mode?
@rogeliog was there anything else you wanted to do on this diff?
@cpojer I think it is in a good shape now, apart of the issue when there is not enough room in the terminal. We probably do need to find a good solution for it before merging, because it currently breaks really badly 😞
Do you have any ideas for that one? What about clearing the terminal when entering pattern mode?