use real hashes

microsoft · Nov 6, 2024 · 474c58c · 474c58c
1 parent 1da5078
commit 474c58c
Show file tree

Hide file tree

Showing 10 changed files with 149 additions and 230 deletions.
diff --git a/change/beachball-ba7bb90f-ba01-4c9b-84ac-e054aa4db235.json b/change/beachball-ba7bb90f-ba01-4c9b-84ac-e054aa4db235.json
@@ -0,0 +1,7 @@
+{
+  "type": "patch",
+  "comment": "Replace usage of uuid library with built-in crypto.randomUUID()",
+  "packageName": "beachball",
+  "email": "[email protected]",
+  "dependentChangeType": "patch"
+}
diff --git a/change/beachball-c1b55758-27ac-4dc8-b073-8380c991c183.json b/change/beachball-c1b55758-27ac-4dc8-b073-8380c991c183.json
@@ -1,6 +1,6 @@
 {
   "type": "minor",
-  "comment": "Add option changelog.uniqueFilenames",
+  "comment": "Add option `changelog.uniqueFilenames` for adding a hash to the changelog filename based on the package name",
   "packageName": "beachball",
   "email": "[email protected]",
   "dependentChangeType": "patch"

diff --git a/package.json b/package.json
@@ -51,7 +51,6 @@
     "semver": "^7.0.0",
     "toposort": "^2.0.2",
     "p-graph": "^1.1.2",
-    "uuid": "^9.0.0",
     "workspace-tools": "^0.38.0",
     "yargs-parser": "^21.0.0"
   },
@@ -65,7 +64,6 @@
     "@types/semver": "^7.3.13",
     "@types/tmp": "^0.2.3",
     "@types/toposort": "^2.0.3",
-    "@types/uuid": "^9.0.0",
     "@types/yargs-parser": "^21.0.0",
     "find-free-port": "^2.0.0",
     "gh-pages": "^5.0.0",

