-
-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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
Haste2 #599
Haste2 #599
Conversation
cb5b5fb
to
f8e711c
Compare
The new module execution code is a little bit more ugly but it is super-hot code and yields a 10 sec perf win on www (~8.5 % of the entire runtime). |
omg it works. |
#dope |
5f8942d
to
96ab6e9
Compare
Outstanding list of TODOs:
Future release:
None of these should be blockers for merging this PR except for testing this at FB (no point in building this if it doesn't work on all of our tests ). I will try to run it on Monday and fix minor issues as they come up (assuming it won't blow up completely :) ). The first for TODO items are blockers for tagging a new version of jest itself, of course. |
Correct me if I'm wrong, but it looks like the (soon to be pluggable) module resolver still can only provide filepaths to resolved modules, and the file contents are actually loaded from disk by the From the perspective of being able to plug-in different module transformation+bundling pipelines (eg. webpack, browserify), with the goal of reducing the amount of duplicated config/machinery required to use Jest, it would be great to have the option of providing module sources as the output of the resolver, to be used, instead of only providing filepaths. In both browserify and webpack, resolving and transforming of modules is interleaved, and transforms/loaders are async, so the easiest place to plug them in would probably be in place of |
All the other stuff looks good though 😄 |
@jsdf thanks for the review! What would the ideal API look like to you? I take it webpack/browserify pre-transform code and then resolve them? I can imagine to merge the preprocessor and the resolver: resolving the dependencies for a path will pre-transform the file and apply the transform recursively for every dependency before resolving its dependencies. Now that there is only a single entry point for transform (there used to be three!) this should be easy to change. All that it needs is a map from real path to the cache path, which should be achievable using the |
High level description of all the changes:
Here is a breakdown of all the changes from oldest to latest: cpojer/jest@89481fb...cpojer:5f82cc75872ae02e97fcfa5f41039f8f6d6e3686
cpojer/jest@9a4e8d0...cpojer:000d0d9d6c53b64e5c25b9e2018c3035c8f50a4b
|
@cpojer your description of the combined preprocessor and resolver sounds right. For the webpack/browserify use case, the ideal API would be (most of which is already there):
|
Ok we are back in business. I just rebased this branch on top of all the work that went into jest since I started on this diff. Everything up until "Use |
I've published this PR as |
Gone from 70 tests in 26 tests suites running in ~21secs to ~9secs (after warming up) |
👏👏 |
Working great for us, also appears to have fixed #632 👏 |
I'm running tests with |
@irudoy can you try passing |
@cpojer it didn't help |
If you are willing to add tests to Jest that point out this issue you are seeing with your integration or sahre a test suite with a repro, I can try to look into this. |
Thanks! Note that 0.9 is not out yet and you should be safe using 0.8. Would you mind opening a separate issue on this repo with the details of this issue and what you'd expect to work that doesn't any more? :) |
Published jest@next again as 0.9.0-fb2 which should resolve the watcher issues. |
Summary:This fixes unmock resolution for node_modules when using npm3. The way this is solved is by checking whether the parent dependency is unmocked and both modules are within a `node_modules` folder. This has taken a while, mainly because it required #599 to be merged. I also added some more caching that makes test runs even faster. The react-native test suite now completes in about 6s (down from 7.5-8s). There might be more in the near future ;) This fixes #554, #730, facebook/react#5183, facebook/relay#832 and will be part of 0.9.0. I'll publish 0.9.0-fb3 today with this fix. Closes #732 Differential Revision: D2959610 fb-gh-sync-id: c374b7a2bcdfddf768905356a08948d9156eb028 shipit-source-id: c374b7a2bcdfddf768905356a08948d9156eb028
Ohhh, there is an open task to improve caching when coverage is being used. If anyone wants to work on that, it should improve test runs with coverage by 20-50 %. |
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
This pull request is used to track the implementation of node-haste2.
Summary
--onlyChanged
that should work much better and find the correct set of related tests to run.Performance Improvements
This diff/PR forms the basis for all the work on Jest that I'm planning to do in the near term and will set up Jest for further success. It should be much easier to iterate on Jest from now on given that we have replaced the biggest legacy system and I can finally focus on solving some of the community problems.