Skip to content

Commit 0167cd7

Browse files
bmiddhaCopilot
andcommitted
refactor: convert TS private methods and accessors to #private
Extend the earlier field-only #private conversion to also cover private methods and accessors across apps/lockfile-explorer, apps/playwright-browser-tunnel, apps/rundown, apps/rush-mcp-server, apps/rush-serve-dashboard, libraries/rush-daemon, libraries/rush-daemon-transport, libraries/rush-terminal-renderer, libraries/rushell, rush-plugins, and vscode-extensions. Strips one conventional leading underscore from each converted member name. Reverted two members that the symbol-aware tool converted but that are unsafe in practice: - rush-buildxl-graph-plugin's test-only 'declare private _configHash' cannot use 'declare' with a private identifier (TS18019), matching the earlier decision to keep it TS-private for the Object.setPrototypeOf mock. - AmazonS3Client's '_writeWarningLine' is spied on and stubbed via '(s3Client as any)._writeWarningLine' in AmazonS3Client.test.ts, which requires runtime reflection that true private fields do not support. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8ebd5bf2-c44b-42d5-be25-e7936d4b0a14
1 parent dd8d670 commit 0167cd7

36 files changed

Lines changed: 339 additions & 339 deletions

File tree

apps/lockfile-explorer/src/cli/lint/LintCommandLineParser.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ export class LintCommandLineParser extends CommandLineParser {
2222

2323
this.globalTerminal = terminal;
2424

25-
this._populateActions();
25+
this.#populateActions();
2626
}
2727