diff --git a/src/__functional__/changelog/prepareChangelogPaths.test.ts b/src/__functional__/changelog/prepareChangelogPaths.test.ts
@@ -7,10 +7,12 @@ import { createTestFileStructure } from '../../__fixtures__/createTestFileStruct
 
 describe('prepareChangelogPaths', () => {
   let consoleLogMock: jest.SpiedFunction<typeof console.log>;
-  let consoleWarnMock: jest.SpiedFunction<typeof console.warn>;
   let tempDir: string | undefined;
+
   const fakeDir = slash(path.resolve('/faketmpdir'));
-  const suffixRegexp = /^[0-9a-f]{8}$/;
+  const packageName = 'test';
+  /** This is the beginning of the md5 hash digest of "test" */
+  const testHash = '098f6bcd';
 
   /** Wrapper that calls `prepareChangelogPaths` and converts the result to forward slashes */
   function prepareChangelogPathsWrapper(options: Parameters<typeof prepareChangelogPaths>[0]) {
@@ -26,11 +28,12 @@ describe('prepareChangelogPaths', () => {
 
   beforeAll(() => {
     consoleLogMock = jest.spyOn(console, 'log').mockImplementation(() => {});
-    consoleWarnMock = jest.spyOn(console, 'warn').mockImplementation(() => {});
+    // there will be a bunch of ignorable warnings because /faketmpdir doesn't exist
+    jest.spyOn(console, 'warn').mockImplementation(() => {});
   });
 
   afterEach(() => {
-    consoleWarnMock.mockClear();
+    consoleLogMock.mockClear();
     tempDir && removeTempDir(tempDir);
     tempDir = undefined;
   });
@@ -39,6 +42,7 @@ describe('prepareChangelogPaths', () => {
     const paths = prepareChangelogPathsWrapper({
       options: { generateChangelog: false, changelog: {} },
       changelogAbsDir: fakeDir,
+      packageName,
     });
 
     expect(paths).toEqual({});
@@ -48,6 +52,7 @@ describe('prepareChangelogPaths', () => {
     const paths = prepareChangelogPathsWrapper({
       options: { generateChangelog: true },
       changelogAbsDir: fakeDir,
+      packageName,
     });
 
     expect(paths).toEqual({ md: `${fakeDir}/CHANGELOG.md`, json: `${fakeDir}/CHANGELOG.json` });
@@ -57,6 +62,7 @@ describe('prepareChangelogPaths', () => {
     const paths = prepareChangelogPathsWrapper({
       options: { generateChangelog: 'md' },
       changelogAbsDir: fakeDir,
+      packageName,
     });
 
     expect(paths).toEqual({ md: `${fakeDir}/CHANGELOG.md` });
@@ -66,193 +72,146 @@ describe('prepareChangelogPaths', () => {
     const paths = prepareChangelogPathsWrapper({
       options: { generateChangelog: 'json' },
       changelogAbsDir: fakeDir,
+      packageName,
     });
 
     expect(paths).toEqual({ json: `${fakeDir}/CHANGELOG.json` });
   });
 
-  it('returns new suffixed paths if uniqueFilenames is true and no files exist', () => {
-    const paths = prepareChangelogPathsWrapper({
+  it('returns new paths with hashes if uniqueFilenames is true and no files exist', () => {
+    const options = {
       options: { generateChangelog: true, changelog: { uniqueFilenames: true } },
       changelogAbsDir: fakeDir,
-    });
+    };
+
+    const paths = prepareChangelogPathsWrapper({ ...options, packageName: 'test' });
 
     expect(paths).toEqual({
-      md: `${fakeDir}/CHANGELOG-${paths.suffix}.md`,
-      json: `${fakeDir}/CHANGELOG-${paths.suffix}.json`,
-      suffix: expect.stringMatching(suffixRegexp),
+      md: `${fakeDir}/CHANGELOG-${testHash}.md`,
+      json: `${fakeDir}/CHANGELOG-${testHash}.json`,
+    });
+
+    // hash is based on package name, not path or anything else
+    const otherPaths = prepareChangelogPathsWrapper({ ...options, packageName: 'other' });
+
+    expect(otherPaths).toEqual({
+      md: `${fakeDir}/CHANGELOG-795f3202.md`,
+      json: `${fakeDir}/CHANGELOG-795f3202.json`,
     });
   });
 
-  it('returns new suffixed md path if uniqueFilenames is true and generateChangelog is "md"', () => {
+  it('returns new md path with hash if uniqueFilenames is true and generateChangelog is "md"', () => {
     const paths = prepareChangelogPathsWrapper({
       options: { generateChangelog: 'md', changelog: { uniqueFilenames: true } },
       changelogAbsDir: fakeDir,
+      packageName,
     });
 
     expect(paths).toEqual({
-      md: `${fakeDir}/CHANGELOG-${paths.suffix}.md`,
-      suffix: expect.stringMatching(suffixRegexp),
+      md: `${fakeDir}/CHANGELOG-${testHash}.md`,
     });
   });
 
-  it('returns new suffixed json path if uniqueFilenames is true and generateChangelog is "json"', () => {
+  it('returns new json path with hash if uniqueFilenames is true and generateChangelog is "json"', () => {
     const paths = prepareChangelogPathsWrapper({
       options: { generateChangelog: 'json', changelog: { uniqueFilenames: true } },
       changelogAbsDir: fakeDir,
+      packageName,
     });
 
     expect(paths).toEqual({
-      json: `${fakeDir}/CHANGELOG-${paths.suffix}.json`,
-      suffix: expect.stringMatching(suffixRegexp),
+      json: `${fakeDir}/CHANGELOG-${testHash}.json`,
     });
   });
 
-  it('detects existing suffixed files if uniqueFilenames is true', () => {
-    const suffix = 'abcdef12';
+  it('migrates existing non-hash file to path with hash (uniqueFilenames false to true)', () => {
     tempDir = createTestFileStructure({
-      [`CHANGELOG-${suffix}.md`]: 'existing md',
-      [`CHANGELOG-${suffix}.json`]: {},
+      'CHANGELOG.md': 'existing md',
+      'CHANGELOG.json': {},
     });
 
     const paths = prepareChangelogPathsWrapper({
       options: { generateChangelog: true, changelog: { uniqueFilenames: true } },
       changelogAbsDir: tempDir,
+      packageName,
     });
+
     expect(paths).toEqual({
-      md: `${tempDir}/CHANGELOG-${suffix}.md`,
-      json: `${tempDir}/CHANGELOG-${suffix}.json`,
-      suffix,
+      md: `${tempDir}/CHANGELOG-${testHash}.md`,
+      json: `${tempDir}/CHANGELOG-${testHash}.json`,
     });
-  });
 
-  it('detects suffix from existing md file if uniqueFilenames is true', () => {
-    // only the md file is present, but its suffix will be used for both files
-    const suffix = 'abcdef12';
-    tempDir = createTestFileStructure({
-      [`CHANGELOG-${suffix}.md`]: 'existing md',
-    });
+    expect(fs.existsSync(`${tempDir}/CHANGELOG.md`)).toBe(false);
+    expect(fs.readFileSync(paths.md!, 'utf8')).toBe('existing md');
 
-    const pathsWithBoth = prepareChangelogPathsWrapper({
-      options: { generateChangelog: true, changelog: { uniqueFilenames: true } },
-      changelogAbsDir: tempDir,
-    });
-    expect(pathsWithBoth).toEqual({
-      md: `${tempDir}/CHANGELOG-${suffix}.md`,
-      json: `${tempDir}/CHANGELOG-${suffix}.json`,
-      suffix,
-    });
+    expect(fs.existsSync(`${tempDir}/CHANGELOG.json`)).toBe(false);
+    expect(fs.readFileSync(paths.json!, 'utf8')).toBe('{}');
+
+    expect(consoleLogMock).toHaveBeenCalledWith(expect.stringContaining('Renamed existing changelog file'));
+    expect(consoleLogMock).toHaveBeenCalledTimes(2);
   });
 
-  it('detects suffix from existing json file if uniqueFilenames is true', () => {
-    // only the json file is present, but its suffix will be used for both files
-    const suffix = 'abcdef12';
+  it('migrates existing path with hash to non-hash path (uniqueFilenames true to false)', () => {
+    const oldName = 'CHANGELOG-abcdef08';
     tempDir = createTestFileStructure({
-      [`CHANGELOG-${suffix}.json`]: {},
+      [`${oldName}.md`]: 'existing md',
+      [`${oldName}.json`]: {},
     });
 
     const paths = prepareChangelogPathsWrapper({
-      options: { generateChangelog: true, changelog: { uniqueFilenames: true } },
+      options: { generateChangelog: true },
       changelogAbsDir: tempDir,
+      packageName,
     });
+
     expect(paths).toEqual({
-      md: `${tempDir}/CHANGELOG-${suffix}.md`,
-      json: `${tempDir}/CHANGELOG-${suffix}.json`,
-      suffix,
+      md: `${tempDir}/CHANGELOG.md`,
+      json: `${tempDir}/CHANGELOG.json`,
     });
-  });
 
-  it('uses newer suffix if uniqueFilenames is true and md/json files have different suffixes', async () => {
-    const suffixJson = 'abcdef34';
-    const suffixMd = 'abcdef12';
-    tempDir = createTestFileStructure({
-      [`CHANGELOG-${suffixJson}.json`]: {},
-    });
-    // ensure different timestamps by waiting 1ms
-    await new Promise(resolve => setTimeout(resolve, 1));
-    fs.writeFileSync(path.join(tempDir, `CHANGELOG-${suffixMd}.md`), 'existing md');
+    expect(fs.existsSync(`${tempDir}/${oldName}.md`)).toBe(false);
+    expect(fs.readFileSync(paths.md!, 'utf8')).toBe('existing md');
 
-    const paths = prepareChangelogPathsWrapper({
-      options: { generateChangelog: true, changelog: { uniqueFilenames: true } },
-      changelogAbsDir: tempDir,
-    });
-    // md suffix is preferred for both files because the file is newer
-    expect(paths).toEqual({
-      md: `${tempDir}/CHANGELOG-${suffixMd}.md`,
-      json: `${tempDir}/CHANGELOG-${suffixMd}.json`,
-      suffix: suffixMd,
-    });
-    expect(consoleWarnMock).toHaveBeenCalledWith(
-      expect.stringContaining('Found changelog files with multiple suffixes')
-    );
+    expect(fs.existsSync(`${tempDir}/${oldName}.json`)).toBe(false);
+    expect(fs.readFileSync(paths.json!, 'utf8')).toBe('{}');
+
+    expect(consoleLogMock).toHaveBeenCalledWith(expect.stringContaining('Renamed existing changelog file'));
+    expect(consoleLogMock).toHaveBeenCalledTimes(2);
   });
 
-  it('uses newest suffix if uniqueFilenames is true and there are multiple suffixed files', async () => {
-    const lastSuffix = 'abcdef12';
+  it('renames newest file if uniqueFilenames is true and there are multiple files with hashes', async () => {
+    const file1 = 'CHANGELOG-12345678.md';
+    const file2 = 'CHANGELOG-fbcd40ef.md';
+    const lastHash = 'abcdef12';
     tempDir = createTestFileStructure({
-      'CHANGELOG-12345678.md': 'existing md',
-      'CHANGELOG-abcd40ef.md': 'existing md',
+      [file1]: 'md 1',
+      [file2]: 'md 2',
     });
     // ensure different timestamps by waiting 1ms
     await new Promise(resolve => setTimeout(resolve, 1));
-    fs.writeFileSync(path.join(tempDir, `CHANGELOG-${lastSuffix}.md`), 'existing md');
-
-    const paths = prepareChangelogPathsWrapper({
-      options: { generateChangelog: true, changelog: { uniqueFilenames: true } },
-      changelogAbsDir: tempDir,
-    });
-    expect(paths).toEqual({
-      md: `${tempDir}/CHANGELOG-${lastSuffix}.md`,
-      json: `${tempDir}/CHANGELOG-${lastSuffix}.json`,
-      suffix: lastSuffix,
-    });
-    expect(consoleWarnMock).toHaveBeenCalledWith(
-      expect.stringContaining('Found changelog files with multiple suffixes')
-    );
-  });
-
-  it('migrates existing non-suffixed file to suffixed path', () => {
-    tempDir = createTestFileStructure({
-      'CHANGELOG.md': 'existing md',
-    });
+    fs.writeFileSync(path.join(tempDir, `CHANGELOG-${lastHash}.md`), 'last md');
 
     const paths = prepareChangelogPathsWrapper({
       options: { generateChangelog: true, changelog: { uniqueFilenames: true } },
       changelogAbsDir: tempDir,
+      packageName,
     });
 
+    // Paths use the actual hash of "test"
     expect(paths).toEqual({
-      md: `${tempDir}/CHANGELOG-${paths.suffix}.md`,
-      json: `${tempDir}/CHANGELOG-${paths.suffix}.json`,
-      suffix: expect.stringMatching(suffixRegexp),
-    });
-    expect(fs.existsSync(`${tempDir}/CHANGELOG.md`)).toBe(false);
-    expect(fs.readFileSync(paths.md!, 'utf8')).toBe('existing md');
-
-    expect(consoleLogMock).toHaveBeenCalledWith(
-      expect.stringContaining('Renamed existing non-suffixed changelog file')
-    );
-  });
-
-  it('migrates existing suffixed file to non-suffixed path', () => {
-    const suffixedName = 'CHANGELOG-abcdef08.md';
-    tempDir = createTestFileStructure({
-      [suffixedName]: 'existing md',
-    });
-
-    const paths = prepareChangelogPathsWrapper({
-      options: { generateChangelog: true },
-      changelogAbsDir: tempDir,
+      md: `${tempDir}/CHANGELOG-${testHash}.md`,
+      json: `${tempDir}/CHANGELOG-${testHash}.json`,
     });
 
-    expect(paths).toEqual({
-      md: `${tempDir}/CHANGELOG.md`,
-      json: `${tempDir}/CHANGELOG.json`,
-    });
+    // The most recently modified file is renamed
+    expect(fs.existsSync(`${tempDir}/CHANGELOG-${lastHash}.md`)).toBe(false);
+    expect(fs.readFileSync(paths.md!, 'utf8')).toBe('last md');
 
-    expect(fs.existsSync(`${tempDir}/${suffixedName}`)).toBe(false);
-    expect(fs.readFileSync(paths.md!, 'utf8')).toBe('existing md');
+    expect(consoleLogMock).toHaveBeenCalledWith(expect.stringContaining('Renamed existing changelog file'));
+    expect(consoleLogMock).toHaveBeenCalledTimes(1);
 
-    expect(consoleLogMock).toHaveBeenCalledWith(expect.stringContaining('Renamed existing suffixed changelog file'));
+    // The other files are untouched
+    expect(fs.readFileSync(`${tempDir}/${file1}`, 'utf8')).toBe('md 1');
+    expect(fs.readFileSync(`${tempDir}/${file2}`, 'utf8')).toBe('md 2');
   });
 });
diff --git a/src/__functional__/changelog/writeChangelog.test.ts b/src/__functional__/changelog/writeChangelog.test.ts
@@ -1,6 +1,5 @@
 import { describe, expect, it, beforeAll, afterAll, afterEach } from '@jest/globals';
 import fs from 'fs-extra';
-import path from 'path';
 import { generateChangeFiles, getChange, fakeEmail as author } from '../../__fixtures__/changeFiles';
 import {
   readChangelogJson,
@@ -11,7 +10,6 @@ import {
 import { initMockLogs } from '../../__fixtures__/mockLogs';
 import { RepositoryFactory } from '../../__fixtures__/repositoryFactory';
 import { writeChangelog } from '../../changelog/writeChangelog';
-import { _getExistingSuffixedChangelogs } from '../../changelog/prepareChangelogPaths';
 import { getPackageInfos } from '../../monorepo/getPackageInfos';
 import { readChangeFiles } from '../../changefile/readChangeFiles';
 import type { BeachballOptions } from '../../types/BeachballOptions';
@@ -513,7 +511,7 @@ describe('writeChangelog', () => {
     });
   });
 
-  it('appends to existing changelog when migrating from non-suffixed to suffixed changelog filenames', async () => {
+  it('appends to existing changelog when migrating from uniqueFilenames=false to true', async () => {
     repo = sharedSingleRepo;
     const options = getOptions();
 
@@ -537,16 +535,14 @@ describe('writeChangelog', () => {
 
     // Verify the old changelog is moved
     expect(readChangelogMd(repo.rootPath)).toBeNull();
-    // Get suffixed changelog paths
-    const paths = _getExistingSuffixedChangelogs(repo.rootPath);
-    expect(paths).toMatchObject({ md: expect.any(String), json: expect.any(String) });
 
     // Read the changelogs again and verify that the previous content is still there
-    const secondChangelogMd = readChangelogMd(repo.rootPath, path.basename(paths.md!));
+    // ("acbd18db" is the start of the md5 hash digest of "foo")
+    const secondChangelogMd = readChangelogMd(repo.rootPath, 'CHANGELOG-acbd18db.md');
     expect(secondChangelogMd).toContain('extra change');
     expect(secondChangelogMd).toContain(trimChangelogMd(firstChangelogMd!));
 
-    const secondChangelogJson = readChangelogJson(repo.rootPath, path.basename(paths.json!));
+    const secondChangelogJson = readChangelogJson(repo.rootPath, 'CHANGELOG-acbd18db.json');
     expect(secondChangelogJson).toEqual({
       name: 'foo',
       entries: [expect.anything(), firstChangelogJson!.entries[0]],