Skip to content

Use native private members in Rush tooling - #5941

Open
Bharat Middha (bmiddha) wants to merge 7 commits into
mainfrom
bmiddha/native-private-fields-rush-ecosystem
Open

Use native private members in Rush tooling#5941
Bharat Middha (bmiddha) wants to merge 7 commits into
mainfrom
bmiddha/native-private-fields-rush-ecosystem

Conversation

@bmiddha

@bmiddha Bharat Middha (bmiddha) commented Aug 19, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Summary

  • convert TypeScript-private fields, methods, and accessors across Rush applications, plugins, daemon packages, and extensions to ECMAScript #private members
  • replace private-member destructuring with direct private access
  • keep TypeScript-private members where tests synthesize instances or spy on internals
  • preserve the public PlaywrightBrowserTunnel.status getter while replacing its private setter with #setStatus()
  • keep package behavior and public APIs unchanged

Validation

  • built and tested all 30 affected Rush ecosystem projects and dependents

Dependency

Stacked on #5947, which fixes @rushstack/no-new-null handling for ECMAScript private members. Narrow suppressions remain because these packages consume the released decoupled plugin; they can be removed after the patched plugin is published and adopted.

@bmiddha
refactor: convert TS private fields to #private
dd8d670 
Convert eligible TS 'private' class property declarations to
ECMAScript #private fields across apps/rush, apps/rush-mcp-server,
apps/lockfile-explorer, apps/playwright-browser-tunnel, apps/zipsync,
apps/rundown, apps/trace-import, apps/cpu-profile-summarizer,
apps/rush-serve-dashboard, libraries/rush-terminal-renderer,
libraries/rush-daemon, libraries/rush-daemon-transport,
libraries/rushell, rush-plugins, repo-scripts/repo-toolbox, and
vscode-extensions. Strips one conventional leading underscore from
each converted field name.

Rewrote an unsupported destructuring assignment in
RedisCobuildLockProvider (const { _terminal: terminal } = this;)
to plain property access so it could be converted.

Retained rush-plugins/rush-buildxl-graph-plugin's test-only
'declare private _configHash' field as TS-private: that mock relies
on Object.setPrototypeOf to backfill a field on a plain object,
which true ECMAScript private fields cannot support.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 8ebd5bf2-c44b-42d5-be25-e7936d4b0a14
@github-project-automation github-project-automation Bot moved this to Needs triage in Bug Triage Aug 19, 2026
@bmiddha Bharat Middha (bmiddha) mentioned this pull request Aug 19, 2026
Draft
Bharat Middha (bmiddha) and others added 4 commits August 19, 2026 15:37
@bmiddha
refactor: convert TS private methods and accessors to #private
0167cd7 
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
@bmiddha
refactor: convert overloaded private methods to #private
f2ebc7c 
Manually inspect the 9 tool-skipped candidates from the private
member conversion. Getter/setter and method-overload declarations
that share a TypeScript symbol are known false positives in the
symbol-aware codemod: it flags every overload signature as an
'unsupported reference' to every other overload of the same method.

For AmazonS3Client's '_makeSignedRequestAsync' (3 overload
signatures) and HttpBuildCacheProvider's '_tryGetCredentialsAsync'
(4 overload signatures + implementation), a repo-wide search found
only ordinary same-class 'this.method(...)' call sites -- no
bracket access, 'as any' casts, reflection, prototype tricks, or
test spies. Converted every overload declaration and call site to
'#makeSignedRequestAsync' / '#tryGetCredentialsAsync'.

Left PlaywrightBrowserTunnel's 'status' accessor pair (public
getter / private setter) as TS-private: this is not a false
positive. Native ECMAScript private accessors have no way to make
only the setter private while the getter of the same name stays
public -- '#status' would need to be a single accessor pair with
uniform visibility, and it would collide with the existing
'#status' backing field. Converting it would require restructuring
the field name, which is outside the scope of a straightforward
private-to-# conversion.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 8ebd5bf2-c44b-42d5-be25-e7936d4b0a14
@bmiddha
refactor(playwright-browser-tunnel): eliminate TS-private status setter
64946f8 
Replace the mixed-visibility 'public get status() / private set
status()' accessor pair with a public 'get status()' plus a native
private method '#setStatus(newStatus)'. Native ECMAScript private
accessors require the getter and setter to share one name with
uniform visibility, so the private setter could not be converted to
a same-named '#status' accessor without colliding with the existing
'#status' backing field. Renaming the mutator to a private method
sidesteps that restriction while keeping the public read-only
getter contract unchanged.

Updated all 5 internal 'this.status = <value>' assignments to
'this.#setStatus(<value>)'.