2828
protected override async onExecuteAsync(): Promise<void> {
@@ -33,7 +33,7 @@ export class LintCommandLineParser extends CommandLineParser {
3333
await super.onExecuteAsync();
3434
}
3535

36-
private _populateActions(): void {
36+
#populateActions(): void {
3737
const terminal: ITerminal = this.globalTerminal;
3838
this.addAction(new InitAction(terminal));
3939
this.addAction(new CheckAction(terminal));

apps/lockfile-explorer/src/cli/lint/actions/CheckAction.ts

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ export class CheckAction extends CommandLineAction {
5959
this.#docMap = new Map();
6060
}
6161

62-
private async _checkVersionCompatibilityAsync(
62+
async #checkVersionCompatibilityAsync(
6363
shrinkwrapFileMajorVersion: number,
6464
packages: lockfileTypes.PackageSnapshots | undefined,
6565
dependencyPath: pnpmTypes.DepPath,
@@ -79,7 +79,7 @@ export class CheckAction extends CommandLineAction {
7979
await Promise.all(
8080
Object.entries(packages[dependencyPath].dependencies ?? {}).map(
8181
async ([dependencyPackageName, dependencyPackageVersion]) => {
82-
await this._checkVersionCompatibilityAsync(
82+
await this.#checkVersionCompatibilityAsync(
8383
shrinkwrapFileMajorVersion,
8484
packages,
8585
splicePackageWithVersion(
@@ -96,7 +96,7 @@ export class CheckAction extends CommandLineAction {
9696
}
9797
}
9898

99-
private async _searchAndValidateDependenciesAsync(
99+
async #searchAndValidateDependenciesAsync(
100100
project: RushConfigurationProject,
101101
requiredVersions: Record<string, string>
102102
): Promise<void> {
@@ -139,10 +139,10 @@ export class CheckAction extends CommandLineAction {
139139
this.#rushConfiguration.getProjectByName(dependencyName);
140140
if (dependencyProject && !this.#checkedProjects?.has(dependencyProject)) {
141141
this.#checkedProjects!.add(project);
142-
await this._searchAndValidateDependenciesAsync(dependencyProject, requiredVersions);
142+
await this.#searchAndValidateDependenciesAsync(dependencyProject, requiredVersions);
143143
}
144144
} else {
145-
await this._checkVersionCompatibilityAsync(
145+
await this.#checkVersionCompatibilityAsync(
146146
shrinkwrapFileMajorVersion,
147147
packages,
148148
fullDependencyPath,
@@ -156,7 +156,7 @@ export class CheckAction extends CommandLineAction {
156156
);
157157
}
158158

159-
private async _performVersionRestrictionCheckAsync(
159+
async #performVersionRestrictionCheckAsync(
160160
requiredVersions: Record<string, string>,
161161
projectName: string
162162
): Promise<string | undefined> {
@@ -169,7 +169,7 @@ export class CheckAction extends CommandLineAction {
169169
);
170170
}
171171
this.#checkedProjects.add(project);
172-
await this._searchAndValidateDependenciesAsync(project, requiredVersions);
172+
await this.#searchAndValidateDependenciesAsync(project, requiredVersions);
173173
return undefined;
174174
} catch (e) {
175175
return e.message;
@@ -201,7 +201,7 @@ export class CheckAction extends CommandLineAction {
201201
async ({ requiredVersions, project, rule }) => {
202202
switch (rule) {
203203
case 'restrict-versions': {
204-
const message: string | undefined = await this._performVersionRestrictionCheckAsync(
204+
const message: string | undefined = await this.#performVersionRestrictionCheckAsync(
205205
requiredVersions,
206206
project
207207
);

apps/lockfile-explorer/src/utils/PackageUpdateChecker.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -170,7 +170,7 @@ export class PackageUpdateChecker {
170170
return undefined;
171171
}
172172

173-
const cacheFilePath: string = this._getCacheFilePath();
173+
const cacheFilePath: string = this.#getCacheFilePath();
174174

175175
let latestVersion: string | undefined;
176176
if (!this.#forceCheck) {
@@ -200,7 +200,7 @@ export class PackageUpdateChecker {
200200
};
201201
}
202202

203-
private _getCacheFilePath(): string {
203+
#getCacheFilePath(): string {
204204
// Replace characters that are unsafe in file names (e.g. the "/" in scoped package names).
205205
const sanitizedName: string = this.#packageName.replace(/[^a-zA-Z0-9._-]/g, '_');
206206
return `${CACHE_FOLDER}/${sanitizedName}.json`;

apps/playwright-browser-tunnel/src/PlaywrightBrowserTunnel.ts

Lines changed: 23 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,7 @@ export class PlaywrightTunnel {
159159
terminal.writeLine(`keepRunning: ${this.#keepRunning}`);
160160
while (this.#keepRunning) {
161161
if (!this.#initWsPromise) {
162-
this.#initWsPromise = this._initPlaywrightBrowserTunnelAsync();
162+
this.#initWsPromise = this.#initPlaywrightBrowserTunnelAsync();
163163
} else {
164164
terminal.writeLine(`Tunnel is already running with status: ${this.status}`);
165165
}
@@ -197,7 +197,7 @@ export class PlaywrightTunnel {
197197
// TODO: We should implement an uninstall command to remove installed Playwright browsers
198198
// public async uninstallPlaywrightBrowsersAsync(): Promise<void> {}
199199

200-
private async _runCommandAsync(command: string, args: string[]): Promise<void> {
200+
async #runCommandAsync(command: string, args: string[]): Promise<void> {
201201
const tmpPath: string = this.#playwrightInstallPath;
202202
await FileSystem.ensureFolderAsync(tmpPath);
203203
this.#terminal.writeLine(`Running command: ${command} ${args.join(' ')} in ${tmpPath}`);
@@ -227,30 +227,30 @@ export class PlaywrightTunnel {
227227
await Executable.waitForExitAsync(cp, { throwOnNonZeroExitCode: true, throwOnSignal: true });
228228
}
229229

230-
private async _installPlaywrightCoreAsync({
230+
async #installPlaywrightCoreAsync({
231231
playwrightVersion
232232
}: Pick<IHandshake, 'playwrightVersion'>): Promise<void> {
233233
this.#terminal.writeLine(`Installing playwright-core version ${playwrightVersion}`);
234-
await this._runCommandAsync('npm', [
234+
await this.#runCommandAsync('npm', [
235235
'install',
236236
`playwright-core-${playwrightVersion}@npm:playwright-core@${playwrightVersion}`
237237
]);
238238
}
239239

240-
private async _installPlaywrightBrowsersAsync({
240+
async #installPlaywrightBrowsersAsync({
241241
playwrightVersion,
242242
browserName
243243
}: Pick<IHandshake, 'playwrightVersion' | 'browserName'>): Promise<void> {
244-
await this._installPlaywrightCoreAsync({ playwrightVersion });
244+
await this.#installPlaywrightCoreAsync({ playwrightVersion });
245245
this.#terminal.writeLine(`Executing playwright-core version ${playwrightVersion}`);
246-
await this._runCommandAsync('node', [
246+
await this.#runCommandAsync('node', [
247247
`node_modules/playwright-core-${playwrightVersion}/cli.js`,
248248
'install',
249249
browserName
250250
]);
251251
}
252252

253-
private async _tryConnectAsync(): Promise<WebSocket> {
253+
async #tryConnectAsync(): Promise<WebSocket> {
254254
const wsEndpoint: string | undefined = this.#wsEndpoint;
255255
if (!wsEndpoint) {
256256
throw new Error('WebSocket endpoint is not defined');
@@ -269,14 +269,14 @@ export class PlaywrightTunnel {
269269

270270
// TODO: Only supporting one test at a time.
271271
// Need to support multiple simultaneous connections for parallel tests.
272-
private async _pollConnectionAsync(): Promise<WebSocket> {
272+
async #pollConnectionAsync(): Promise<WebSocket> {
273273
this.#terminal.writeLine(`Waiting for WebSocket connection`);
274274
return await new Promise((resolve, reject) => {
275275
this.#pollInterval = setInterval(() => {
276276
if (this.#pendingConnectionAttempt) {
277277
return; // Skip if a connection attempt is already in progress
278278
}
279-
const connectionPromise: Promise<WebSocket> = this._tryConnectAsync();
279+
const connectionPromise: Promise<WebSocket> = this.#tryConnectAsync();
280280
this.#pendingConnectionAttempt = connectionPromise;
281281
connectionPromise
282282
.then((ws: WebSocket) => {
@@ -294,7 +294,7 @@ export class PlaywrightTunnel {
294294
});
295295
}
296296

297-
private async _waitForIncomingConnectionAsync(): Promise<WebSocket> {
297+
async #waitForIncomingConnectionAsync(): Promise<WebSocket> {
298298
this.#terminal.writeLine('Waiting for incoming WebSocket connection');
299299

300300
return await new Promise<WebSocket>((resolve, reject) => {
@@ -334,7 +334,7 @@ export class PlaywrightTunnel {
334334
// TODO: If a user runs this for the first time, `this._playwrightBrowsersInstalled` will be empty
335335
// and it will try to install the browsers every time. We should persist this information. Maybe a cache file with text per
336336
// machine instance?
337-
private async _setupPlaywrightAsync({
337+
async #setupPlaywrightAsync({
338338
playwrightVersion,
339339
browserName
340340
}: Pick<IHandshake, 'playwrightVersion' | 'browserName'>): Promise<typeof import('playwright-core')> {
@@ -344,15 +344,15 @@ export class PlaywrightTunnel {
344344
this.#terminal.writeLine(
345345
`Playwright browser not found. Installing playwright-core version ${playwrightVersion}`
346346
);
347-
await this._installPlaywrightBrowsersAsync({ playwrightVersion, browserName });
347+
await this.#installPlaywrightBrowsersAsync({ playwrightVersion, browserName });
348348
this.#playwrightBrowsersInstalled.add(browserKey);
349349
}
350350

351351
this.#terminal.writeLine(`Using playwright-core version ${playwrightVersion} for browser server`);
352352
return await import(`${this.#playwrightInstallPath}/node_modules/playwright-core-${playwrightVersion}`);
353353
}
354354

355-
private async _getPlaywrightBrowserServerProxyAsync({
355+
async #getPlaywrightBrowserServerProxyAsync({
356356
browserName,
357357
playwrightVersion,
358358
launchOptions
@@ -385,7 +385,7 @@ export class PlaywrightTunnel {
385385
`Launch options after validation: ${JSON.stringify(logOptions)} (headless: false enforced)`
386386
);
387387

388-
const playwright: typeof import('playwright-core') = await this._setupPlaywrightAsync({
388+
const playwright: typeof import('playwright-core') = await this.#setupPlaywrightAsync({
389389
playwrightVersion,
390390
browserName
391391
});
@@ -410,7 +410,7 @@ export class PlaywrightTunnel {
410410
};
411411
}
412412

413-
private _validateHandshake(rawHandshake: unknown): IHandshake {
413+
#validateHandshake(rawHandshake: unknown): IHandshake {
414414
if (
415415
typeof rawHandshake !== 'object' ||
416416
rawHandshake === null ||
@@ -447,7 +447,7 @@ export class PlaywrightTunnel {
447447
}
448448

449449
// ws1 is the tunnel websocket, ws2 is the browser server websocket
450-
private async _setupForwardingAsync(ws1: WebSocket, ws2: WebSocket): Promise<void> {
450+
async #setupForwardingAsync(ws1: WebSocket, ws2: WebSocket): Promise<void> {
451451
this.#terminal.writeLine('Setting up message forwarding between ws1 and ws2');
452452
this.#terminal.writeLine(` ws1 (tunnel) readyState: ${getWebSocketReadyStateString(ws1.readyState)}`);
453453
this.#terminal.writeLine(` ws2 (browser) readyState: ${getWebSocketReadyStateString(ws2.readyState)}`);
@@ -519,16 +519,16 @@ export class PlaywrightTunnel {
519519
* and setting up the browser server.
520520
* Returns when the handshake is complete and the browser server is running.
521521
*/
522-
private async _initPlaywrightBrowserTunnelAsync(): Promise<WebSocket> {
522+
async #initPlaywrightBrowserTunnelAsync(): Promise<WebSocket> {
523523
let handshake: IHandshake | undefined = undefined;
524524
let client: WebSocket | undefined = undefined;
525525
let browserServer: BrowserServer | undefined = undefined;
526526

527527
this.status = 'waiting-for-connection';
528528
const ws: WebSocket =
529529
this.#mode === 'poll-connection'
530-
? await this._pollConnectionAsync()
531-
: await this._waitForIncomingConnectionAsync();
530+
? await this.#pollConnectionAsync()
531+
: await this.#waitForIncomingConnectionAsync();
532532

533533
ws.on('open', () => {
534534
this.#terminal.writeLine(`WebSocket connection established`);
@@ -564,7 +564,7 @@ export class PlaywrightTunnel {
564564
const rawHandshakeString: string = data.toString();
565565
const rawHandshake: unknown = JSON.parse(rawHandshakeString);
566566
terminal.writeLine(`Received handshake: ${rawHandshakeString}`);
567-
handshake = this._validateHandshake(rawHandshake);
567+
handshake = this.#validateHandshake(rawHandshake);
568568

569569
// Call the onBeforeLaunch callback if provided
570570
if (this.#onBeforeLaunch) {
@@ -582,7 +582,7 @@ export class PlaywrightTunnel {
582582

583583
this.status = 'setting-up-browser-server';
584584
const browserServerProxy: IBrowserServerProxy =
585-
await this._getPlaywrightBrowserServerProxyAsync(handshake);
585+
await this.#getPlaywrightBrowserServerProxyAsync(handshake);
586586
client = browserServerProxy.client;
587587
browserServer = browserServerProxy.browserServer;
588588

@@ -616,7 +616,7 @@ export class PlaywrightTunnel {
616616
await Async.sleepAsync(2000);
617617

618618
ws.send(JSON.stringify({ action: 'handshakeAck' }));
619-
await this._setupForwardingAsync(ws, client);
619+
await this.#setupForwardingAsync(ws, client);
620620

621621
// Clean up message handler after successful handshake
622622
ws.off('message', onMessageHandler);

apps/rundown/src/Rundown.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ export class Rundown {
3434
// ["path/to/launcher.js", "path/to/target-script.js", "first-target-arg"]
3535
const nodeArgs: string[] = [path.join(__dirname, 'launcher.js'), absoluteScriptPath, ...expandedArgs];
3636

37-
await this._spawnLauncherAsync(nodeArgs, quiet, ignoreExitCode);
37+
await this.#spawnLauncherAsync(nodeArgs, quiet, ignoreExitCode);
3838

3939
if (!quiet) {
4040
console.log();
@@ -107,7 +107,7 @@ export class Rundown {
107107
FileSystem.writeFile(reportPath, data);
108108
}
109109

110-
private async _spawnLauncherAsync(
110+
async #spawnLauncherAsync(
111111
nodeArgs: string[],
112112
quiet: boolean,
113113
ignoreExitCode: boolean

apps/rundown/src/launcher.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ class Launcher {
3131
return [nodeArg, this.targetScriptPathArg, ...remainderArgs];
3232
}
3333

34-
private _sendIpcTraceBatch(): void {
34+
#sendIpcTraceBatch(): void {
3535
if (this.#ipcTraceRecordsBatch.length > 0) {
3636
const batch: IIpcTraceRecord[] = [...this.#ipcTraceRecordsBatch];
3737
this.#ipcTraceRecordsBatch.length = 0;
@@ -49,7 +49,7 @@ class Launcher {
4949
const importedModules: Set<unknown> = this.#importedModules; // for closure
5050
const importedModulePaths: Set<string> = this.#importedModulePaths; // for closure
5151
const ipcTraceRecordsBatch: IIpcTraceRecord[] = this.#ipcTraceRecordsBatch; // for closure
52-
const sendIpcTraceBatch: () => void = this._sendIpcTraceBatch.bind(this); // for closure
52+
const sendIpcTraceBatch: () => void = this.#sendIpcTraceBatch.bind(this); // for closure
5353

5454
function hookedRequire(this: NodeModule, moduleName: string): unknown {
5555
// NOTE: The "this" pointer is the calling NodeModule, so we rely on closure
@@ -100,7 +100,7 @@ class Launcher {
100100
_copyProperties(hookedRequire, realRequire);
101101

102102
process.on('exit', () => {
103-
this._sendIpcTraceBatch();
103+
this.#sendIpcTraceBatch();
104104
process.send!({
105105
id: 'done'
106106
} as IIpcDone);

apps/rush-mcp-server/src/server.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,21 +29,21 @@ export class RushMCPServer extends McpServer {
2929
}
3030

3131
public async startAsync(): Promise<void> {
32-
this._initializeTools();
33-
this._registerTools();
32+
this.#initializeTools();
33+
this.#registerTools();
3434

3535
await this.#pluginLoader.loadAsync();
3636
}
3737

38-
private _initializeTools(): void {
38+
#initializeTools(): void {
3939
this.#tools.push(new RushConflictResolverTool());
4040
this.#tools.push(new RushMigrateProjectTool(this.#rushWorkspacePath));
4141
this.#tools.push(new RushCommandValidatorTool());
4242
this.#tools.push(new RushWorkspaceDetailsTool());
4343
this.#tools.push(new RushProjectDetailsTool());
4444
}
4545

46-
private _registerTools(): void {
46+
#registerTools(): void {
4747
process.chdir(this.#rushWorkspacePath);
4848

4949
for (const tool of this.#tools) {

apps/rush-mcp-server/src/tools/conflict-resolver.tool.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ export class RushConflictResolverTool extends BaseTool {
2222
});
2323
}
2424

25-
private _tryGetSubspaceNameFromLockfilePath(
25+
#tryGetSubspaceNameFromLockfilePath(
2626
lockfilePath: string,
2727
rushConfiguration: RushConfiguration
2828
): string | null {
@@ -37,7 +37,7 @@ export class RushConflictResolverTool extends BaseTool {
3737

3838
public async executeAsync({ lockfilePath }: { lockfilePath: string }): Promise<CallToolResult> {
3939
const rushConfiguration: RushConfiguration = await getRushConfiguration();
40-
const subspaceName: string | null = this._tryGetSubspaceNameFromLockfilePath(
40+
const subspaceName: string | null = this.#tryGetSubspaceNameFromLockfilePath(
4141
lockfilePath,
4242
rushConfiguration
4343
);

0 commit comments

Comments
 (0)