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

[heft] Add options to only resolve symlinks that are within node_modules #5011

Merged
merged 9 commits into from
Nov 21, 2024

Conversation

dmichon-msft
Copy link
Contributor

Summary

TypeScript and Jest spend a lot of CPU cycles invoking realpath on inputs that don't contain any symbolic links. This PR adds a new package @rushstack/real-node-module-path that has similar semantics to realpath, but only performs file system calls if the path is under node_modules.

Details

Adds an option onlyResolveSymlinksInNodeModules to config/typescript.json that will replace the builtin realpath with the new package to accelerate initial file system crawling.

Adds a custom resolver @rushstack/heft-jest-plugin/lib/exports/jest-node-modules-symlink-resolver.js that replaces the realpathSync method within Jest's default resolver with the new package.

How it was tested

Unit tests for the behavior of the function.
Invoked from TypeScript and Jest in heft-webpack5-everything-test.
Enabled both options in local-node-rig so that projects in rushstack build with them.

Impacted documentation

Readmes, schema for typescript.json.

Copy link
Member

@D4N14L D4N14L left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this also an issue that Webpack would need a resolver for?

@dmichon-msft
Copy link
Contributor Author

Is this also an issue that Webpack would need a resolver for?

For webpack I'd rather use webpack-workspace-resolve-plugin, which annihilates realpath calls altogether. Unfortunately I still haven't quite gotten that working properly on Windows yet.

Copy link
Member

@D4N14L D4N14L left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks generally good, a couple comments

libraries/node-core-library/src/RealNodeModulePath.ts Outdated Show resolved Hide resolved
libraries/node-core-library/src/RealNodeModulePath.ts Outdated Show resolved Hide resolved
libraries/node-core-library/src/RealNodeModulePath.ts Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@dmichon-msft dmichon-msft merged commit 6f5f09d into microsoft:main Nov 21, 2024
4 checks passed
@dmichon-msft dmichon-msft deleted the real-node-module-path branch November 21, 2024 22:39
@dmichon-msft
Copy link
Contributor Author

To clarify, motivation was based on this:

Raw execution

image

Execution with access tracing enabled

image

Note the 10 seconds of realpath, which these changes work to mostly eliminate.

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

Successfully merging this pull request may close these issues.

3 participants