Revert "Temporarily revert npm-registry-fetch usage (#1145)"

This reverts commit f666c1e.

Author: ecraig12345

README.md

@@ -108,7 +108,7 @@ beachball publish -r http://localhost:4873 -t beta
 
 ### Overriding concurrency
 
-In large monorepos, the process of fetching versions for sync or before publishing can be time-consuming due to the high number of packages. To optimize performance, you can override the concurrency for fetching from the registry by setting `options.npmReadConcurrency` (default: 5). You can also increase concurrency for hook calls and publish operations via `options.concurrency` (default: 1; respects topological order).
+In large monorepos, the process of fetching versions for sync or before publishing can be time-consuming due to the high number of packages. To optimize performance, you can override the concurrency for fetching from the registry by setting `options.npmReadConcurrency` (default: 10). You can also increase concurrency for hook calls and publish operations via `options.concurrency` (default: 1; respects topological order).
 
 ### API surface
 

docs/overview/configuration.md

@@ -91,7 +91,7 @@ For the latest full list of supported options, see `RepoOptions` [in this file](
 | `groupChanges` | `boolean` | `false` | repo | Write multiple changes to a single change file |
 | `hooks` | [`HooksOptions`][4] | | repo | Hooks for custom pre/post publish actions |
 | `ignorePatterns` | `string[]` | | repo | Ignore changes in files matching these glob patterns ([see notes][6]) |
-| `npmReadConcurrency` | number | 5 | repo | Maximum concurrency for fetching package versions from the registry (see `concurrency` for write operations) |
+| `npmReadConcurrency` | number | 10 | repo | Maximum concurrency for fetching package versions from the registry (see `concurrency` for write operations) |
 | `package` | `string` | | repo | Specifies which package the command relates to (overrides change detection based on `git diff`) |
 | `prereleasePrefix` | `string` | | repo | Prerelease prefix, e.g. `"beta"`. Note that if this is specified, packages with change type major/minor/patch will be bumped as prerelease instead. |
 | `packStyle` | `'sequential' \| 'layer'` | `'sequential'` | repo | With `packToPath`, how to organize the tgz files. `'sequential'` uses numeric prefixes to ensure topological ordering. `'layer'` groups the packages into numbered subfolders based on dependency tree layers. |

package.json

@@ -55,6 +55,7 @@
     "cosmiconfig": "^9.0.1",
     "execa": "^5.1.1",
     "minimatch": "^3.1.5",
+    "npm-registry-fetch": "^14.0.0",
     "p-graph": "^1.3.0",
     "p-limit": "^3.1.0",
     "prompts": "^2.4.2",
@@ -66,6 +67,7 @@
     "@jest/globals": "^29.0.0",
     "@types/minimatch": "^3.0.0",
     "@types/node": "^22.0.0",
+    "@types/npm-registry-fetch": "^8.0.9",
     "@types/prompts": "^2.4.2",
     "@types/semver": "^7.3.13",
     "@types/tmp": "^0.2.3",

src/__e2e__/publishE2E.test.ts

@@ -28,7 +28,7 @@ import { validate } from '../validation/validate';
 // If an issue is found in the future that could only be caught by this test using real npm,
 // a new test file with a real registry should be created to cover that specific scenario.
 jest.mock('../packageManager/npm');
-// jest.mock('npm-registry-fetch');
+jest.mock('npm-registry-fetch');
 
 describe('publish command (e2e)', () => {
   const npmMock = initNpmMock();

src/__e2e__/publishRegistry.test.ts

@@ -23,7 +23,7 @@ import { validate } from '../validation/validate';
 // If an issue is found in the future that could only be caught by this test using real npm,
 // a new test file with a real registry should be created to cover that specific scenario.
 jest.mock('../packageManager/npm');
-// jest.mock('npm-registry-fetch');
+jest.mock('npm-registry-fetch');
 
 describe('publish command (registry)', () => {
   const npmMock = initNpmMock();
@@ -193,8 +193,7 @@ describe('publish command (registry)', () => {
 
     expect(npmMock.getPublishedPackage('foo')!.version).toEqual('1.1.0');
     expect(npmMock.getPublishedPackage('bar')!.version).toEqual('1.4.0');
-    expect(npmMock.mock).toHaveBeenCalledTimes(4);
-    // expect(npmMock.mock).toHaveBeenCalledTimes(2);
+    expect(npmMock.mock).toHaveBeenCalledTimes(2);
     expect(logs.mocks.error).not.toHaveBeenCalled();
   });
 

src/__e2e__/syncE2E.test.ts

@@ -18,7 +18,7 @@ import { getScopedPackages } from '../monorepo/getScopedPackages';
 // If an issue is found in the future that could only be caught by this test using real npm,
 // a new test file with a real registry should be created to cover that specific scenario.
 jest.mock('../packageManager/npm');
-// jest.mock('npm-registry-fetch');
+jest.mock('npm-registry-fetch');
 
 describe('sync command (e2e)', () => {
   const mockNpm = initNpmMock();

src/__fixtures__/mockNpm.test.ts

@@ -2,23 +2,16 @@
 // But this added complexity greatly speeds up the other npm-related tests by removing the
 // dependency on actual npm CLI calls and a fake registry (which are very slow).
 
-import { afterEach, beforeAll, describe, expect, it, jest } from '@jest/globals';
+import { afterAll, afterEach, beforeAll, describe, expect, it, jest } from '@jest/globals';
 import fs from 'fs';
-// import fetch from 'npm-registry-fetch';
+import fetch from 'npm-registry-fetch';
 import { type NpmResult, npm } from '../packageManager/npm';
 import type { PackageJson } from '../types/PackageInfo';
-import {
-  initNpmMock,
-  _makeRegistryData,
-  _mockNpmPack,
-  _mockNpmPublish,
-  _mockNpmShow,
-  type MockNpmResult,
-} from './mockNpm';
+import { initNpmMock, _makeRegistryData, _mockNpmPack, _mockNpmPublish, type MockNpmResult } from './mockNpm';
 import * as readJsonModule from '../object/readJson';
 
 jest.mock('fs');
-// jest.mock('npm-registry-fetch');
+jest.mock('npm-registry-fetch');
 jest.mock('../object/readJson');
 jest.mock('../packageManager/npm');
 
@@ -93,94 +86,6 @@ describe('_makeRegistryData', () => {
   });
 });
 
-describe('_mockNpmShow', () => {
-  function getErrorResult(errorMessage: string) {
-    return {
-      stdout: '',
-      stderr: errorMessage,
-      all: errorMessage,
-      success: false,
-      failed: true,
-    } as NpmResult;
-  }
-
-  function getShowResult(params: { name: string; version: string }) {
-    const { name, version } = params;
-    const output = JSON.stringify({
-      ...data[name].versions[version],
-      'dist-tags': data[name]['dist-tags'],
-      versions: Object.keys(data[name].versions),
-    });
-
-    return {
-      stdout: output,
-      stderr: '',
-      all: output,
-      success: true,
-      failed: false,
-    } as NpmResult;
-  }
-
-  const data = _makeRegistryData({
-    foo: {
-      versions: ['1.0.0-beta', '1.0.0', '1.0.1'],
-      'dist-tags': { latest: '1.0.1', beta: '1.0.0-beta' },
-    },
-    '@foo/bar': {
-      versions: ['2.0.0-beta', '2.0.0', '2.0.1'],
-      'dist-tags': { latest: '2.0.1', beta: '2.0.0-beta' },
-    },
-  });
-
-  it("errors if package doesn't exist", async () => {
-    const emptyData = _makeRegistryData({});
-    const result = await _mockNpmShow(emptyData, ['foo'], { cwd: undefined });
-    expect(result).toEqual(getErrorResult('[fake] code E404 - foo - not found'));
-  });
-
-  it('returns requested version plus dist-tags and version list', async () => {
-    const result = await _mockNpmShow(data, ['foo@1.0.0'], { cwd: undefined });
-    expect(result).toEqual(getShowResult({ name: 'foo', version: '1.0.0' }));
-  });
-
-  it('returns requested version of scoped package', async () => {
-    const result = await _mockNpmShow(data, ['@foo/bar@2.0.0'], { cwd: undefined });
-    expect(result).toEqual(getShowResult({ name: '@foo/bar', version: '2.0.0' }));
-  });
-
-  it('returns requested tag', async () => {
-    const result = await _mockNpmShow(data, ['foo@beta'], { cwd: undefined });
-    expect(result).toEqual(getShowResult({ name: 'foo', version: '1.0.0-beta' }));
-  });
-
-  it('returns requested tag of scoped package', async () => {
-    const result = await _mockNpmShow(data, ['@foo/bar@beta'], { cwd: undefined });
-    expect(result).toEqual(getShowResult({ name: '@foo/bar', version: '2.0.0-beta' }));
-  });
-
-  it('returns latest version if no version requested', async () => {
-    const result = await _mockNpmShow(data, ['foo'], { cwd: undefined });
-    expect(result).toEqual(getShowResult({ name: 'foo', version: '1.0.1' }));
-  });
-
-  it('returns latest version of scoped package if no version requested', async () => {
-    const result = await _mockNpmShow(data, ['@foo/bar'], { cwd: undefined });
-    expect(result).toEqual(getShowResult({ name: '@foo/bar', version: '2.0.1' }));
-  });
-
-  it("errors if requested version doesn't exist", async () => {
-    const result = await _mockNpmShow(data, ['foo@2.0.0'], { cwd: undefined });
-    expect(result).toEqual(getErrorResult('[fake] code E404 - foo@2.0.0 - not found'));
-  });
-
-  // support for this could be added later
-  it('currently throws if requested version is a range', async () => {
-    await expect(() => _mockNpmShow(data, ['foo@^1.0.0'], { cwd: undefined })).rejects.toThrow(
-      /not currently supported/
-    );
-  });
-});
-
 describe('_mockNpmPublish', () => {
   function getPublishResult(params: { error?: string; tag?: string }) {
     const { error, tag } = params;
@@ -362,25 +267,29 @@ describe('mockNpm', () => {
     packageJson = undefined;
   });
 
-  // describe('mockFetchJson', () => {
-  //   it('mocks registry fetch', async () => {
-  //     npmMock.setRegistryData({ foo: { versions: ['1.0.0'] } });
-  //     expect(fetch.json).toHaveProperty('mock');
-  //     const result = await fetch.json('/foo');
-  //     expect(result).toEqual({
-  //       name: 'foo',
-  //       modified: expect.any(String),
-  //       versions: { '1.0.0': { name: 'foo', version: '1.0.0' } },
-  //       'dist-tags': { latest: '1.0.0' },
-  //     });
-  //   });
-
-  //   it('resets calls and registry after each test', () => {
-  //     expect(npmMock.mockFetchJson).not.toHaveBeenCalled();
-  //     // registry data for foo was set in the previous test but should have been cleared
-  //     expect(() => fetch.json('/foo')).toThrow('404 Not Found');
-  //   });
-  // });
+  afterAll(() => {
+    jest.restoreAllMocks();
+  });
+
+  describe('mockFetchJson', () => {
+    it('mocks registry fetch', async () => {
+      npmMock.setRegistryData({ foo: { versions: ['1.0.0'] } });
+      expect(fetch.json).toHaveProperty('mock');
+      const result = await fetch.json('/foo');
+      expect(result).toEqual({
+        name: 'foo',
+        modified: expect.any(String),
+        versions: { '1.0.0': { name: 'foo', version: '1.0.0' } },
+        'dist-tags': { latest: '1.0.0' },
+      });
+    });
+
+    it('resets calls and registry after each test', () => {
+      expect(npmMock.mockFetchJson).not.toHaveBeenCalled();
+      // registry data for foo was set in the previous test but should have been cleared
+      expect(() => fetch.json('/foo')).toThrow('404 Not Found');
+    });
+  });
 
   describe('getPublishedVersions', () => {
     it('gets data for a package', () => {
@@ -433,23 +342,14 @@ describe('mockNpm', () => {
         versions: ['1.0.0'],
         'dist-tags': { latest: '1.0.0' },
       });
-      expect(await npm(['show', 'foo'], { cwd: undefined })).toMatchObject({
-        success: true,
-        stdout: JSON.stringify({
-          name: 'foo',
-          version: '1.0.0',
-          'dist-tags': { latest: '1.0.0' },
-          versions: ['1.0.0'],
-        }),
+      expect(await fetch.json('/foo')).toEqual({
+        name: 'foo',
+        modified: expect.any(String),
+        versions: {
+          '1.0.0': { name: 'foo', version: '1.0.0' },
+        },
+        'dist-tags': { latest: '1.0.0' },
       });
-      // expect(await fetch.json('/foo')).toEqual({
-      //   name: 'foo',
-      //   modified: expect.any(String),
-      //   versions: {
-      //     '1.0.0': { name: 'foo', version: '1.0.0' },
-      //   },
-      //   'dist-tags': { latest: '1.0.0' },
-      // });
     });
   });
 
@@ -494,15 +394,6 @@ describe('mockNpm', () => {
       expect(npmMock.mock).toHaveBeenCalledTimes(1);
       expect(npmMock.mock).toHaveBeenCalledWith(['foo'], expect.objectContaining({ cwd: undefined }));
     });
-
-    it('TEMP mocks npm show command', async () => {
-      npmMock.setRegistryData({ foo: { versions: ['1.0.0'] } });
-      const result = await npm(['show', 'foo'], { cwd: undefined });
-      expect(result).toMatchObject({
-        success: true,
-        stdout: expect.stringContaining('"name":"foo"'),
-      });
-    });
   });
 
   describe('setCommandOverride', () => {