Skip to content

Use native private members in rush-lib #5939

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.

Sign up for GitHub

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

Open
Bharat Middha (bmiddha) wants to merge 4 commits into
base: bmiddha/fix-no-new-null-native-private
Choose a base branch
from
Open

Use native private members in rush-lib #5939

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
95b76f9
refactor(rush-lib): use native private fields
bmiddha Aug 19, 2026
86218bb
refactor(rush-lib): use native private methods and accessors
bmiddha Aug 19, 2026
1f8e037
fix(rush-lib): suppress legacy null warning
bmiddha Aug 19, 2026
07c0f06
Merge remote-tracking branch 'origin/bmiddha/fix-no-new-null-native-p…
bmiddha Aug 20, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 24 additions & 24 deletions libraries/rush-lib/src/api/ApprovedPackagesConfiguration.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,39 +59,39 @@ const _jsonSchema: JsonSchema = JsonSchema.fromLoadedObject(schemaJson);
export class ApprovedPackagesConfiguration {
public items: ApprovedPackagesItem[] = [];

private _itemsByName: Map<string, ApprovedPackagesItem> = new Map<string, ApprovedPackagesItem>();
#itemsByName: Map<string, ApprovedPackagesItem> = new Map<string, ApprovedPackagesItem>();

private _loadedJson!: IApprovedPackagesJson;
private _jsonFilename: string;
#loadedJson!: IApprovedPackagesJson;
#jsonFilename: string;

public constructor(jsonFilename: string) {
this._jsonFilename = jsonFilename;
this.#jsonFilename = jsonFilename;
this.clear();
}

/**
* Clears all the settings, returning to an empty state.
*/
public clear(): void {
this._itemsByName.clear();
this._loadedJson = {
this.#itemsByName.clear();
this.#loadedJson = {
// Ensure this comes first in the key ordering
$schema: '',
packages: []
};
}

public getItemByName(packageName: string): ApprovedPackagesItem | undefined {
return this._itemsByName.get(packageName);
return this.#itemsByName.get(packageName);
}

public addOrUpdatePackage(packageName: string, reviewCategory: string): boolean {
let changed: boolean = false;

let item: ApprovedPackagesItem | undefined = this._itemsByName.get(packageName);
let item: ApprovedPackagesItem | undefined = this.#itemsByName.get(packageName);
if (!item) {
item = new ApprovedPackagesItem(packageName);
this._addItem(item);
this.#addItem(item);
changed = true;
}

Expand All @@ -107,7 +107,7 @@ export class ApprovedPackagesConfiguration {
* If the file exists, calls loadFromFile().
*/
public tryLoadFromFile(approvedPackagesPolicyEnabled: boolean): boolean {
if (!FileSystem.exists(this._jsonFilename)) {
if (!FileSystem.exists(this.#jsonFilename)) {
return false;
}

Expand All @@ -116,7 +116,7 @@ export class ApprovedPackagesConfiguration {
if (!approvedPackagesPolicyEnabled) {
// eslint-disable-next-line no-console
console.log(
`Warning: Ignoring "${path.basename(this._jsonFilename)}" because the` +
`Warning: Ignoring "${path.basename(this.#jsonFilename)}" because the` +
` "approvedPackagesPolicy" setting was not specified in ${RushConstants.rushJsonFilename}`
);
}
Expand All @@ -129,14 +129,14 @@ export class ApprovedPackagesConfiguration {
*/
public loadFromFile(): void {
const approvedPackagesJson: IApprovedPackagesJson = JsonFile.loadAndValidate(
this._jsonFilename,
this.#jsonFilename,
_jsonSchema
);

this.clear();

for (const browserPackage of approvedPackagesJson.packages) {
this._addItemJson(browserPackage, this._jsonFilename);
this.#addItemJson(browserPackage, this.#jsonFilename);
}
}

Expand All @@ -148,9 +148,9 @@ export class ApprovedPackagesConfiguration {
// (which passed schema validation).

// eslint-disable-next-line dot-notation
this._loadedJson['$schema'] = JsonSchemaUrls.approvedPackages;
this.#loadedJson['$schema'] = JsonSchemaUrls.approvedPackages;

this._loadedJson.packages = [];
this.#loadedJson.packages = [];

this.items.sort((a: ApprovedPackagesItem, b: ApprovedPackagesItem) => {
return a.packageName.localeCompare(b.packageName);
Expand All @@ -166,11 +166,11 @@ export class ApprovedPackagesConfiguration {
allowedCategories: allowedCategories
};

this._loadedJson.packages.push(itemJson);
this.#loadedJson.packages.push(itemJson);
}

// Save the file
let body: string = JsonFile.stringify(this._loadedJson);
let body: string = JsonFile.stringify(this.#loadedJson);

// Unindent the allowedCategories array to improve readability
body = body.replace(/("allowedCategories": +\[)([^\]]+)/g, (substring: string, ...args: string[]) => {
Expand All @@ -180,16 +180,16 @@ export class ApprovedPackagesConfiguration {
// Add a header
body = '// DO NOT ADD COMMENTS IN THIS FILE. They will be lost when the Rush tool resaves it.\n' + body;

FileSystem.writeFile(this._jsonFilename, body, {
FileSystem.writeFile(this.#jsonFilename, body, {
convertLineEndings: NewlineKind.CrLf
});
}

/**
* Helper function only used by the constructor when loading the file.
*/
private _addItemJson(itemJson: IApprovedPackagesItemJson, jsonFilename: string): void {
if (this._itemsByName.has(itemJson.name)) {
#addItemJson(itemJson: IApprovedPackagesItemJson, jsonFilename: string): void {
if (this.#itemsByName.has(itemJson.name)) {
throw new Error(
`Error loading package review file ${jsonFilename}:\n` +
` the name "${itemJson.name}" appears more than once`
Expand All @@ -202,18 +202,18 @@ export class ApprovedPackagesConfiguration {
item.allowedCategories.add(allowedCategory);
}
}
this._addItem(item);
this.#addItem(item);
}

/**
* Helper function that adds an already created ApprovedPackagesItem to the
* list and set.
*/
private _addItem(item: ApprovedPackagesItem): void {
if (this._itemsByName.has(item.packageName)) {
#addItem(item: ApprovedPackagesItem): void {
if (this.#itemsByName.has(item.packageName)) {
throw new InternalError('Duplicate key');
}
this.items.push(item);
this._itemsByName.set(item.packageName, item);
this.#itemsByName.set(item.packageName, item);
}
}
28 changes: 14 additions & 14 deletions libraries/rush-lib/src/api/ChangeFile.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ import { Git } from '../logic/Git';
* This class represents a single change file.
*/
export class ChangeFile {
private _changeFileData: IChangeFile;
private _rushConfiguration: RushConfiguration;
#changeFileData: IChangeFile;
#rushConfiguration: RushConfiguration;

/**
* @internal
Expand All @@ -30,16 +30,16 @@ export class ChangeFile {
throw new Error(`rushConfiguration does not have a value`);
}

this._changeFileData = changeFileData;
this._rushConfiguration = rushConfiguration;
this.#changeFileData = changeFileData;
this.#rushConfiguration = rushConfiguration;
}

/**
* Adds a change entry into the change file
* @param data - change information
*/
public addChange(data: IChangeInfo): void {
this._changeFileData.changes.push(data);
this.#changeFileData.changes.push(data);
}

/**
Expand All @@ -48,7 +48,7 @@ export class ChangeFile {
*/
public getChanges(packageName: string): IChangeInfo[] {
const changes: IChangeInfo[] = [];
for (const info of this._changeFileData.changes) {
for (const info of this.#changeFileData.changes) {
if (info.packageName === packageName) {
changes.push(info);
}
Expand All @@ -63,7 +63,7 @@ export class ChangeFile {
*/
public writeSync(): string {
const filePath: string = this.generatePath();
JsonFile.save(this._changeFileData, filePath, {
JsonFile.save(this.#changeFileData, filePath, {
ensureFolderExists: true
});
return filePath;
Expand All @@ -76,7 +76,7 @@ export class ChangeFile {
*/
public generatePath(): string {
let branch: string | undefined = undefined;
const git: Git = new Git(this._rushConfiguration);
const git: Git = new Git(this.#rushConfiguration);
const repoInfo: gitInfo.GitRepoInfo | undefined = git.getGitInfo();
branch = repoInfo && repoInfo.branch;
if (!branch) {
Expand All @@ -89,13 +89,13 @@ export class ChangeFile {
// flag rarely had any effect, and a second invocation would silently clobber the
// change file written by the first one. See GitHub issue #2195.
// example filename: yourbranchname_2017-05-01-20-20-30.json
const timestamp: string | undefined = this._getTimestamp(true);
const timestamp: string | undefined = this.#getTimestamp(true);
const filename: string = branch
? this._escapeFilename(`${branch}_${timestamp}.json`)
? this.#escapeFilename(`${branch}_${timestamp}.json`)
: `${timestamp}.json`;
const filePath: string = path.join(
this._rushConfiguration.changesFolder,
...this._changeFileData.packageName.split('/'),
this.#rushConfiguration.changesFolder,
...this.#changeFileData.packageName.split('/'),
filename
);
return filePath;
Expand All @@ -105,7 +105,7 @@ export class ChangeFile {
* Gets the current time, formatted as YYYY-MM-DD-HH-MM
* When useSeconds is true, the seconds are appended as well: YYYY-MM-DD-HH-MM-SS
*/
private _getTimestamp(useSeconds: boolean = false): string | undefined {
#getTimestamp(useSeconds: boolean = false): string | undefined {
// Create a date string with the current time

// dateString === "2016-10-19T22:47:49.606Z"
Expand Down Expand Up @@ -137,7 +137,7 @@ export class ChangeFile {
return undefined;
}

private _escapeFilename(filename: string, replacer: string = '-'): string {
#escapeFilename(filename: string, replacer: string = '-'): string {
// Removes / ? < > \ : * | ", really anything that isn't a letter, number, '.' '_' or '-'
const badCharacters: RegExp = /[^a-zA-Z0-9._-]/g;
return filename.replace(badCharacters, replacer);
Expand Down
24 changes: 12 additions & 12 deletions libraries/rush-lib/src/api/CobuildConfiguration.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,9 +76,9 @@ export class CobuildConfiguration {
*/
public readonly cobuildWithoutCacheAllowed: boolean;

private _cobuildLockProvider: ICobuildLockProvider | undefined;
private readonly _cobuildLockProviderFactory: CobuildLockProviderFactory;
private readonly _cobuildJson: ICobuildJson;
#cobuildLockProvider: ICobuildLockProvider | undefined;
readonly #cobuildLockProviderFactory: CobuildLockProviderFactory;
readonly #cobuildJson: ICobuildJson;

private constructor(options: ICobuildConfigurationOptions) {
const { cobuildJson, cobuildLockProviderFactory, rushConfiguration } = options;
Expand All @@ -91,8 +91,8 @@ export class CobuildConfiguration {
this.cobuildWithoutCacheAllowed =
rushConfiguration.experimentsConfiguration.configuration.allowCobuildWithoutCache ?? false;

this._cobuildLockProviderFactory = cobuildLockProviderFactory;
this._cobuildJson = cobuildJson;
this.#cobuildLockProviderFactory = cobuildLockProviderFactory;
this.#cobuildJson = cobuildJson;
}

/**
Expand Down Expand Up @@ -127,25 +127,25 @@ export class CobuildConfiguration {
public async createLockProviderAsync(terminal: ITerminal): Promise<void> {
if (this.cobuildFeatureEnabled) {
terminal.writeLine(`Running cobuild (runner ${this.cobuildContextId}/${this.cobuildRunnerId})`);
const cobuildLockProvider: ICobuildLockProvider = await this._cobuildLockProviderFactory(
this._cobuildJson
const cobuildLockProvider: ICobuildLockProvider = await this.#cobuildLockProviderFactory(
this.#cobuildJson
);
this._cobuildLockProvider = cobuildLockProvider;
await this._cobuildLockProvider.connectAsync();
this.#cobuildLockProvider = cobuildLockProvider;
await this.#cobuildLockProvider.connectAsync();
}
}

public async destroyLockProviderAsync(): Promise<void> {
if (this.cobuildFeatureEnabled) {
await this._cobuildLockProvider?.disconnectAsync();
await this.#cobuildLockProvider?.disconnectAsync();
}
}

public getCobuildLockProvider(): ICobuildLockProvider {
if (!this._cobuildLockProvider) {
if (!this.#cobuildLockProvider) {
throw new Error(`Cobuild lock provider has not been created`);
}
return this._cobuildLockProvider;
return this.#cobuildLockProvider;
}
}

Expand Down
18 changes: 9 additions & 9 deletions libraries/rush-lib/src/api/CommandLineConfiguration.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@ export class CommandLineConfiguration {
/**
* A map of bulk command names to their corresponding synthetic phase identifiers
*/
private readonly _syntheticPhasesByTranslatedBulkCommandName: Map<string, IPhase> = new Map();
readonly #syntheticPhasesByTranslatedBulkCommandName: Map<string, IPhase> = new Map();

/**
* Use CommandLineConfiguration.loadFromFile()
Expand Down Expand Up @@ -332,7 +332,7 @@ export class CommandLineConfiguration {
const safePhases: Set<IPhase> = new Set();
const cycleDetector: Set<IPhase> = new Set();
for (const phase of this.phases.values()) {
this._checkForPhaseSelfCycles(phase, cycleDetector, safePhases);
this.#checkForPhaseSelfCycles(phase, cycleDetector, safePhases);
}
}

Expand Down Expand Up @@ -445,7 +445,7 @@ export class CommandLineConfiguration {

case RushConstants.bulkCommandKind: {
// Translate the bulk command into a phased command
normalizedCommand = this._translateBulkCommandToPhasedCommand(command);
normalizedCommand = this.#translateBulkCommandToPhasedCommand(command);
break;
}
}
Expand Down Expand Up @@ -480,7 +480,7 @@ export class CommandLineConfiguration {
let buildCommand: Command | undefined = this.commands.get(RushConstants.buildCommandName);
if (!buildCommand) {
// If the build command was not specified in the config file, add the default build command
buildCommand = this._translateBulkCommandToPhasedCommand(DEFAULT_BUILD_COMMAND_JSON);
buildCommand = this.#translateBulkCommandToPhasedCommand(DEFAULT_BUILD_COMMAND_JSON);
buildCommand.disableBuildCache = DEFAULT_BUILD_COMMAND_JSON.disableBuildCache;
buildCommandPhases = buildCommand.phases;
buildCommandOriginalPhases = buildCommand.originalPhases;
Expand Down Expand Up @@ -545,7 +545,7 @@ export class CommandLineConfiguration {
if (normalizedParameter.associatedCommands) {
for (const associatedCommandName of normalizedParameter.associatedCommands) {
const syntheticPhase: IPhase | undefined =
this._syntheticPhasesByTranslatedBulkCommandName.get(associatedCommandName);
this.#syntheticPhasesByTranslatedBulkCommandName.get(associatedCommandName);
if (syntheticPhase) {
// If this parameter was associated with a bulk command, include the association
// with the synthetic phase
Expand Down Expand Up @@ -598,7 +598,7 @@ export class CommandLineConfiguration {
* @param phasesInPath The current path from the start node to `phase`
* @param cycleFreePhases Phases that have already been fully walked and confirmed to not be in any cycles
*/
private _checkForPhaseSelfCycles(
#checkForPhaseSelfCycles(
phase: IPhase,
phasesInPath: Set<IPhase>,
cycleFreePhases: Set<IPhase>
Expand All @@ -619,7 +619,7 @@ export class CommandLineConfiguration {
);
} else {
phasesInPath.add(dependency);
this._checkForPhaseSelfCycles(dependency, phasesInPath, cycleFreePhases);
this.#checkForPhaseSelfCycles(dependency, phasesInPath, cycleFreePhases);
phasesInPath.delete(dependency);
}
}
Expand Down Expand Up @@ -709,7 +709,7 @@ export class CommandLineConfiguration {
(this.additionalPathFolders as string[]).unshift(pathFolder);
}

private _translateBulkCommandToPhasedCommand(command: IBulkCommandJson): IPhasedCommandConfig {
#translateBulkCommandToPhasedCommand(command: IBulkCommandJson): IPhasedCommandConfig {
const phaseName: string = command.name;
const phase: IPhase = {
name: phaseName,
Expand All @@ -730,7 +730,7 @@ export class CommandLineConfiguration {
}

this.phases.set(phaseName, phase);
this._syntheticPhasesByTranslatedBulkCommandName.set(command.name, phase);
this.#syntheticPhasesByTranslatedBulkCommandName.set(command.name, phase);

const phases: Set<IPhase> = new Set([phase]);

Expand Down
Loading
Loading