Worker: Babel ignore node_modules by default + some minor babel transform perf improvements #435
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I ended up digging a bit into the performance here since I noticed that with newer versions of
jscodeshift
, what used to be quick (under Jests 5s timeout) was now consistently taking over 10 seconds.(Aside, it actually was taking closer to 30s when I first started, but I believe this was due to an insanely large default
BABEL_CACHE_PATH
that consistently took over 5s to read from disk. Deleting that was my biggest perf win by far)For the purposes of my testing, I was running a jest test that also happened to run jscodeshift via the node API. This task also imported several larger dependencies.
After the cache was cleared, via performance timing in the
setup
function, this function took an average of 14.157 seconds across 5 runs (13.661, 13.479, 13.438, 15.136, 15.071) with a hot cache and 11.408 seconds across 5 runs (12.133, 11.419, 11.321, 11.201, 10.965) with the cache disabled with theBABEL_DISABLE_CACHE
env variable.I'll probably continue to use
BABEL_DISABLE_CACHE
in my tooling going forward due to it seemingly being a performance improvement whenbabel/register
is used like this, so all numbers going forward will make the assumption that it is being used.The swap to require
plugins
beforebabel/register
installed it hooks and reverting those hooks after the transform were required aren't too significant, but seemed worth including since I already had tested them locally. They reduced the average time to 10.571 seconds across 5 runs (10.769, 10.884, 10.379, 10.510, 10.312), a 7.3% decrease. Both these changes relate to just avoiding havingrequire
logic go throughbabel/register
if it doesn't need to, either be requiring it before it hooks in, or removing the hooks before furtherrequire
s occur.The bigger, and more controversial change, is adding
/node_modules/
to theignore
list. This was proposed in #294 (comment), but I'm not sure if there is more context there. By defaultbabel/register
would ignore thenode_modules
of the cwd, but we explicitly want to compile files outside of the cwd here 🤔 . Should this be scoped to the transforms directory? Anynode_modules
in parent directories of the transform? Allnode_modules
? Should it be configurable instead?For my transform specifically, this reduced the
setup
time down to an average of 2.536 seconds across 5 runs (2.866, 3.169, 2.553, 1.632, 2.458), and additional 76% decrease from before. Though this is obviously dependent what dependencies a transform is pulling inI'm happy to split the
/node_modules/
change into a separate PR for further discussion if desired, but figured I'd put it all here for now since they are a bit related.