Skip to content

Commit a2507e8

Browse files
bmiddhaCopilot
andauthored
Use native Node.js standard APIs (#5933)
* refactor: use native Node.js APIs Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8ebd5bf2-c44b-42d5-be25-e7936d4b0a14 * refactor: deprecate Text.replaceAll Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8ebd5bf2-c44b-42d5-be25-e7936d4b0a14 * refactor: reuse path and newline helpers Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8ebd5bf2-c44b-42d5-be25-e7936d4b0a14 --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8ebd5bf2-c44b-42d5-be25-e7936d4b0a14
1 parent 80c05e2 commit a2507e8

18 files changed

Lines changed: 86 additions & 50 deletions

File tree

apps/api-extractor/src/analyzer/SourceFileLocationFormatter.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import * as path from 'node:path';
55

66
import type * as ts from 'typescript';
77

8-
import { Path, Text } from '@rushstack/node-core-library';
8+
import { Path } from '@rushstack/node-core-library';
99

1010
export interface ISourceFileLocationFormatOptions {
1111
sourceFileLine?: number;
@@ -47,7 +47,7 @@ export class SourceFileLocationFormatter {
4747
}
4848

4949
// Convert it to a Unix-style path
50-
scrubbedPath = Text.replaceAll(scrubbedPath, '\\', '/');
50+
scrubbedPath = Path.convertToSlashes(scrubbedPath);
5151
result += scrubbedPath;
5252

5353
if (options.sourceFileLine) {

apps/api-extractor/src/api/ExtractorConfig.ts

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ import {
1717
PackageJsonLookup,
1818
type INodePackageJson,
1919
PackageName,
20-
Text,
2120
InternalError,
2221
Path,
2322
NewlineKind
@@ -1310,8 +1309,8 @@ function _expandStringWithTokens(
13101309
): string {
13111310
value = value ? value.trim() : '';
13121311
if (value !== '') {
1313-
value = Text.replaceAll(value, '<unscopedPackageName>', tokenContext.unscopedPackageName);
1314-
value = Text.replaceAll(value, '<packageName>', tokenContext.packageName);
1312+
value = value.replaceAll('<unscopedPackageName>', tokenContext.unscopedPackageName);
1313+
value = value.replaceAll('<packageName>', tokenContext.packageName);
13151314

13161315
const projectFolderToken: string = '<projectFolder>';
13171316
if (value.indexOf(projectFolderToken) === 0) {

apps/lockfile-explorer/src/graph/lfxGraphLoader.ts

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,6 @@
44
import type * as lockfileTypes from '@pnpm/lockfile.types';
55
import type * as pnpmTypes from '@pnpm/types';
66

7-
import { Text } from '@rushstack/node-core-library';
8-
97
import {
108
type ILfxGraphDependencyOptions,
119
type ILfxGraphEntryOptions,
@@ -398,9 +396,9 @@ function createPackageLockfileEntry(options: {
398396

399397
// Rewrite to:
400398
// "@rushstack/m@1.0.0; @rushstack/n@2.0.0"
401-
suffix = Text.replaceAll(suffix, ')(', '; ');
402-
suffix = Text.replaceAll(suffix, '(', '');
403-
suffix = Text.replaceAll(suffix, ')', '');
399+
suffix = suffix.replaceAll(')(', '; ');
400+
suffix = suffix.replaceAll('(', '');
401+
suffix = suffix.replaceAll(')', '');
404402
result.entrySuffix = suffix;
405403

406404
// @rushstack/l@1.0.0(@rushstack/m@1.0.0)(@rushstack/n@2.0.0)
@@ -412,10 +410,10 @@ function createPackageLockfileEntry(options: {
412410
// --> @rushstack+l@1.0.0_@rushstack+m@1.0.0_@rushstack+n@2.0.0
413411

414412
// @rushstack/l 1.0.0 (@rushstack/m@1.0.0)(@rushstack/n@2.0.0)
415-
dotPnpmSubfolder = Text.replaceAll(slashlessRawEntryId, '/', '+');
416-
dotPnpmSubfolder = Text.replaceAll(dotPnpmSubfolder, ')(', '_');
417-
dotPnpmSubfolder = Text.replaceAll(dotPnpmSubfolder, '(', '_');
418-
dotPnpmSubfolder = Text.replaceAll(dotPnpmSubfolder, ')', '');
413+
dotPnpmSubfolder = slashlessRawEntryId.replaceAll('/', '+');
414+
dotPnpmSubfolder = dotPnpmSubfolder.replaceAll(')(', '_');
415+
dotPnpmSubfolder = dotPnpmSubfolder.replaceAll('(', '_');
416+
dotPnpmSubfolder = dotPnpmSubfolder.replaceAll(')', '');
419417
}
420418

421419
// Example:

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

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -88,10 +88,8 @@ const CACHE_FOLDER: string = `${homedir()}/.rushstack/update-checks`;
8888

8989
async function _tryFetchLatestVersionAsync(packageName: string): Promise<string | undefined> {
9090
const url: string = `${REGISTRY_BASE_URL}/${encodeURIComponent(packageName)}/latest`;
91-
const controller: AbortController = new AbortController();
92-
const timeout: NodeJS.Timeout = setTimeout(() => controller.abort(), FETCH_TIMEOUT_MS);
9391
try {
94-
const response: Response = await fetch(url, { signal: controller.signal });
92+
const response: Response = await fetch(url, { signal: AbortSignal.timeout(FETCH_TIMEOUT_MS) });
9593
if (!response.ok) {
9694
return undefined;
9795
}
@@ -101,8 +99,6 @@ async function _tryFetchLatestVersionAsync(packageName: string): Promise<string
10199
} catch {
102100
// Network errors, timeouts, and parse failures are all silent.
103101
return undefined;
104-
} finally {
105-
clearTimeout(timeout);
106102
}
107103
}
108104

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

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,37 @@ describe(PackageUpdateChecker.name, () => {
120120
expect(saveSpy).not.toHaveBeenCalled();
121121
});
122122

123+
it('returns undefined when the registry request times out', async () => {
124+
loadSpy.mockRejectedValue(new Error('ENOENT'));
125+
126+
const nativeTimeout: typeof AbortSignal.timeout = AbortSignal.timeout.bind(AbortSignal);
127+
const timeoutSpy: jest.SpyInstance = jest
128+
.spyOn(AbortSignal, 'timeout')
129+
.mockImplementation(() => nativeTimeout(1));
130+
131+
let fetchSignal: AbortSignal | undefined;
132+
fetchSpy.mockImplementation(
133+
async (input: Parameters<typeof fetch>[0], init?: RequestInit): Promise<Response> => {
134+
void input;
135+
fetchSignal = init?.signal as AbortSignal;
136+
await new Promise<void>((resolve) => {
137+
fetchSignal!.addEventListener('abort', () => resolve(), { once: true });
138+
});
139+
throw fetchSignal.reason;
140+
}
141+
);
142+
143+
const checker: PackageUpdateChecker = new PackageUpdateChecker({
144+
packageName: PACKAGE_NAME,
145+
currentVersion: CURRENT_VERSION
146+
});
147+
expect(await checker.tryGetUpdateAsync()).toBeUndefined();
148+
expect(fetchSignal?.aborted).toBe(true);
149+
expect((fetchSignal?.reason as DOMException).name).toBe('TimeoutError');
150+
expect(timeoutSpy).toHaveBeenCalledWith(5000);
151+
expect(saveSpy).not.toHaveBeenCalled();
152+
});
153+
123154
it('returns undefined on non-ok HTTP response', async () => {
124155
loadSpy.mockRejectedValue(new Error('ENOENT'));
125156
fetchSpy.mockResolvedValue(makeFetchResponse('', false));

apps/rundown/src/Rundown.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import * as path from 'node:path';
66

77
import stringArgv from 'string-argv';
88

9-
import { FileSystem, PackageJsonLookup, Sort, Text } from '@rushstack/node-core-library';
9+
import { FileSystem, PackageJsonLookup, Path, Sort } from '@rushstack/node-core-library';
1010

1111
import type { IpcMessage } from './LauncherTypes';
1212

@@ -57,7 +57,7 @@ export class Rundown {
5757
importedPackageFolders.add(path.basename(importedPackageFolder));
5858
} else {
5959
const relativePath: string = path.relative(process.cwd(), importedPackageFolder);
60-
importedPackageFolders.add(Text.replaceAll(relativePath, '\\', '/'));
60+
importedPackageFolders.add(Path.convertToSlashes(relativePath));
6161
}
6262
} else {
6363
// If the importedPath does not belong to an NPM package, then rundown-snapshot.log can ignore it.
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
{
2+
"changes": [
3+
{
4+
"packageName": "@rushstack/node-core-library",
5+
"comment": "Deprecate `Text.replaceAll()` in favor of the native `String.prototype.replaceAll()` function.",
6+
"type": "minor"
7+
}
8+
],
9+
"packageName": "@rushstack/node-core-library",
10+
"email": "5100938+bmiddha@users.noreply.github.com"
11+
}

common/reviews/api/node-core-library.api.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -968,6 +968,7 @@ export class Text {
968968
static padStart(s: string, minimumLength: number, paddingCharacter?: string): string;
969969
static readLinesFromIterable(iterable: Iterable<string | Buffer | null>, options?: IReadLinesFromIterableOptions): Generator<string>;
970970
static readLinesFromIterableAsync(iterable: AsyncIterable<string | Buffer>, options?: IReadLinesFromIterableOptions): AsyncGenerator<string>;
971+
// @deprecated
971972
static replaceAll(input: string, searchValue: string, replaceValue: string): string;
972973
static reverse(s: string): string;
973974
static splitByNewLines(s: undefined): undefined;

libraries/node-core-library/src/JsonFile.ts

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -570,8 +570,7 @@ function _formatJsonHeaderComment(headerComment: string): string {
570570
if (headerComment === '') {
571571
return '';
572572
}
573-
const lines: string[] = headerComment.split('\n');
574-
const result: string[] = [];
573+
const lines: string[] = Text.convertToLf(headerComment).split('\n');
575574
for (const line of lines) {
576575
if (!/^\s*$/.test(line) && !/^\s*\/\//.test(line)) {
577576
throw new Error(
@@ -580,7 +579,6 @@ function _formatJsonHeaderComment(headerComment: string): string {
580579
JSON.stringify(line)
581580
);
582581
}
583-
result.push(Text.replaceAll(line, '\r', ''));
584582
}
585583
return lines.join('\n') + '\n';
586584
}

libraries/node-core-library/src/Text.ts

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -97,14 +97,21 @@ function* readLinesFromChunk(
9797
*/
9898
export class Text {
9999
/**
100-
* Returns the same thing as targetString.replace(searchValue, replaceValue), except that
101-
* all matches are replaced, rather than just the first match.
100+
* Replaces every occurrence of `searchValue` with `replaceValue`.
101+
*
102+
* @remarks
103+
* This method has the same behavior as {@link https://developer.mozilla.org/docs/Web/JavaScript/Reference/Global_Objects/String/replaceAll | String.prototype.replaceAll}.
104+
* In particular, if `searchValue` is an empty string, `replaceValue` is inserted before the first
105+
* UTF-16 code unit, between each code unit, and after the last code unit.
106+
*
102107
* @param input - The string to be modified
103108
* @param searchValue - The value to search for
104109
* @param replaceValue - The replacement text
110+
*
111+
* @deprecated Use `String.prototype.replaceAll()` instead.
105112
*/
106113
public static replaceAll(input: string, searchValue: string, replaceValue: string): string {
107-
return input.split(searchValue).join(replaceValue);
114+
return input.replaceAll(searchValue, replaceValue);
108115
}
109116

110117
/**

0 commit comments

Comments
 (0)