cli/new: more strict input of globals and stop merging them into params

Signed-off-by: Patrik Oldsberg <poldsberg@gmail.com>

packages/cli/src/lib/new/createNewPackage.ts

@@ -21,17 +21,11 @@ import {
   selectTemplateInteractively,
 } from './preparation';
 import { executePortableTemplate } from './execution';
-import { PortableTemplateParams } from './types';
+import { PortableTemplateGlobals, PortableTemplateParams } from './types';
 
 export type CreateNewPackageOptions = {
   preselectedTemplateId?: string;
-  globals: {
-    private?: boolean;
-    npmRegistry?: string;
-    scope?: string;
-    license?: string;
-    baseVersion?: string;
-  };
+  globals: PortableTemplateGlobals;
   prefilledParams: PortableTemplateParams;
 };
 
@@ -47,7 +41,6 @@ export async function createNewPackage(options: CreateNewPackageOptions) {
   const template = await loadPortableTemplate(selectedTemplate);
 
   const params = await collectPortableTemplateParams({
-    config,
     template,
     prefilledParams: options.prefilledParams,
   });

packages/cli/src/lib/new/preparation/collectPortableTemplateParams.test.ts

@@ -40,30 +40,8 @@ describe('collectTemplateParams', () => {
   it('should return default values if not provided', async () => {
     await expect(collectPortableTemplateParams(baseOptions)).resolves.toEqual({
       pluginId: 'test',
-      private: true,
-      baseVersion: '0.1.0',
       owner: '',
-      license: 'Apache-2.0',
       targetPath: '/example',
-      scope: '',
-    });
-  });
-
-  it('should include all non-standard global and prompt values', async () => {
-    await expect(
-      collectPortableTemplateParams({
-        ...baseOptions,
-        config: { ...baseOptions.config, globals: { foo: 'bar' } },
-      }),
-    ).resolves.toEqual({
-      pluginId: 'test',
-      private: true,
-      baseVersion: '0.1.0',
-      owner: '',
-      license: 'Apache-2.0',
-      targetPath: '/example',
-      scope: '',
-      foo: 'bar',
     });
   });
 
@@ -77,12 +55,7 @@ describe('collectTemplateParams', () => {
       }),
     ).resolves.toEqual({
       pluginId: 'other',
-      private: true,
-      baseVersion: '0.1.0',
-      owner: '',
-      license: 'Apache-2.0',
       targetPath: '/example',
-      scope: '',
     });
   });
 });

packages/cli/src/lib/new/preparation/collectPortableTemplateParams.ts

