From 08bf562c98592faf0de6149eab05f5eaf76e6f51 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Fri, 7 Feb 2025 17:08:27 +0100 Subject: [PATCH] cli/new: more strict input separation and types Signed-off-by: Patrik Oldsberg --- packages/cli/src/lib/new/createNewPackage.ts | 7 +-- .../new/execution/executePortableTemplate.ts | 6 +-- ...s => collectPortableTemplateInput.test.ts} | 37 ++++++++++---- ...ams.ts => collectPortableTemplateInput.ts} | 48 ++++++++++++++----- packages/cli/src/lib/new/preparation/index.ts | 2 +- packages/cli/src/lib/new/types.ts | 31 ++++++++++++ 6 files changed, 103 insertions(+), 28 deletions(-) rename packages/cli/src/lib/new/preparation/{collectPortableTemplateParams.test.ts => collectPortableTemplateInput.test.ts} (67%) rename packages/cli/src/lib/new/preparation/{collectPortableTemplateParams.ts => collectPortableTemplateInput.ts} (81%) diff --git a/packages/cli/src/lib/new/createNewPackage.ts b/packages/cli/src/lib/new/createNewPackage.ts index 2db3cc5f36..3fc3317eed 100644 --- a/packages/cli/src/lib/new/createNewPackage.ts +++ b/packages/cli/src/lib/new/createNewPackage.ts @@ -15,7 +15,7 @@ */ import { - collectPortableTemplateParams, + collectPortableTemplateInput, loadPortableTemplate, loadPortableTemplateConfig, selectTemplateInteractively, @@ -40,7 +40,8 @@ export async function createNewPackage(options: CreateNewPackageOptions) { ); const template = await loadPortableTemplate(selectedTemplate); - const params = await collectPortableTemplateParams({ + const input = await collectPortableTemplateInput({ + config, template, prefilledParams: options.prefilledParams, }); @@ -48,6 +49,6 @@ export async function createNewPackage(options: CreateNewPackageOptions) { await executePortableTemplate({ config, template, - params, + input, }); } diff --git a/packages/cli/src/lib/new/execution/executePortableTemplate.ts b/packages/cli/src/lib/new/execution/executePortableTemplate.ts index 3d641efa97..586515a0fe 100644 --- a/packages/cli/src/lib/new/execution/executePortableTemplate.ts +++ b/packages/cli/src/lib/new/execution/executePortableTemplate.ts @@ -27,19 +27,19 @@ import { createDirName, resolvePackageName } from './utils'; import { runAdditionalActions } from './additionalActions'; import { executePluginPackageTemplate } from './executePluginPackageTemplate'; import { TemporaryDirectoryManager } from './TemporaryDirectoryManager'; -import { PortableTemplateConfig, PortableTemplateParams } from '../types'; +import { PortableTemplateConfig, PortableTemplateInput } from '../types'; import { PortableTemplate } from '../types'; type ExecuteNewTemplateOptions = { config: PortableTemplateConfig; template: PortableTemplate; - params: PortableTemplateParams; + input: PortableTemplateInput; }; export async function executePortableTemplate( options: ExecuteNewTemplateOptions, ) { - const { template, params } = options; + const { config, template, input } = options; const tmpDirManager = TemporaryDirectoryManager.create(); diff --git a/packages/cli/src/lib/new/preparation/collectPortableTemplateParams.test.ts b/packages/cli/src/lib/new/preparation/collectPortableTemplateInput.test.ts similarity index 67% rename from packages/cli/src/lib/new/preparation/collectPortableTemplateParams.test.ts rename to packages/cli/src/lib/new/preparation/collectPortableTemplateInput.test.ts index f7d78c53dc..51b778e3e1 100644 --- a/packages/cli/src/lib/new/preparation/collectPortableTemplateParams.test.ts +++ b/packages/cli/src/lib/new/preparation/collectPortableTemplateInput.test.ts @@ -16,7 +16,7 @@ import inquirer from 'inquirer'; import { PortableTemplateConfig } from '../types'; -import { collectPortableTemplateParams } from './collectPortableTemplateParams'; +import { collectPortableTemplateInput } from './collectPortableTemplateInput'; describe('collectTemplateParams', () => { const baseOptions = { @@ -33,15 +33,24 @@ describe('collectTemplateParams', () => { }, prefilledParams: { pluginId: 'test', - owner: '', + owner: 'me', }, }; it('should return default values if not provided', async () => { - await expect(collectPortableTemplateParams(baseOptions)).resolves.toEqual({ - pluginId: 'test', - owner: '', - targetPath: '/example', + await expect(collectPortableTemplateInput(baseOptions)).resolves.toEqual({ + roleParams: { + role: 'frontend-plugin', + pluginId: 'test', + }, + builtInParams: { + owner: 'me', + }, + params: { + pluginId: 'test', + owner: 'me', + }, + globals: {}, }); }); @@ -49,13 +58,23 @@ describe('collectTemplateParams', () => { jest.spyOn(inquirer, 'prompt').mockResolvedValueOnce({ pluginId: 'other' }); await expect( - collectPortableTemplateParams({ + collectPortableTemplateInput({ ...baseOptions, prefilledParams: {}, }), ).resolves.toEqual({ - pluginId: 'other', - targetPath: '/example', + roleParams: { + role: 'frontend-plugin', + pluginId: 'other', + }, + builtInParams: { + owner: undefined, + }, + params: { + pluginId: 'other', + owner: undefined, + }, + globals: {}, }); }); }); diff --git a/packages/cli/src/lib/new/preparation/collectPortableTemplateParams.ts b/packages/cli/src/lib/new/preparation/collectPortableTemplateInput.ts similarity index 81% rename from packages/cli/src/lib/new/preparation/collectPortableTemplateParams.ts rename to packages/cli/src/lib/new/preparation/collectPortableTemplateInput.ts index 3602052244..4c0c2d04dc 100644 --- a/packages/cli/src/lib/new/preparation/collectPortableTemplateParams.ts +++ b/packages/cli/src/lib/new/preparation/collectPortableTemplateInput.ts @@ -18,32 +18,38 @@ import inquirer, { DistinctQuestion } from 'inquirer'; import { getCodeownersFilePath, parseOwnerIds } from '../../codeowners'; import { paths } from '../../paths'; import { + PortableTemplateConfig, + PortableTemplateInput, + PortableTemplateInputBuiltInParams, + PortableTemplateInputRoleParams, PortableTemplateParams, PortableTemplatePrompt, PortableTemplateRole, } from '../types'; import { PortableTemplate } from '../types'; +const RESERVED_PROMPT_NAMES = ['name', 'pluginId', 'moduleId', 'owner']; + type CollectTemplateParamsOptions = { + config: PortableTemplateConfig; template: PortableTemplate; prefilledParams: PortableTemplateParams; }; -export async function collectPortableTemplateParams( +export async function collectPortableTemplateInput( options: CollectTemplateParamsOptions, -): Promise { - const { template, prefilledParams } = options; +): Promise { + const { config, template, prefilledParams } = options; const codeOwnersFilePath = await getCodeownersFilePath(paths.targetRoot); - const prompts = getPromptsForRole(template.role); + const rolePrompts = getPromptsForRole(template.role); - if (codeOwnersFilePath) { - prompts.push(ownerPrompt()); - } - if (template.prompts) { - prompts.push(...template.prompts.map(customPrompt)); - } + const buildInPrompts = codeOwnersFilePath ? [ownerPrompt()] : []; + + const templatePrompts = template.prompts?.map(customPrompt) ?? []; + + const prompts = [...rolePrompts, ...buildInPrompts, ...templatePrompts]; const needsAnswer = []; const prefilledAnswers = {} as PortableTemplateParams; @@ -59,10 +65,23 @@ export async function collectPortableTemplateParams( needsAnswer, ); - return { + const answers = { ...prefilledAnswers, ...promptAnswers, - targetPath: template.targetPath, + }; + + return { + roleParams: { + role: template.role, + name: answers.name, + pluginId: answers.pluginId, + moduleId: answers.moduleId, + } as PortableTemplateInputRoleParams, + builtInParams: { + owner: answers.owner, + } as PortableTemplateInputBuiltInParams, + params: answers, + globals: config.globals, }; } @@ -157,6 +176,11 @@ export function ownerPrompt(): DistinctQuestion { } export function customPrompt(prompt: PortableTemplatePrompt): DistinctQuestion { + if (RESERVED_PROMPT_NAMES.includes(prompt.id)) { + throw new Error( + `Prompt ID '${prompt.id}' is reserved and cannot be used in a template`, + ); + } return { type: 'input', name: prompt.id, diff --git a/packages/cli/src/lib/new/preparation/index.ts b/packages/cli/src/lib/new/preparation/index.ts index e4630686bf..8b2b253997 100644 --- a/packages/cli/src/lib/new/preparation/index.ts +++ b/packages/cli/src/lib/new/preparation/index.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -export { collectPortableTemplateParams } from './collectPortableTemplateParams'; +export { collectPortableTemplateInput } from './collectPortableTemplateInput'; export { loadPortableTemplateConfig } from './loadPortableTemplateConfig'; export { selectTemplateInteractively } from './selectTemplateInteractively'; export { loadPortableTemplate } from './loadPortableTemplate'; diff --git a/packages/cli/src/lib/new/types.ts b/packages/cli/src/lib/new/types.ts index 35e522242d..4a70e03d7f 100644 --- a/packages/cli/src/lib/new/types.ts +++ b/packages/cli/src/lib/new/types.ts @@ -78,3 +78,34 @@ export type PortableTemplateGlobals = { private?: boolean; scope?: string; }; + +export type PortableTemplateInputRoleParams = + | { + role: 'web-library' | 'node-library' | 'common-library'; + name: string; + } + | { + role: + | 'plugin-web-library' + | 'plugin-node-library' + | 'plugin-common-library' + | 'frontend-plugin' + | 'backend-plugin'; + pluginId: string; + } + | { + role: 'frontend-plugin-module' | 'backend-plugin-module'; + pluginId: string; + moduleId: string; + }; + +export type PortableTemplateInputBuiltInParams = { + owner?: string; +}; + +export type PortableTemplateInput = { + roleParams: PortableTemplateInputRoleParams; + builtInParams: PortableTemplateInputBuiltInParams; + params: PortableTemplateParams; + globals: PortableTemplateParams; +};