Add emitDependency caching and fix pnpm symlinks

[ijjk]

This removes the resolve caching introduced in #237 and replaces it with caching the entire emitDependency call so that all nested operations shouldn't need to be cache separately.

This provides much better performance than the previous caching as testing against a couple large apps showed these results:

• vercel.com: before(70 - 100 seconds) after(20 - 30 seconds)
• private project: before(170 - 200 seconds) after(50 - 60 seconds)

[codecov]

Codecov Report

Merging #238 (ec49f51) into main (d568f9a) will increase coverage by 0.03%.
The diff coverage is 86.44%.

❗ Current head ec49f51 differs from pull request most recent head b46709c. Consider uploading reports for the commit b46709c to get more accurate results

@@            Coverage Diff             @@
##             main     #238      +/-   ##
==========================================
+ Coverage   81.25%   81.28%   +0.03%     
==========================================
  Files          13       13              
  Lines        1472     1480       +8     
  Branches      545      546       +1     
==========================================
+ Hits         1196     1203       +7     
- Misses        109      110       +1     
  Partials      167      167              

Impacted Files	Coverage Δ
src/resolve-dependency.ts	69.81% <80.00%> (-2.12%)	⬇️
src/node-file-trace.ts	86.99% <86.36%> (+0.23%)	⬆️
src/analyze.ts	86.49% <100.00%> (ø)
src/utils/sharedlib-emit.ts	66.66% <100.00%> (-1.34%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d568f9a...b46709c. Read the comment docs.

[styfle]

Why use a proxy here?

[ijjk]

It's to allow us to catch all calls to emitFile even nested ones and add the files to the cache.

[styfle]

Nice work! 🎉

[guybedford]

Great improvements, and looks good to me, didn't read all the code but could just ask a few questions if it would help:

• How the gains are achieved here? If it is primarily disk, what were the misses on the previous PR? And if it is primarily due to compute, that would be very surprising since 1000s of dependencies should be fully iterable in well under 5 seconds.
• With the caching stuff it's also worth putting some thought to lifetimes - what's nice about the stat cache etc is they are easier to mutate / coordinate on for longer period caching, whereas approaches like these PRs move towards more opaque caching. Or possibly doing a separation of node_modules caching from user code caching. What sort of cache lifetime applies to these types of builds?

[ijjk]

It seems eliminating iterating over the dependencies reduces the time significantly on large projects e.g. thousands of dependencies with > 100 separate runs of nodeFileTrace sharing the same cache.

I think currently we should consider the emitDependency cache specific to one build as invalidating this cache would require checking all files in the cache entry and if any of them changed the parent entry needs to be invalidated. I'm not sure if we're attempting persisting this cache for multiple builds that could have had files change between them in any current uses of nft but if we are we can disable it by clearing the emitDependency cache between runs.

[guybedford]

It seems eliminating iterating over the dependencies reduces the time significantly on large projects e.g. thousands of dependencies with > 100 separate runs of nodeFileTrace sharing the same cache.

This really is very surprising to me, as it is fully possible to iterate 1000s of modules in well under a second. So there must be something else going on. Could be worth digging into some further analysis there, but this PR is good to land too of course.

[guybedford]

Perhaps there is some quadratic behaviour in the iteration?

[pi0]

Is this an expected behavior that includes symlink directories in fileList?