perf: add TSGO_BINARY env var and fast-path sibling check in getExePath#3451
perf: add TSGO_BINARY env var and fast-path sibling check in getExePath#3451nileshpatil6 wants to merge 2 commits intomicrosoft:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR improves tsgo startup performance in monorepos by avoiding expensive module resolution in the native-preview launcher’s executable path lookup.
Changes:
- Add
TSGO_BINARYenv var override to bypass resolution and return a user-specified executable path. - Add a fast-path sibling lookup for the platform package (
@typescript/native-preview-{platform}-{arch}) before falling back toimport.meta.resolve.
| if (envPath) { | ||
| if (!fs.existsSync(envPath)) { | ||
| throw new Error("TSGO_BINARY points to non-existent file: " + envPath); | ||
| } |
There was a problem hiding this comment.
The TSGO_BINARY validation only checks fs.existsSync(envPath), but the error says it points to a non-existent file. existsSync also returns true for directories, so a directory (or other non-file) value will bypass this check and then fail later with a less clear exec error. Consider either validating that the path is a regular file (and possibly executable) or adjusting the error message to match what’s actually being checked.
| } | |
| } | |
| const envStat = fs.statSync(envPath); | |
| if (!envStat.isFile()) { | |
| throw new Error("TSGO_BINARY does not point to a file: " + envPath); | |
| } |
| const envPath = process.env.TSGO_BINARY; | ||
| if (envPath) { | ||
| if (!fs.existsSync(envPath)) { | ||
| throw new Error("TSGO_BINARY points to non-existent file: " + envPath); | ||
| } | ||
| return envPath; | ||
| } |
There was a problem hiding this comment.
On Windows, the normal resolution path applies a \\?\ long-path prefix when the resolved executable path is long. The TSGO_BINARY fast-path returns the env var value verbatim, so it can reintroduce long-path failures for the same setups this code is trying to support. Consider applying the same Windows long-path handling (and any other normalization you rely on later) before returning envPath.
…dows long-path prefix
|
If someone has this path, why wouldn't they just run it directly? This seems really special case-y. |
Fixes the performance issue raised in #2836 where
import.meta.resolveis called on every separate tsgo invocation in large monorepos.Two changes in
getExePath.js:1.
TSGO_BINARYenvironment variableIf set, the function returns that path immediately and skips all resolution. This is useful for monorepo setups where users want to hardcode the binary location (e.g. in their shell config or CI environment). It also works as a diagnostic tool to verify whether path resolution is the actual bottleneck.
If the path does not exist, a clear error is thrown rather than silently failing.
2. Fast-path sibling check before
import.meta.resolveIn the installed package branch, the code now checks if the platform binary exists at the predictable sibling location (
../../{expectedPackage}/lib/) using a singlefs.existsSynccall before falling back toimport.meta.resolve. This covers the standard npm/yarn layout where both@typescript/native-previewand@typescript/native-preview-{platform}-{arch}sit side by side undernode_modules/@typescript/. If the sibling is not found there (e.g. unusual hoisting), it falls through to the existingimport.meta.resolvelogic unchanged.The path calculation is the same as the one already used in the
built/npmbranch, so it is not new math.Testing
I wrote a local test suite covering these cases before opening this PR:
TSGO_BINARYpointing to a real file returns it correctlyTSGO_BINARYpointing to a missing file throws with a clear messagenode_modules/@typescript/native-preview/libdirectory/_packages/native-preview/lib) detection still works/built/npm/native-preview/lib) detection still worksAll 7 tests passed on Node 22.