Merge pull request #373 from Financial-Times/fix-zod-create

Fix zod validation issues with the create package

core/cli/src/messages.ts

@@ -102,7 +102,8 @@ ${invalidOptions
   .map(([plugin, error]) => fromZodError(error, { prefix: `- ${s.plugin(plugin)} has the issue(s)` }).message)
   .join('\n')}
 
-Please update the options so that they are the expected types. You can refer to the README for the plugin for examples and descriptions of the options used.`
+Please update the options so that they are the expected types. You can refer to the README for the plugin for examples and descriptions of the options used.
+`
 
 export const formatUnusedOptions = (
   unusedOptions: string[],

core/create/src/index.ts

@@ -3,13 +3,14 @@ import { rootLogger as winstonLogger, styles } from '@dotcom-tool-kit/logger'
 import type { RCFile } from '@dotcom-tool-kit/types'
 import loadPackageJson from '@financial-times/package-json'
 import { exec as _exec } from 'child_process'
-import type { ValidConfig } from 'dotcom-tool-kit/lib/config'
+import { loadConfig } from 'dotcom-tool-kit/lib/config'
 import { promises as fs } from 'fs'
 import * as yaml from 'js-yaml'
+import Logger from 'komatsu'
 import pacote from 'pacote'
 import path from 'path'
 import { promisify } from 'util'
-import { hasToolKitConflicts, Logger, runTasksWithLogger } from './logger'
+import { catchToolKitErrorsInLogger, hasToolKitConflicts } from './logger'
 import makefileHint from './makefile'
 import confirmationPrompt from './prompts/confirmation'
 import conflictsPrompt, { installHooks } from './prompts/conflicts'
@@ -43,7 +44,7 @@ async function executeMigration(
   deleteConfig: boolean,
   addEslintConfig: boolean,
   configFile: string
-): Promise<ValidConfig> {
+): Promise<void> {
   for (const pkg of packagesToInstall) {
     const { version } = await pacote.manifest(pkg)
     packageJson.requireDependency({
@@ -78,11 +79,7 @@ async function executeMigration(
     ? logger.logPromise(fs.unlink(circleConfigPath), 'removing old CircleCI config')
     : Promise.resolve()
 
-  const initialTasks = Promise.all([configPromise, eslintConfigPromise, unlinkPromise]).then(
-    () => winstonLogger
-  )
-
-  return runTasksWithLogger(logger, initialTasks, installHooks, 'installing Tool Kit hooks', true)
+  await Promise.all([configPromise, eslintConfigPromise, unlinkPromise])
 }
 
 async function main() {
@@ -126,38 +123,36 @@ async function main() {
   if (!confirm) {
     return
   }
-  let config: ValidConfig | undefined
+  // Carry out the proposed changes: install + uninstall packages, add config
+  // files, etc.
+  await executeMigration(deleteConfig, addEslintConfig, configFile)
+  const config = await loadConfig(winstonLogger, { validate: false })
+  // Give the user a chance to set any configurable options for the plugins
+  // they've installed.
+  const cancelled = await optionsPrompt({ logger, config, toolKitConfig, configPath })
+  if (cancelled) {
+    return
+  }
   try {
-    // Carry out the proposed changes: install + uninstall packages, run
-    // --install logic etc.
-    config = await executeMigration(deleteConfig, addEslintConfig, configFile)
+    await catchToolKitErrorsInLogger(logger, installHooks(winstonLogger), 'installing Tool Kit hooks', true)
   } catch (error) {
     if (hasToolKitConflicts(error)) {
       // Additional questions asked if we have any task conflicts, letting the
       // user to specify the order they want tasks to run in.
-      config = await conflictsPrompt({
+      const conflictsCancelled = await conflictsPrompt({
         error: error as ToolkitErrorModule.ToolKitConflictError,
         logger,
         toolKitConfig,
         configPath
       })
+      if (conflictsCancelled) {
+        return
+      }
     } else {
       throw error
     }
   }
 
-  // Only run final prompts if execution was successful (this also means these
-  // are skipped if the user cancels out of the conflict resolution prompt.)
-  if (!config) {
-    return
-  }
-  // Give the user a chance to set any configurable options for the plugins
-  // they've installed.
-  const cancelled = await optionsPrompt({ logger, config, toolKitConfig, configPath })
-  if (cancelled) {
-    return
-  }
-
   if (originalCircleConfig?.includes('triggers')) {
     await scheduledPipelinePrompt()
   }

core/create/src/logger.ts

@@ -1,59 +1,11 @@
 import * as ToolkitErrorModule from '@dotcom-tool-kit/error'
 import importCwd from 'import-cwd'
-import type { Spinner } from 'komatsu'
-import Komatsu from 'komatsu'
+import Logger from 'komatsu'
 
-export type LoggerError = Error & {
+export type LoggerError = (Error | ToolkitErrorModule.ToolKitError) & {
   logged?: boolean
 }
 
-export type labels = {
-  waiting: string
-  pending: string
-  done: string
-  fail: string
-}
-
-// TODO backport this to Komatsu mainline?
-export class Logger extends Komatsu {
-  stop(): void {
-    if (
-      !Array.from(this.spinners.values()).some(
-        (spinner: Spinner | { status: 'not-started' }) => spinner.status === 'not-started'
-      )
-    )
-      super.stop()
-  }
-
-  renderSymbol(spinner: Spinner | { status: 'not-started' }): string {
-    if (spinner.status === 'not-started') {
-      return '-'
-    }
-
-    return super.renderSymbol(spinner)
-  }
-
-  async logPromiseWait<T>(wait: Promise<T>, labels: labels): Promise<{ interim: T; id: string }> {
-    const id = Math.floor(parseInt(`zzzzzz`, 36) * Math.random())
-      .toString(36)
-      .padStart(6, '0')
-
-    this.log(id, { message: labels.waiting, status: 'not-started' })
-
-    let interim
-    try {
-      interim = await wait
-    } catch (error) {
-      const loggerError = error as LoggerError
-      // should have been logged by logPromise
-      loggerError.logged = true
-      throw error
-    }
-
-    return { interim, id }
-  }
-}
-
 export function hasToolKitConflicts(error: unknown): boolean {
   try {
     // we need to import hasToolkitConflicts from the app itself instead of npx
@@ -69,26 +21,29 @@ export function hasToolKitConflicts(error: unknown): boolean {
   }
 }
 
-export async function runTasksWithLogger<T, U>(
+// helper function to include ToolKitError details in logs when available and,
+// optionally, don't print errors when we get Tool Kit conflicts if we're
+// expecting them
+export async function catchToolKitErrorsInLogger<T>(
   logger: Logger,
-  wait: Promise<T>,
-  run: (interim: T) => Promise<U>,
+  run: Promise<T>,
   label: string,
   allowConflicts: boolean
-): Promise<U> {
-  const labels: labels = {
-    waiting: `not ${label} yet`,
+): Promise<T> {
+  const labels = {
     pending: label,
     done: `finished ${label}`,
     fail: `error with ${label}`
   }
 
-  const { interim, id } = await logger.logPromiseWait(wait, labels)
+  const id = Math.floor(parseInt(`zzzzzz`, 36) * Math.random())
+    .toString(36)
+    .padStart(6, '0')
 
   try {
     logger.log(id, { message: labels.pending })
 
-    const result = await run(interim)
+    const result = await run
     logger.log(id, { status: 'done', message: labels.done })
     return result
   } catch (error) {
@@ -98,9 +53,13 @@ export async function runTasksWithLogger<T, U>(
     if (allowConflicts && hasToolKitConflicts(error)) {
       logger.log(id, { status: 'done', message: 'finished installing hooks, but found conflicts' })
     } else {
+      let message = labels.fail
+      if ('details' in loggerError) {
+        message += ' –\n' + loggerError.details
+      }
       logger.log(id, {
         status: 'fail',
-        message: labels.fail,
+        message,
         error: loggerError.logged ? undefined : loggerError
       })
     }

core/create/src/prompts/conflicts.ts

@@ -7,9 +7,10 @@ import type installHooksType from 'dotcom-tool-kit/lib/install'
 import { promises as fs } from 'fs'
 import importCwd from 'import-cwd'
 import * as yaml from 'js-yaml'
+import type Logger from 'komatsu'
 import ordinal from 'ordinal'
 import prompt from 'prompts'
-import { Logger, runTasksWithLogger } from '../logger'
+import { catchToolKitErrorsInLogger } from '../logger'
 
 interface ConflictsParams {
   error: ToolkitErrorModule.ToolKitConflictError
@@ -35,12 +36,7 @@ export function installHooks(logger: typeof winstonLogger): Promise<ValidConfig>
   }).default(logger)
 }
 
-export default async ({
-  error,
-  logger,
-  toolKitConfig,
-  configPath
-}: ConflictsParams): Promise<ValidConfig | undefined> => {
+export default async ({ error, logger, toolKitConfig, configPath }: ConflictsParams): Promise<boolean> => {
   const orderedHooks: { [hook: string]: string[] } = {}
 
   for (const conflict of error.conflicts) {
@@ -64,7 +60,7 @@ Please select the ${ordinal(i)} package to run.`,
       })
 
       if (nextIdx === undefined) {
-        return
+        return true
       } else if (nextIdx === null) {
         break
       } else {
@@ -87,13 +83,21 @@ sound alright?`
   })
 
   if (confirm) {
-    const configPromise = logger.logPromise(
-      fs.writeFile(configPath, configFile).then(() => winstonLogger),
+    await logger.logPromise(
+      fs.writeFile(configPath, configFile),
       `recreating ${styles.filepath('.toolkitrc.yml')}`
     )
     // Clear config cache now that config has been updated
     clearConfigCache()
 
-    return runTasksWithLogger(logger, configPromise, installHooks, 'installing Tool Kit hooks again', false)
+    await catchToolKitErrorsInLogger(
+      logger,
+      installHooks(winstonLogger),
+      'installing Tool Kit hooks again',
+      false
+    )
+
+    return false
   }
+  return true
 }