Move getQuestionsForPackage to its own file (#1006)

change/beachball-fbe57b6e-69d7-4628-8c28-6000d9e9449b.json

@@ -0,0 +1,7 @@
+{
+  "type": "patch",
+  "comment": "Internal refactor: getQuestionsForPackage to its own file",
+  "packageName": "beachball",
+  "email": "elcraig@microsoft.com",
+  "dependentChangeType": "patch"
+}

src/__tests__/changefile/getQuestionsForPackage.test.ts

@@ -1,6 +1,6 @@
 import { describe, expect, it, jest } from '@jest/globals';
 import prompts from 'prompts';
-import { _getQuestionsForPackage } from '../../changefile/promptForChange';
+import { getQuestionsForPackage } from '../../changefile/getQuestionsForPackage';
 import { ChangeFilePromptOptions } from '../../types/ChangeFilePrompt';
 import { initMockLogs } from '../../__fixtures__/mockLogs';
 import { makePackageInfos } from '../../__fixtures__/packageInfos';
@@ -8,12 +8,12 @@ import { makePackageInfos } from '../../__fixtures__/packageInfos';
 /**
  * This covers the first part of `promptForChange`: determining what questions to ask for each package.
  */
-describe('promptForChange _getQuestionsForPackage', () => {
+describe('getQuestionsForPackage', () => {
   /** Package name used in the tests */
   const pkg = 'foo';
 
-  /** Basic params for `_getQuestionsForPackage`, for a package named `foo` */
-  const defaultQuestionsParams: Parameters<typeof _getQuestionsForPackage>[0] = {
+  /** Basic params for `getQuestionsForPackage`, for a package named `foo` */
+  const defaultQuestionsParams: Parameters<typeof getQuestionsForPackage>[0] = {
     pkg,
     packageInfos: makePackageInfos({ [pkg]: {} }),
     packageGroups: {},
@@ -24,7 +24,7 @@ describe('promptForChange _getQuestionsForPackage', () => {
   const logs = initMockLogs();
 
   it('works in basic case', () => {
-    const questions = _getQuestionsForPackage(defaultQuestionsParams);
+    const questions = getQuestionsForPackage(defaultQuestionsParams);
     expect(questions).toEqual([
       {
         choices: [
@@ -50,7 +50,7 @@ describe('promptForChange _getQuestionsForPackage', () => {
 
   // it's somewhat debatable if this is correct (maybe --type should be the override for disallowedChangeTypes?)
   it('errors if options.type is disallowed', () => {
-    const questions = _getQuestionsForPackage({
+    const questions = getQuestionsForPackage({
       ...defaultQuestionsParams,
       packageInfos: makePackageInfos({ [pkg]: { combinedOptions: { disallowedChangeTypes: ['major'] } } }),
       options: { type: 'major', message: '' },
@@ -60,7 +60,7 @@ describe('promptForChange _getQuestionsForPackage', () => {
   });
 
   it('errors if there are no valid change types for package', () => {
-    const questions = _getQuestionsForPackage({
+    const questions = getQuestionsForPackage({
       ...defaultQuestionsParams,
       packageInfos: makePackageInfos({
         [pkg]: { combinedOptions: { disallowedChangeTypes: ['major', 'minor', 'patch', 'none'] } },
@@ -71,7 +71,7 @@ describe('promptForChange _getQuestionsForPackage', () => {
   });
 
   it('respects disallowedChangeTypes', () => {
-    const questions = _getQuestionsForPackage({
+    const questions = getQuestionsForPackage({
       ...defaultQuestionsParams,
       packageInfos: makePackageInfos({ [pkg]: { combinedOptions: { disallowedChangeTypes: ['major'] } } }),
     });
@@ -80,7 +80,7 @@ describe('promptForChange _getQuestionsForPackage', () => {
   });
 
   it('allows prerelease change for package with prerelease version', () => {
-    const questions = _getQuestionsForPackage({
+    const questions = getQuestionsForPackage({
       ...defaultQuestionsParams,
       packageInfos: makePackageInfos({ [pkg]: { version: '1.0.0-beta.1' } }),
     });
@@ -90,7 +90,7 @@ describe('promptForChange _getQuestionsForPackage', () => {
 
   // this is a bit weird as well, but documenting current behavior
   it('excludes prerelease if disallowed', () => {
-    const questions = _getQuestionsForPackage({
+    const questions = getQuestionsForPackage({
       ...defaultQuestionsParams,
       packageInfos: makePackageInfos({
         [pkg]: { version: '1.0.0-beta.1', combinedOptions: { disallowedChangeTypes: ['prerelease'] } },
@@ -101,7 +101,7 @@ describe('promptForChange _getQuestionsForPackage', () => {
   });
 
   it('excludes the change type question when options.type is specified', () => {
-    const questions = _getQuestionsForPackage({
+    const questions = getQuestionsForPackage({
       ...defaultQuestionsParams,
       options: { type: 'patch', message: '' },
     });
@@ -110,7 +110,7 @@ describe('promptForChange _getQuestionsForPackage', () => {
   });
 
   it('excludes the change type question with only one valid option', () => {
-    const questions = _getQuestionsForPackage({
+    const questions = getQuestionsForPackage({
       ...defaultQuestionsParams,
       packageInfos: makePackageInfos({
         [pkg]: { combinedOptions: { disallowedChangeTypes: ['major', 'minor', 'none'] } },
@@ -121,7 +121,7 @@ describe('promptForChange _getQuestionsForPackage', () => {
   });
 
   it('excludes the change type question when prerelease is implicitly the only valid option', () => {
-    const questions = _getQuestionsForPackage({
+    const questions = getQuestionsForPackage({
       ...defaultQuestionsParams,
       packageInfos: makePackageInfos({
         [pkg]: {
@@ -135,7 +135,7 @@ describe('promptForChange _getQuestionsForPackage', () => {
   });
 
   it('excludes the comment question when options.message is set', () => {
-    const questions = _getQuestionsForPackage({
+    const questions = getQuestionsForPackage({
       ...defaultQuestionsParams,
       options: { message: 'message' },
     });
@@ -147,7 +147,7 @@ describe('promptForChange _getQuestionsForPackage', () => {
     const customQuestions: prompts.PromptObject[] = [{ name: 'custom', message: 'custom prompt', type: 'text' }];
     const changePrompt: ChangeFilePromptOptions['changePrompt'] = jest.fn(() => customQuestions);
 
-    const questions = _getQuestionsForPackage({
+    const questions = getQuestionsForPackage({
       ...defaultQuestionsParams,
       packageInfos: makePackageInfos({ [pkg]: { combinedOptions: { changeFilePrompt: { changePrompt } } } }),
     });
@@ -166,7 +166,7 @@ describe('promptForChange _getQuestionsForPackage', () => {
   it('does case-insensitive filtering on description suggestions', async () => {
     const recentMessages = ['Foo', 'Bar', 'Baz'];
     const recentMessageChoices = [{ title: 'Foo' }, { title: 'Bar' }, { title: 'Baz' }];
-    const questions = _getQuestionsForPackage({ ...defaultQuestionsParams, recentMessages });
+    const questions = getQuestionsForPackage({ ...defaultQuestionsParams, recentMessages });
     expect(questions).toEqual([
       expect.anything(),
       expect.objectContaining({

src/__tests__/changefile/promptForChange.test.ts

@@ -3,7 +3,6 @@ import prompts from 'prompts';
 import {
   promptForChange,
   _getChangeFileInfoFromResponse,
-  _getQuestionsForPackage,
   _promptForPackageChange,
 } from '../../changefile/promptForChange';
 import { ChangeFilePromptOptions } from '../../types/ChangeFilePrompt';
@@ -16,15 +15,17 @@ import { makePackageInfos } from '../../__fixtures__/packageInfos';
 // so instead we inject a custom mock stdout stream, as well as stdin for entering answers
 let stdin: MockStdin;
 let stdout: MockStdout;
-jest.mock(
-  'prompts',
-  () =>
-    ((questions, options) => {
-      questions = Array.isArray(questions) ? questions : [questions];
-      questions = questions.map(q => ({ ...q, stdin, stdout }));
-      return (jest.requireActual('prompts') as typeof prompts)(questions, options);
-    }) as typeof prompts
-);
+jest.mock('prompts', () => {
+  const realPrompts = jest.requireActual('prompts') as typeof prompts;
+
+  return ((questions, options) => {
+    const questionsArr = Array.isArray(questions) ? questions : [questions];
+    return realPrompts(
+      questionsArr.map(q => ({ ...q, stdin, stdout })),
+      options
+    );
+  }) as typeof prompts;
+});
 
 /**
  * These combined tests mainly cover various early bail-out cases (the only meaningful logic in

src/__tests__/changefile/promptForChange_promptForPackageChange.test.ts

@@ -1,24 +1,27 @@
 import { afterEach, beforeEach, describe, expect, it, jest } from '@jest/globals';
 import prompts from 'prompts';
-import { _getQuestionsForPackage, _promptForPackageChange } from '../../changefile/promptForChange';
+import { _promptForPackageChange } from '../../changefile/promptForChange';
 import { initMockLogs } from '../../__fixtures__/mockLogs';
 import { MockStdin } from '../../__fixtures__/mockStdin';
 import { MockStdout } from '../../__fixtures__/mockStdout';
 import { makePackageInfos } from '../../__fixtures__/packageInfos';
+import { getQuestionsForPackage } from '../../changefile/getQuestionsForPackage';
 
 // prompts writes to stdout (not console) in a way that can't really be mocked with spies,
 // so instead we inject a custom mock stdout stream, as well as stdin for entering answers
 let stdin: MockStdin;
 let stdout: MockStdout;
-jest.mock(
-  'prompts',
-  () =>
-    ((questions, options) => {
-      questions = Array.isArray(questions) ? questions : [questions];
-      questions = questions.map(q => ({ ...q, stdin, stdout }));
-      return (jest.requireActual('prompts') as typeof prompts)(questions, options);
-    }) as typeof prompts
-);
+jest.mock('prompts', () => {
+  const realPrompts = jest.requireActual('prompts') as typeof prompts;
+
+  return ((questions, options) => {
+    const questionsArr = Array.isArray(questions) ? questions : [questions];
+    return realPrompts(
+      questionsArr.map(q => ({ ...q, stdin, stdout })),
+      options
+    );
+  }) as typeof prompts;
+});
 
 /**
  * This covers the actual prompting part of `promptForChange`.
@@ -27,8 +30,8 @@ describe('promptForChange _promptForPackageChange', () => {
   /** Package name used in the tests */
   const pkg = 'foo';
 
-  /** Basic params for `_getQuestionsForPackage`, for a package named `foo` */
-  const defaultQuestionsParams: Parameters<typeof _getQuestionsForPackage>[0] = {
+  /** Basic params for `getQuestionsForPackage`, for a package named `foo` */
+  const defaultQuestionsParams: Parameters<typeof getQuestionsForPackage>[0] = {
     pkg,
     packageInfos: makePackageInfos({ [pkg]: {} }),
     packageGroups: {},
@@ -64,7 +67,7 @@ describe('promptForChange _promptForPackageChange', () => {
   });
 
   it('prompts for change type and description', async () => {
-    const questions = _getQuestionsForPackage(defaultQuestionsParams);
+    const questions = getQuestionsForPackage(defaultQuestionsParams);
     expect(questions).toEqual(expectedQuestions);
 
     const answersPromise = _promptForPackageChange(questions!, pkg);
@@ -90,7 +93,7 @@ describe('promptForChange _promptForPackageChange', () => {
 
   it('accepts custom description typed by character', async () => {
     // For this one we provide a type in options and only ask for the description
-    const questions = _getQuestionsForPackage({
+    const questions = getQuestionsForPackage({
       ...defaultQuestionsParams,
       options: { type: 'minor', message: '' },
     });
@@ -121,7 +124,7 @@ describe('promptForChange _promptForPackageChange', () => {
 
   it('accepts custom description pasted', async () => {
     // For this one we provide a type in options and only ask for the description
-    const questions = _getQuestionsForPackage({
+    const questions = getQuestionsForPackage({
       ...defaultQuestionsParams,
       options: { type: 'minor', message: '' },
     });
@@ -151,7 +154,7 @@ describe('promptForChange _promptForPackageChange', () => {
 
   it('accepts custom description pasted with newline', async () => {
     // For this one we provide a type in options and only ask for the description
-    const questions = _getQuestionsForPackage({
+    const questions = getQuestionsForPackage({
       ...defaultQuestionsParams,
       options: { type: 'minor', message: '' },
     });
@@ -177,7 +180,7 @@ describe('promptForChange _promptForPackageChange', () => {
 
   it('uses options selected with arrow keys', async () => {
     const recentMessages = ['first', 'second', 'third'];
-    const questions = _getQuestionsForPackage({ ...defaultQuestionsParams, recentMessages });
+    const questions = getQuestionsForPackage({ ...defaultQuestionsParams, recentMessages });
     expect(questions).toEqual(expectedQuestions);
 
     const answerPromise = _promptForPackageChange(questions!, pkg);
@@ -223,7 +226,7 @@ describe('promptForChange _promptForPackageChange', () => {
   });
 
   it('filters options while typing', async () => {
-    const questions = _getQuestionsForPackage({
+    const questions = getQuestionsForPackage({
       ...defaultQuestionsParams,
       recentMessages: ['foo', 'bar', 'baz'],
       options: { type: 'minor', message: '' },
@@ -254,7 +257,7 @@ describe('promptForChange _promptForPackageChange', () => {
   });
 
   it('handles pressing delete while typing', async () => {
-    const questions = _getQuestionsForPackage({
+    const questions = getQuestionsForPackage({
       ...defaultQuestionsParams,
       recentMessages: ['foo', 'bar', 'baz'],
       options: { type: 'minor', message: '' },
@@ -288,7 +291,7 @@ describe('promptForChange _promptForPackageChange', () => {
   });
 
   it('returns no answers if cancelled with ctrl-c', async () => {
-    const questions = _getQuestionsForPackage(defaultQuestionsParams);
+    const questions = getQuestionsForPackage(defaultQuestionsParams);
     expect(questions).toEqual(expectedQuestions);
 
     const answerPromise = _promptForPackageChange(questions!, pkg);

src/changefile/getQuestionsForPackage.ts

@@ -0,0 +1,107 @@
+import prompts from 'prompts';
+import semver from 'semver';
+import { ChangeType } from '../types/ChangeInfo';
+import { BeachballOptions } from '../types/BeachballOptions';
+import { DefaultPrompt } from '../types/ChangeFilePrompt';
+import { getDisallowedChangeTypes } from './getDisallowedChangeTypes';
+import { PackageGroups, PackageInfos } from '../types/PackageInfo';
+
+/**
+ * Build the list of questions to ask the user for this package.
+ * Also validates the options and returns undefined if there's an issue.
+ */
+export function getQuestionsForPackage(params: {
+  pkg: string;
+  packageInfos: PackageInfos;
+  packageGroups: PackageGroups;
+  options: Pick<BeachballOptions, 'message' | 'type'>;
+  recentMessages: string[];
+}): prompts.PromptObject[] | undefined {
+  const { pkg, packageInfos, options, recentMessages } = params;
+  const packageInfo = packageInfos[pkg];
+
+  const changeTypePrompt = getChangeTypePrompt(params);
+  if (!changeTypePrompt) {
+    return;
+  }
+
+  const defaultPrompt: DefaultPrompt = {
+    changeType: !options.type && changeTypePrompt.choices!.length > 1 ? changeTypePrompt : undefined,
+    description: !options.message ? getDescriptionPrompt(recentMessages) : undefined,
+  };
+
+  const questions =
+    packageInfo.combinedOptions.changeFilePrompt?.changePrompt?.(defaultPrompt, pkg) || Object.values(defaultPrompt);
+
+  return questions.filter((q): q is prompts.PromptObject => !!q);
+}
+
+function getChangeTypePrompt(params: {
+  pkg: string;
+  packageInfos: PackageInfos;
+  packageGroups: PackageGroups;
+  options: Pick<BeachballOptions, 'type'>;
+}): prompts.PromptObject<string> | undefined {
+  const { pkg, packageInfos, packageGroups, options } = params;
+  const packageInfo = packageInfos[pkg];
+
+  const disallowedChangeTypes = getDisallowedChangeTypes(pkg, packageInfos, packageGroups) || [];
+
+  if (options.type && disallowedChangeTypes.includes(options.type as ChangeType)) {
+    console.error(`Change type "${options.type}" is not allowed for package "${pkg}"`);
+    return;
+  }
+
+  const showPrereleaseOption = !!semver.prerelease(packageInfo.version);
+  const changeTypeChoices: prompts.Choice[] = [
+    ...(showPrereleaseOption ? [{ value: 'prerelease', title: ' Prerelease - bump prerelease version' }] : []),
+    { value: 'patch', title: ' Patch      - bug fixes; no API changes.' },
+    { value: 'minor', title: ' Minor      - small feature; backwards compatible API changes.' },
+    {
+      value: 'none',
+      title: ' None       - this change does not affect the published package in any way.',
+    },
+    { value: 'major', title: ' Major      - major feature; breaking changes.' },
+  ].filter(choice => !disallowedChangeTypes?.includes(choice.value as ChangeType));
+
+  if (!changeTypeChoices.length) {
+    console.error(`No valid change types available for package "${pkg}"`);
+    return;
+  }
+
+  return {
+    type: 'select',
+    name: 'type',
+    message: 'Change type',
+    choices: changeTypeChoices,
+  };
+}
+
+function getDescriptionPrompt(recentMessages: string[]): prompts.PromptObject<string> {
+  // Do case-insensitive filtering of recent commit messages
+  const recentMessageChoices: prompts.Choice[] = recentMessages.map(msg => ({ title: msg }));
+  const getSuggestions = (input: string) =>
+    input
+      ? recentMessageChoices.filter(({ title }) => title.toLowerCase().startsWith(input.toLowerCase()))
+      : recentMessageChoices;
+
+  return {
+    type: 'autocomplete',
+    name: 'comment',
+    message: 'Describe changes (type or choose one)',
+    choices: recentMessageChoices,
+    suggest: (input: string) => Promise.resolve(getSuggestions(input)),
+    // prompts doesn't have proper support for "freeform" input (value not in the list), and the
+    // previously implemented hack of adding the input to the returned list from `suggest`
+    // no longer works. So this new hack adds the current input as the fallback.
+    // https://github.com/terkelg/prompts/issues/131
+    onState: function (this: any, state: any) {
+      // If there are no suggestions, update the value to match the input, and unset the fallback
+      // (this.suggestions may be out of date if the user pasted text ending with a newline, so re-calculate)
+      if (!getSuggestions(this.input).length) {
+        this.value = this.input;
+        this.fallback = '';
+      }
+    },
+  };
+}

src/changefile/promptForChange.ts

@@ -1,11 +1,9 @@
 import prompts from 'prompts';
-import { prerelease } from 'semver';
 import { ChangeFileInfo, ChangeType } from '../types/ChangeInfo';
 import { BeachballOptions } from '../types/BeachballOptions';
 import { isValidChangeType } from '../validation/isValidChangeType';
-import { DefaultPrompt } from '../types/ChangeFilePrompt';
-import { getDisallowedChangeTypes } from './getDisallowedChangeTypes';
 import { PackageGroups, PackageInfos } from '../types/PackageInfo';
+import { getQuestionsForPackage } from './getQuestionsForPackage';
 
 type ChangePromptResponse = { type?: ChangeType; comment?: string };
 
@@ -29,7 +27,7 @@ export async function promptForChange(params: {
   // Get the questions for each package first, in case one package has a validation issue
   const packageQuestions: { [pkg: string]: prompts.PromptObject[] } = {};
   for (const pkg of changedPackages) {
-    const questions = _getQuestionsForPackage({ pkg, ...params });
+    const questions = getQuestionsForPackage({ pkg, ...params });
     if (!questions) {
       return; // validation issue
     }
@@ -54,91 +52,6 @@ export async function promptForChange(params: {
   return packageChangeInfo;
 }
 
-/**
- * Build the list of questions to ask the user for this package.
- * @internal exported for testing
- */
-export function _getQuestionsForPackage(params: {
-  pkg: string;
-  packageInfos: PackageInfos;
-  packageGroups: PackageGroups;
-  options: Pick<BeachballOptions, 'message' | 'type'>;
-  recentMessages: string[];
-}): prompts.PromptObject[] | undefined {
-  const { pkg, packageInfos, packageGroups, options, recentMessages } = params;
-
-  const disallowedChangeTypes = getDisallowedChangeTypes(pkg, packageInfos, packageGroups);
-
-  if (options.type && disallowedChangeTypes?.includes(options.type as ChangeType)) {
-    console.error(`Change type "${options.type}" is not allowed for package "${pkg}"`);
-    return;
-  }
-
-  const packageInfo = packageInfos[pkg];
-  const showPrereleaseOption = !!prerelease(packageInfo.version);
-  const changeTypeChoices: prompts.Choice[] = [
-    ...(showPrereleaseOption ? [{ value: 'prerelease', title: ' Prerelease - bump prerelease version' }] : []),
-    { value: 'patch', title: ' Patch      - bug fixes; no API changes.' },
-    { value: 'minor', title: ' Minor      - small feature; backwards compatible API changes.' },
-    {
-      value: 'none',
-      title: ' None       - this change does not affect the published package in any way.',
-    },
-    { value: 'major', title: ' Major      - major feature; breaking changes.' },
-  ].filter(choice => !disallowedChangeTypes?.includes(choice.value as ChangeType));
-
-  if (!changeTypeChoices.length) {
-    console.error(`No valid change types available for package "${pkg}"`);
-    return;
-  }
-
-  const changeTypePrompt: prompts.PromptObject<string> = {
-    type: 'select',
-    name: 'type',
-    message: 'Change type',
-    choices: changeTypeChoices,
-  };
-
-  // Do case-insensitive filtering of recent commit messages
-  const recentMessageChoices: prompts.Choice[] = recentMessages.map(msg => ({ title: msg }));
-  const getSuggestions = (input: string) =>
-    input
-      ? recentMessageChoices.filter(({ title }) => title.toLowerCase().startsWith(input.toLowerCase()))
-      : recentMessageChoices;
-
-  const descriptionPrompt: prompts.PromptObject<string> = {
-    type: 'autocomplete',
-    name: 'comment',
-    message: 'Describe changes (type or choose one)',
-    choices: recentMessageChoices,
-    suggest: (input: string) => Promise.resolve(getSuggestions(input)),
-    // prompts doesn't have proper support for "freeform" input (value not in the list), and the
-    // previously implemented hack of adding the input to the returned list from `suggest`
-    // no longer works. So this new hack adds the current input as the fallback.
-    // https://github.com/terkelg/prompts/issues/131
-    onState: function (this: any, state: any) {
-      // If there are no suggestions, update the value to match the input, and unset the fallback
-      // (this.suggestions may be out of date if the user pasted text ending with a newline, so re-calculate)
-      if (!getSuggestions(this.input).length) {
-        this.value = this.input;
-        this.fallback = '';
-      }
-    },
-  };
-
-  const showChangeTypePrompt = !options.type && changeTypePrompt.choices!.length > 1;
-
-  const defaultPrompt: DefaultPrompt = {
-    changeType: showChangeTypePrompt ? changeTypePrompt : undefined,
-    description: !options.message ? descriptionPrompt : undefined,
-  };
-  const defaultPrompts = [defaultPrompt.changeType, defaultPrompt.description];
-
-  return (packageInfo.combinedOptions.changeFilePrompt?.changePrompt?.(defaultPrompt, pkg) || defaultPrompts).filter(
-    (q): q is prompts.PromptObject => !!q
-  );
-}
-
 /**
  * Do the actual prompting.
  * @internal exported for testing