This removes the last remaining TS 'private' class member across
the requested conversion scopes; only the two previously-documented
unsafe members (rush-buildxl-graph-plugin's declare-only
'_configHash' and AmazonS3Client's reflection-accessed
'_writeWarningLine') remain TS-private by design.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 8ebd5bf2-c44b-42d5-be25-e7936d4b0a14
@bmiddha
fix(mcp-server): suppress legacy null warning
f1a0e2f 
The decoupled ESLint plugin does not recognize native private methods, causing no-new-null to report a false positive.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 8ebd5bf2-c44b-42d5-be25-e7936d4b0a14
@bmiddha Bharat Middha (bmiddha) changed the title Use native private fields in Rush tooling Use native private members in Rush tooling Aug 20, 2026
@bmiddha
fix(eslint-plugin): recognize native privacy
6740253 
Treat PrivateIdentifier class keys like TypeScript private members so no-new-null does not report private API types.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 8ebd5bf2-c44b-42d5-be25-e7936d4b0a14
@bmiddha
Merge remote-tracking branch 'origin/bmiddha/fix-no-new-null-native-p…
34aca9e 
…rivate' into bmiddha/native-private-fields-rush-ecosystem
@bmiddha
Bharat Middha (bmiddha) changed the base branch from main to bmiddha/fix-no-new-null-native-private August 20, 2026 06:05
@bmiddha Bharat Middha (bmiddha) mentioned this pull request Aug 20, 2026
Merged
@mojaza
Mo Jazayeri (mojaza) requested a balanced review from Copilot August 21, 2026 18:43
Copilot AI reviewed Aug 21, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Migrates Rush ecosystem tooling from TypeScript-private members to ECMAScript #private members without changing public APIs.

Changes:

  • Converts private fields, methods, and accessors across 30 Rush projects.
  • Replaces private-member destructuring with direct access.
  • Preserves test hooks and the public PlaywrightTunnel.status getter; retains a temporary lint suppression pending #5947 adoption.

Reviewed changes

Copilot reviewed 66 out of 66 changed files in this pull request and generated no comments.

