Skip to content

Commit

Permalink
[heft] Add options to only resolve symlinks that are within node_modu…
Browse files Browse the repository at this point in the history
…les (#5011)

* [node-core-library] Add RealNodeModulePathResolver

* [heft-typescript] Add onlyResolveSymlinksInNodeModules option

* [heft-jest] Add node module symlink resolver

* Test resolver

* Use features in local-node-rig

* Fix duplicate types

* Disable CI fail-fast

* Pay extra lstat

* Fix unit tests

---------

Co-authored-by: David Michon <[email protected]>
  • Loading branch information
dmichon-msft and dmichon-msft authored Nov 21, 2024
1 parent b8f1b2b commit 6f5f09d
Show file tree
Hide file tree
Showing 21 changed files with 649 additions and 22 deletions.
1 change: 1 addition & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ on:
jobs:
build:
strategy:
fail-fast: false
matrix:
include:
- NodeVersion: 18.18.x
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,5 +7,6 @@
"coverageReporters": ["cobertura", "html"],

// Use v8 coverage provider to avoid Babel
"coverageProvider": "v8"
"coverageProvider": "v8",
"resolver": "@rushstack/heft-jest-plugin/lib/exports/jest-node-modules-symlink-resolver"
}
Original file line number Diff line number Diff line change
Expand Up @@ -49,5 +49,7 @@
// "excludeGlobs": [
// "some/path/*.css"
// ]
}
},