@@ -18,7 +18,6 @@ import inquirer, { DistinctQuestion } from 'inquirer';
 import { getCodeownersFilePath, parseOwnerIds } from '../../codeowners';
 import { paths } from '../../paths';
 import {
-  PortableTemplateConfig,
   PortableTemplateParams,
   PortableTemplatePrompt,
   PortableTemplateRole,
@@ -26,7 +25,6 @@ import {
 import { PortableTemplate } from '../types';
 
 type CollectTemplateParamsOptions = {
-  config: PortableTemplateConfig;
   template: PortableTemplate;
   prefilledParams: PortableTemplateParams;
 };
@@ -34,7 +32,7 @@ type CollectTemplateParamsOptions = {
 export async function collectPortableTemplateParams(
   options: CollectTemplateParamsOptions,
 ): Promise<PortableTemplateParams> {
-  const { config, template, prefilledParams } = options;
+  const { template, prefilledParams } = options;
 
   const codeOwnersFilePath = await getCodeownersFilePath(paths.targetRoot);
 
@@ -62,7 +60,6 @@ export async function collectPortableTemplateParams(
   );
 
   return {
-    ...config.globals,
     ...prefilledAnswers,
     ...promptAnswers,
     targetPath: template.targetPath,

packages/cli/src/lib/new/preparation/loadPortableTemplateConfig.test.ts

@@ -19,22 +19,6 @@ import { defaultTemplates } from '../defaultTemplates';
 import { createMockDirectory } from '@backstage/backend-test-utils';
 
 describe('loadPortableTemplateConfig', () => {
-  const mockPkgJson = {
-    backstage: {
-      new: {
-        templates: [
-          { id: 'template1', target: 'path/to/template1' },
-          { id: 'template2', target: 'path/to/template2' },
-        ],
-        globals: {
-          key1: 'value1',
-          key2: 2,
-          key3: true,
-        },
-      },
-    },
-  };
-
   const mockDir = createMockDirectory();
 
   afterEach(() => {
@@ -51,9 +35,8 @@ describe('loadPortableTemplateConfig', () => {
               { id: 'template2', target: 'path/to/template2' },
             ],
             globals: {
-              key1: 'value1',
-              key2: 2,
-              key3: true,
+              license: 'MIT',
+              private: true,
             },
           },
         },
@@ -66,11 +49,14 @@ describe('loadPortableTemplateConfig', () => {
       }),
     ).resolves.toEqual({
       isUsingDefaultTemplates: false,
-      templatePointers: mockPkgJson.backstage.new.templates,
+      templatePointers: [
+        { id: 'template1', target: 'path/to/template1' },
+        { id: 'template2', target: 'path/to/template2' },
+      ],
       globals: {
-        key1: 'value1',
-        key2: 2,
-        key3: true,
+        license: 'MIT',
+        private: true,
+        baseVersion: '0.1.0',
       },
     });
 
@@ -78,18 +64,20 @@ describe('loadPortableTemplateConfig', () => {
       loadPortableTemplateConfig({
         packagePath: mockDir.resolve('package.json'),
         globalOverrides: {
-          key2: 'override',
-          key4: 'override2',
+          license: 'nope',
+          private: false,
         },
       }),
     ).resolves.toEqual({
       isUsingDefaultTemplates: false,
-      templatePointers: mockPkgJson.backstage.new.templates,
+      templatePointers: [
+        { id: 'template1', target: 'path/to/template1' },
+        { id: 'template2', target: 'path/to/template2' },
+      ],
       globals: {
-        key1: 'value1',
-        key2: 'override',
-        key3: true,
-        key4: 'override2',
+        license: 'nope',
+        private: false,
+        baseVersion: '0.1.0',
       },
     });
   });
@@ -100,9 +88,8 @@ describe('loadPortableTemplateConfig', () => {
         backstage: {
           new: {
             globals: {
-              key1: 'value1',
-              key2: 2,
-              key3: true,
+              license: 'MIT',
+              private: true,
             },
           },
         },
@@ -117,9 +104,9 @@ describe('loadPortableTemplateConfig', () => {
       isUsingDefaultTemplates: true,
       templatePointers: defaultTemplates,
       globals: {
-        key1: 'value1',
-        key2: 2,
-        key3: true,
+        license: 'MIT',
+        private: true,
+        baseVersion: '0.1.0',
       },
     });
   });
@@ -156,21 +143,27 @@ describe('loadPortableTemplateConfig', () => {
     ).resolves.toEqual({
       isUsingDefaultTemplates: true,
       templatePointers: defaultTemplates,
-      globals: {},
+      globals: {
+        license: 'Apache-2.0',
+        baseVersion: '0.1.0',
+        private: true,
+      },
     });
 
     await expect(
       loadPortableTemplateConfig({
         packagePath: mockDir.resolve('package.json'),
         globalOverrides: {
-          key: 'override',
+          license: 'nope',
         },
       }),
     ).resolves.toEqual({
       isUsingDefaultTemplates: true,
       templatePointers: defaultTemplates,
       globals: {
-        key: 'override',
+        license: 'nope',
+        baseVersion: '0.1.0',
+        private: true,
       },
     });
   });

packages/cli/src/lib/new/preparation/loadPortableTemplateConfig.ts

@@ -17,7 +17,7 @@
 import fs from 'fs-extra';
 import { paths } from '../../paths';
 import { defaultTemplates } from '../defaultTemplates';
-import { PortableTemplateConfig, PortableTemplateParams } from '../types';
+import { PortableTemplateConfig, PortableTemplateGlobals } from '../types';
 import { z } from 'zod';
 import { fromZodError } from 'zod-validation-error';
 import { ForwardedError } from '@backstage/errors';
@@ -26,6 +26,7 @@ const defaultGlobals = {
   license: 'Apache-2.0',
   baseVersion: '0.1.0',
   private: true,
+  scope: undefined,
 };
 
 const pkgJsonWithNewConfigSchema = z.object({
@@ -44,7 +45,12 @@ const pkgJsonWithNewConfigSchema = z.object({
             )
             .optional(),
           globals: z
-            .record(z.union([z.string(), z.number(), z.boolean()]))
+            .object({
+              license: z.string().optional(),
+              baseVersion: z.string().optional(),
+              private: z.boolean().optional(),
+              scope: z.string().optional(),
+            })
             .optional(),
         })
         .strict()
@@ -55,7 +61,7 @@ const pkgJsonWithNewConfigSchema = z.object({
 
 type LoadConfigOptions = {
   packagePath?: string;
-  globalOverrides?: PortableTemplateParams;
+  globalOverrides?: PortableTemplateGlobals;
 };
 
 export async function loadPortableTemplateConfig(

packages/cli/src/lib/new/types.ts

@@ -28,9 +28,7 @@ export type PortableTemplateConfig = {
   /**
    * Templating globals that should apply to all templates.
    */
-  globals: {
-    [KName in string]?: number | string | boolean;
-  };
+  globals: PortableTemplateGlobals;
 };
 
 export type PortableTemplatePointer = {
@@ -73,3 +71,10 @@ export type PortableTemplate = {
 export type PortableTemplateParams = {
   [KName in string]?: string | number | boolean;
 };
+
+export type PortableTemplateGlobals = {
+  license?: string;
+  baseVersion?: string;
+  private?: boolean;
+  scope?: string;
+};