Show a summary per file
File Description
vscode-extensions/vscode-shared/src/VScodeOutputChannelTerminalProvider.ts Migrates output-channel state.
vscode-extensions/rush-vscode-extension/src/providers/RushProjectsProvider.ts Migrates project-provider state.
vscode-extensions/rush-vscode-extension/src/providers/RushCommandsProvider.ts Migrates command-provider state.
vscode-extensions/rush-vscode-extension/src/logic/RushWorkspace.ts Migrates workspace internals.
vscode-extensions/rush-vscode-extension/src/logic/RushCommandWebViewPanel.ts Migrates webview internals.
rush-plugins/rush-serve-plugin/src/tryEnableBuildStatusWebSocketServer.ts Migrates WebSocket storage.
rush-plugins/rush-serve-plugin/src/RushServePlugin.ts Migrates plugin configuration.
rush-plugins/rush-serve-plugin/src/RushProjectServeConfigFile.ts Migrates configuration loader.
rush-plugins/rush-redis-cobuild-plugin/src/RushRedisCobuildPlugin.ts Migrates plugin options.
rush-plugins/rush-redis-cobuild-plugin/src/RedisCobuildLockProvider.ts Migrates Redis provider internals.
rush-plugins/rush-litewatch-plugin/src/WatchProject.ts Migrates watch-state logic.
rush-plugins/rush-litewatch-plugin/src/WatchManager.ts Migrates manager helpers.
rush-plugins/rush-litewatch-plugin/src/test/WatchManager.test.ts Migrates test provider state.
rush-plugins/rush-http-build-cache-plugin/src/HttpBuildCacheProvider.ts Migrates HTTP cache internals.
rush-plugins/rush-buildxl-graph-plugin/src/GraphProcessor.ts Migrates graph-processing helpers.
rush-plugins/rush-buildxl-graph-plugin/src/DropBuildGraphPlugin.ts Migrates command configuration.
rush-plugins/rush-bridge-cache-plugin/src/BridgeCachePlugin.ts Migrates bridge-cache helpers.
rush-plugins/rush-azure-storage-build-cache-plugin/src/RushAzureInteractiveAuthPlugin.ts Migrates authentication options.
rush-plugins/rush-azure-storage-build-cache-plugin/src/AzureStorageBuildCacheProvider.ts Migrates Azure cache internals.
rush-plugins/rush-azure-storage-build-cache-plugin/src/AzureAuthenticationBase.ts Migrates private credential state.
rush-plugins/rush-azure-storage-build-cache-plugin/src/AdoCodespacesAuthCredential.ts Migrates token decoding.
rush-plugins/rush-amazon-s3-build-cache-plugin/src/AmazonS3Client.ts Migrates S3 client internals.
rush-plugins/rush-amazon-s3-build-cache-plugin/src/AmazonS3BuildCacheProvider.ts Migrates S3 provider internals.
repo-scripts/repo-toolbox/src/cli/actions/ReadmeAction.ts Migrates action state.
repo-scripts/repo-toolbox/src/cli/actions/CollectProjectFilesAction.ts Migrates action parameters.
repo-scripts/repo-toolbox/src/cli/actions/BumpDecoupledLocalDependencies.ts Migrates terminal state.
libraries/rushell/src/Tokenizer.ts Migrates tokenizer cursor and helpers.
libraries/rushell/src/TextRange.ts Migrates bounds validation.
libraries/rushell/src/Rushell.ts Migrates evaluation helpers.
libraries/rushell/src/Parser.ts Migrates parser state and helpers.
libraries/rush-terminal-renderer/src/test/TestTerminal.ts Migrates test terminal storage.
libraries/rush-terminal-renderer/src/test/LegacyPipelineReplica.ts Migrates legacy test pipeline.
libraries/rush-terminal-renderer/src/TerminalSinkWritable.ts Migrates terminal storage.
libraries/rush-terminal-renderer/src/OperationStreamRegistry.ts Migrates stream registry state.
libraries/rush-terminal-renderer/src/LegacyCollatedRenderer.ts Migrates renderer state.
libraries/rush-terminal-renderer/src/HostEventRouter.ts Migrates routing helpers.
libraries/rush-terminal-renderer/src/DaemonRendererHost.ts Migrates renderer-host internals.
libraries/rush-daemon/src/RushDaemonHost.ts Migrates daemon lifecycle state.
libraries/rush-daemon/src/RequestScheduler.ts Migrates scheduler internals.
libraries/rush-daemon/src/DaemonControlSession.ts Migrates session state and handlers.
libraries/rush-daemon-transport/src/DaemonListener.ts Migrates listener state.
libraries/rush-daemon-transport/src/DaemonFrameConnection.ts Migrates connection internals.
apps/zipsync/src/cli/ZipSyncCommandLineParser.ts Migrates CLI parameters.
apps/trace-import/src/TraceImportCommandLineParser.ts Migrates CLI parameters.
apps/rush/src/RushVersionSelector.ts Migrates version-selector state.
apps/rush/src/MinimalRushConfiguration.ts Migrates configuration state.
apps/rush-serve-dashboard/src/modules/ansiSgrParser.ts Migrates ANSI parser state.
apps/rush-mcp-server/src/tools/workspace-details.ts Migrates prompt helpers.
apps/rush-mcp-server/src/tools/migrate-project.tool.ts Migrates migration-tool internals.
apps/rush-mcp-server/src/tools/conflict-resolver.tool.ts Migrates resolver helper with suppression.
apps/rush-mcp-server/src/tools/base.tool.ts Migrates base-tool options.
apps/rush-mcp-server/src/server.ts Migrates server state and helpers.
apps/rush-mcp-server/src/pluginFramework/RushMcpPluginSession.ts Migrates session server storage.
apps/rush-mcp-server/src/pluginFramework/RushMcpPluginLoader.ts Migrates plugin-loader state.
apps/rundown/src/Rundown.ts Migrates import tracking.
apps/rundown/src/launcher.ts Migrates launcher tracking state.
apps/rundown/src/cli/InspectAction.ts Migrates trace parameter.
apps/playwright-browser-tunnel/src/PlaywrightBrowserTunnel.ts Migrates tunnel internals and status setter.
apps/playwright-browser-tunnel/src/HttpServer.ts Migrates server internals.
apps/lockfile-explorer/src/utils/PackageUpdateChecker.ts Migrates update-check state.
apps/lockfile-explorer/src/graph/PnpmfileRunner.ts Migrates worker state.
apps/lockfile-explorer/src/cli/lint/LintCommandLineParser.ts Migrates action initialization helper.
apps/lockfile-explorer/src/cli/lint/actions/InitAction.ts Migrates terminal state.
apps/lockfile-explorer/src/cli/lint/actions/CheckAction.ts Migrates lint-check state.
apps/lockfile-explorer/src/cli/explorer/ExplorerCommandLineParser.ts Migrates CLI parameters.
apps/cpu-profile-summarizer/src/start.ts Migrates summarizer parameters.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Base automatically changed from bmiddha/fix-no-new-null-native-private to main August 21, 2026 18:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@iclanton Ian Clanton-Thuon (iclanton) Awaiting requested review from iclanton iclanton is a code owner
@octogonz Pete Gonzalez (octogonz) Awaiting requested review from octogonz octogonz is a code owner
@apostolisms Apostolis Haitalis (apostolisms) Awaiting requested review from apostolisms apostolisms is a code owner
@dmichon-msft David Michon (dmichon-msft) Awaiting requested review from dmichon-msft dmichon-msft is a code owner
@jxanthony Jonathon Anthony (jxanthony) Awaiting requested review from jxanthony jxanthony is a code owner

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

Bug Triage
Status: Needs triage

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@bmiddha