"onlyResolveSymlinksInNodeModules": true
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"changes": [
{
"packageName": "@rushstack/heft-jest-plugin",
"comment": "Add a custom resolver that only resolves symlinks that are within node_modules.",
"type": "minor"
}
],
"packageName": "@rushstack/heft-jest-plugin"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"changes": [
{
"packageName": "@rushstack/heft-typescript-plugin",
"comment": "Add \"onlyResolveSymlinksInNodeModules\" option to improve performance for typical repository layouts.",
"type": "minor"
}
],
"packageName": "@rushstack/heft-typescript-plugin"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"changes": [
{
"packageName": "@rushstack/node-core-library",
"comment": "Add `RealNodeModulePathResolver` class to get equivalent behavior to `realpath` with fewer system calls (and therefore higher performance) in the typical scenario where the only symlinks in the repository are inside of `node_modules` folders and are links to package folders.",
"type": "minor"
}
],
"packageName": "@rushstack/node-core-library"
}
4 changes: 4 additions & 0 deletions common/config/rush/nonbrowser-approved-packages.json
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,10 @@
"name": "@rushstack/package-extractor",
"allowedCategories": [ "libraries", "vscode-extensions" ]
},
{
"name": "@rushstack/real-node-module-path",
"allowedCategories": [ "libraries" ]
},
{
"name": "@rushstack/rig-package",
"allowedCategories": [ "libraries" ]
Expand Down
1 change: 1 addition & 0 deletions common/reviews/api/heft-typescript-plugin.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ export interface ITypeScriptConfigurationJson {
buildProjectReferences?: boolean;
emitCjsExtensionForCommonJS?: boolean | undefined;
emitMjsExtensionForESModule?: boolean | undefined;
onlyResolveSymlinksInNodeModules?: boolean;
// (undocumented)
project?: string;
staticAssetsToCopy?: IStaticAssetsCopyConfiguration;
Expand Down
22 changes: 19 additions & 3 deletions common/reviews/api/node-core-library.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@
/// <reference types="node" />

import * as child_process from 'child_process';
import * as fs from 'fs';
import * as nodeFs from 'fs';
import * as nodePath from 'path';

// @public
export enum AlreadyExistsBehavior {
Expand Down Expand Up @@ -213,7 +214,7 @@ export type FileSystemCopyFilesAsyncFilter = (sourcePath: string, destinationPat
export type FileSystemCopyFilesFilter = (sourcePath: string, destinationPath: string) => boolean;

// @public
export type FileSystemStats = fs.Stats;
export type FileSystemStats = nodeFs.Stats;

// @public
export class FileWriter {
Expand All @@ -231,7 +232,7 @@ export const FolderConstants: {
};

// @public
export type FolderItem = fs.Dirent;
export type FolderItem = nodeFs.Dirent;

// @public
export interface IAsyncParallelismOptions {
Expand Down Expand Up @@ -605,6 +606,14 @@ export interface IReadLinesFromIterableOptions {
ignoreEmptyLines?: boolean;
}

// @public
export interface IRealNodeModulePathResolverOptions {
// (undocumented)
fs: Pick<typeof nodeFs, 'lstatSync' | 'readlinkSync'>;
// (undocumented)
path: Pick<typeof nodePath, 'isAbsolute' | 'normalize' | 'resolve' | 'sep'>;
}

// @public (undocumented)
export interface IRunWithRetriesOptions<TResult> {
// (undocumented)
Expand Down Expand Up @@ -834,6 +843,13 @@ export class ProtectableMap<K, V> {
get size(): number;
}

// @public
export class RealNodeModulePathResolver {
constructor(options?: IRealNodeModulePathResolverOptions);
clearCache(): void;
readonly realNodeModulePath: (input: string) => string;
}

// @public
export class Sort {
static compareByValue(x: any, y: any): number;
Expand Down
17 changes: 17 additions & 0 deletions common/reviews/api/real-node-module-path.api.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
## API Report File for "@rushstack/real-node-module-path"

> Do not edit this file. It is a report generated by [API Extractor](https://api-extractor.com/).
```ts

/// <reference types="node" />

// @public
export function clearCache(): void;

// @public
export const realNodeModulePath: (input: string) => string;

// (No @packageDocumentation comment for this package)

```
36 changes: 36 additions & 0 deletions heft-plugins/heft-jest-plugin/src/JestRealPathPatch.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
// Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license.
// See LICENSE in the project root for license information.

import * as path from 'node:path';
import { RealNodeModulePathResolver } from '@rushstack/node-core-library/lib/RealNodeModulePath';

const jestResolvePackageFolder: string = path.dirname(require.resolve('jest-resolve/package.json'));
const jestResolveFileWalkersPath: string = path.resolve(jestResolvePackageFolder, './build/fileWalkers.js');

const jestUtilPackageFolder: string = path.dirname(
require.resolve('jest-util/package.json', { paths: [jestResolvePackageFolder] })
);
const jestUtilTryRealpathPath: string = path.resolve(jestUtilPackageFolder, './build/tryRealpath.js');

const { realNodeModulePath }: RealNodeModulePathResolver = new RealNodeModulePathResolver();

const fileWalkersModule: {
realpathSync: (filePath: string) => string;
} = require(jestResolveFileWalkersPath);
fileWalkersModule.realpathSync = realNodeModulePath;

const tryRealpathModule: {
default: (filePath: string) => string;
} = require(jestUtilTryRealpathPath);
tryRealpathModule.default = (input: string): string => {
try {
return realNodeModulePath(input);
} catch (error) {
// Not using the helper from FileSystem here because this code loads in every Jest worker process
// and FileSystem has a lot of extra dependencies
if (error.code !== 'ENOENT' && error.code !== 'EISDIR') {
throw error;
}
}
return input;
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
// Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license.
// See LICENSE in the project root for license information.

import '../JestRealPathPatch';
// Using this syntax because HeftJestResolver uses `export =` syntax.
import resolver = require('../HeftJestResolver');
export = resolver;
70 changes: 55 additions & 15 deletions heft-plugins/heft-typescript-plugin/src/TypeScriptBuilder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,21 @@ import { Worker } from 'worker_threads';

import * as semver from 'semver';
import type * as TTypescript from 'typescript';
import { JsonFile, type IPackageJson, Path, FileError } from '@rushstack/node-core-library';
import {
JsonFile,
type IPackageJson,
Path,
FileError,
RealNodeModulePathResolver
} from '@rushstack/node-core-library';
import type { ITerminal } from '@rushstack/terminal';
import type { IScopedLogger } from '@rushstack/heft';

import type {
ExtendedBuilderProgram,
ExtendedTypeScript,
IExtendedSolutionBuilder
IExtendedSolutionBuilder,
ITypeScriptNodeSystem
} from './internalTypings/TypeScriptInternals';
import type { ITypeScriptConfigurationJson } from './TypeScriptPlugin';
import type { PerformanceMeasurer } from './Performance';
Expand Down Expand Up @@ -314,14 +321,41 @@ export class TypeScriptBuilder {
return timeout;
};

let realpath: typeof ts.sys.realpath = ts.sys.realpath;
if (this._configuration.onlyResolveSymlinksInNodeModules) {
const resolver: RealNodeModulePathResolver = new RealNodeModulePathResolver();
realpath = resolver.realNodeModulePath;
}

const getCurrentDirectory: () => string = () => this._configuration.buildFolderPath;

// Need to also update watchFile and watchDirectory
const system: TTypescript.System = {
const system: ITypeScriptNodeSystem = {
...ts.sys,
getCurrentDirectory: () => this._configuration.buildFolderPath,
realpath,
getCurrentDirectory,
clearTimeout,
setTimeout
};

if (realpath && system.getAccessibleFileSystemEntries) {
const { getAccessibleFileSystemEntries } = system;
system.readDirectory = (folderPath, extensions, exclude, include, depth): string[] => {
return ts.matchFiles(
folderPath,
extensions,
exclude,
include,
ts.sys.useCaseSensitiveFileNames,
getCurrentDirectory(),
depth,
getAccessibleFileSystemEntries,
realpath,
ts.sys.directoryExists
);
};
}

this._tool = {
ts,
system,
Expand Down Expand Up @@ -376,7 +410,7 @@ export class TypeScriptBuilder {
if (!tool.solutionBuilder && !tool.watchProgram) {
//#region CONFIGURE
const { duration: configureDurationMs, tsconfig } = measureTsPerformance('Configure', () => {
const _tsconfig: TTypescript.ParsedCommandLine = this._loadTsconfig(ts);
const _tsconfig: TTypescript.ParsedCommandLine = this._loadTsconfig(tool);
this._validateTsconfig(ts, _tsconfig);

return {
Expand Down Expand Up @@ -430,7 +464,7 @@ export class TypeScriptBuilder {
tsconfig,
compilerHost
} = measureTsPerformance('Configure', () => {
const _tsconfig: TTypescript.ParsedCommandLine = this._loadTsconfig(ts);
const _tsconfig: TTypescript.ParsedCommandLine = this._loadTsconfig(tool);
this._validateTsconfig(ts, _tsconfig);

const _compilerHost: TTypescript.CompilerHost = this._buildIncrementalCompilerHost(tool, _tsconfig);
Expand Down Expand Up @@ -557,7 +591,7 @@ export class TypeScriptBuilder {
if (!tool.solutionBuilder) {
//#region CONFIGURE
const { duration: configureDurationMs, solutionBuilderHost } = measureSync('Configure', () => {
const _tsconfig: TTypescript.ParsedCommandLine = this._loadTsconfig(ts);
const _tsconfig: TTypescript.ParsedCommandLine = this._loadTsconfig(tool);
this._validateTsconfig(ts, _tsconfig);

const _solutionBuilderHost: TSolutionHost = this._buildSolutionBuilderHost(tool);
Expand Down Expand Up @@ -922,19 +956,21 @@ export class TypeScriptBuilder {
return `${outFolderName}:${jsExtensionOverride || '.js'}`;
}

private _loadTsconfig(ts: ExtendedTypeScript): TTypescript.ParsedCommandLine {
private _loadTsconfig(tool: ITypeScriptTool): TTypescript.ParsedCommandLine {
const { ts, system } = tool;
const parsedConfigFile: ReturnType<typeof ts.readConfigFile> = ts.readConfigFile(
this._configuration.tsconfigPath,
ts.sys.readFile
system.readFile
);

const currentFolder: string = path.dirname(this._configuration.tsconfigPath);
const tsconfig: TTypescript.ParsedCommandLine = ts.parseJsonConfigFileContent(
parsedConfigFile.config,
{
fileExists: ts.sys.fileExists,
readFile: ts.sys.readFile,
readDirectory: ts.sys.readDirectory,
fileExists: system.fileExists,
readFile: system.readFile,
readDirectory: system.readDirectory,
realpath: system.realpath,
useCaseSensitiveFileNames: true
},
currentFolder,
Expand Down Expand Up @@ -1054,11 +1090,11 @@ export class TypeScriptBuilder {
// Do nothing
};

const { ts } = tool;
const { ts, system } = tool;

const solutionBuilderHost: TTypescript.SolutionBuilderHost<TTypescript.EmitAndSemanticDiagnosticsBuilderProgram> =
ts.createSolutionBuilderHost(
ts.sys,
system,
this._getCreateBuilderProgram(ts),
tool.reportDiagnostic,
reportSolutionBuilderStatus,
Expand Down Expand Up @@ -1090,7 +1126,11 @@ export class TypeScriptBuilder {
if (tsconfig.options.incremental) {
compilerHost = ts.createIncrementalCompilerHost(tsconfig.options, system);
} else {
compilerHost = ts.createCompilerHost(tsconfig.options, undefined, system);
compilerHost = (ts.createCompilerHostWorker ?? ts.createCompilerHost)(
tsconfig.options,
undefined,
system
);
}

this._changeCompilerHostToUseCache(compilerHost, tool);
Expand Down
8 changes: 8 additions & 0 deletions heft-plugins/heft-typescript-plugin/src/TypeScriptPlugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,12 @@ export interface ITypeScriptConfigurationJson {
*/
useTranspilerWorker?: boolean;

/**
* If true, the TypeScript compiler will only resolve symlinks to their targets if the links are in a node_modules folder.
* This significantly reduces file system operations in typical usage.
*/
onlyResolveSymlinksInNodeModules?: boolean;

/*
* Specifies the tsconfig.json file that will be used for compilation. Equivalent to the "project" argument for the 'tsc' and 'tslint' command line tools.
*
Expand Down Expand Up @@ -365,6 +371,8 @@ export default class TypeScriptPlugin implements IHeftTaskPlugin {

useTranspilerWorker: typeScriptConfigurationJson?.useTranspilerWorker,

onlyResolveSymlinksInNodeModules: typeScriptConfigurationJson?.onlyResolveSymlinksInNodeModules,

tsconfigPath: getTsconfigFilePath(heftConfiguration, typeScriptConfigurationJson),
additionalModuleKindsToEmit: typeScriptConfigurationJson?.additionalModuleKindsToEmit,
emitCjsExtensionForCommonJS: !!typeScriptConfigurationJson?.emitCjsExtensionForCommonJS,
Expand Down
Loading

0 comments on commit 6f5f09d

Please sign in to comment.