chore: improve convert-template error messages

packages/visual-editor/src/cli/commands/internal/convertTemplatesToSectionLibrary.test.ts

@@ -2,6 +2,7 @@ import path from "node:path";
 import fs from "fs-extra";
 import { describe, expect, it } from "vitest";
 import { createTempRoot } from "../../../internal/sectionLibraryValidation/testUtils.ts";
+import { purposes, verticals } from "../../../types/sectionLibrary.ts";
 import { convertTemplatesToSectionLibrary } from "./convertTemplatesToSectionLibrary.ts";
 
 describe("convertTemplatesToSectionLibrary", () => {
@@ -147,3 +148,187 @@ describe("convertTemplatesToSectionLibrary", () => {
     ).toBe(false);
   });
 });
+
+const createConversionFixture = () => {
+  const rootDirectory = createTempRoot("convert-template-errors-");
+  const templateDirectory = path.join(rootDirectory, "src/registry/example");
+  fs.outputJsonSync(path.join(templateDirectory, "template.json"), {
+    displayName: "Example",
+  });
+  fs.outputJsonSync(path.join(templateDirectory, "defaultLayout.json"), {
+    root: {},
+    zones: {},
+    content: [],
+  });
+  fs.outputFileSync(
+    path.join(templateDirectory, "components/Hero.tsx"),
+    'export const Hero: YextComponentConfig = { label: "Hero", render: () => null };'
+  );
+  const libraryDirectory = path.join(rootDirectory, "src/library");
+  fs.outputFileSync(
+    path.join(libraryDirectory, "shared/componentRegistry.ts"),
+    ""
+  );
+  for (const id of ["Directory", "Locator"]) {
+    fs.outputFileSync(path.join(libraryDirectory, `sections/${id}.tsx`), "");
+    fs.outputJsonSync(
+      path.join(libraryDirectory, `layouts/${id}/metadata.json`),
+      {
+        id,
+        pageSetType: id.toUpperCase(),
+      }
+    );
+    fs.outputJsonSync(
+      path.join(libraryDirectory, `layouts/${id}/defaultLayout.json`),
+      {
+        root: {},
+        zones: {},
+        content: [],
+      }
+    );
+  }
+  const convert = () =>
+    convertTemplatesToSectionLibrary({
+      apply: true,
+      deleteSource: false,
+      targetDirectory: rootDirectory,
+      write: () => undefined,
+    });
+  return { rootDirectory, templateDirectory, libraryDirectory, convert };
+};
+
+describe("conversion error diagnostics", () => {
+  it.each([
+    { metadata: null, messages: ["metadata must be a JSON object"] },
+    {
+      metadata: {},
+      messages: [
+        "pageSetType must be ENTITY, DIRECTORY, or LOCATOR; received undefined",
+        "id must be a string; received undefined",
+      ],
+    },
+    {
+      metadata: { id: "bad id", pageSetType: "OTHER" },
+      messages: [
+        'received "OTHER"',
+        'id must match directory name "entity"; received "bad id"',
+        "id must start with a letter or number",
+      ],
+    },
+  ])("explains invalid base metadata: $metadata", ({ metadata, messages }) => {
+    const { libraryDirectory, convert } = createConversionFixture();
+    const metadataPath = path.join(
+      libraryDirectory,
+      "layouts/entity/metadata.json"
+    );
+    fs.outputJsonSync(metadataPath, metadata);
+    for (const message of [metadataPath, ...messages]) {
+      expect(convert).toThrow(message);
+    }
+  });
+
+  it.each(["Legacy", "Base"])(
+    "explains invalid %s layout fields",
+    (description) => {
+      const { templateDirectory, libraryDirectory, convert } =
+        createConversionFixture();
+      const layoutPath = path.join(
+        description === "Legacy"
+          ? templateDirectory
+          : path.join(libraryDirectory, "layouts/Directory"),
+        "defaultLayout.json"
+      );
+      fs.outputJsonSync(layoutPath, { root: null, zones: [], content: {} });
+      for (const message of [
+        layoutPath,
+        "root must be a JSON object; received null",
+        "zones must be a JSON object; received []",
+        "content must be an array; received {}",
+      ]) {
+        expect(convert).toThrow(message);
+      }
+      fs.outputJsonSync(layoutPath, []);
+      expect(convert).toThrow("default layout must be a JSON object");
+    }
+  );
+
+  it.each([
+    { property: "purposes", allowedValues: purposes },
+    { property: "verticals", allowedValues: verticals },
+  ])(
+    "lists allowed $property and invalid entries",
+    ({ property, allowedValues }) => {
+      const { templateDirectory, convert } = createConversionFixture();
+      const metadataPath = path.join(templateDirectory, "template.json");
+      fs.outputJsonSync(metadataPath, {
+        displayName: "Example",
+        [property]: [allowedValues[0], "UNKNOWN", 42],
+      });
+      expect(convert).toThrow(
+        `${property} contains unsupported values: "UNKNOWN", 42`
+      );
+      expect(convert).toThrow(`Allowed values: ${allowedValues.join(", ")}`);
+      fs.outputJsonSync(metadataPath, {
+        displayName: "Example",
+        [property]: "UNKNOWN",
+      });
+      expect(convert).toThrow(
+        `${property} must be an array of supported strings; received "UNKNOWN"`
+      );
+    }
+  );
+
+  it.each(["bad name", "yext-"])(
+    "explains invalid template identifier %s",
+    (name) => {
+      const { rootDirectory, templateDirectory, convert } =
+        createConversionFixture();
+      const renamedDirectory = path.join(rootDirectory, "src/registry", name);
+      fs.renameSync(templateDirectory, renamedDirectory);
+      expect(convert).toThrow(renamedDirectory);
+      expect(convert).toThrow(
+        "must start with a letter or number and contain only letters, numbers, underscores, and hyphens"
+      );
+      if (name === "yext-") {
+        expect(convert).toThrow('After removing the yext prefix, ""');
+      }
+    }
+  );
+
+  it("names both templates with colliding normalized identifiers", () => {
+    const { rootDirectory, templateDirectory, convert } =
+      createConversionFixture();
+    const otherDirectory = path.join(
+      rootDirectory,
+      "src/registry/yext-example"
+    );
+    fs.copySync(templateDirectory, otherDirectory);
+    expect(convert).toThrow(
+      `${templateDirectory} and ${otherDirectory} both normalize to "example"`
+    );
+  });
+
+  it("names the conflicting generated layout and source templates", () => {
+    const { rootDirectory, templateDirectory, convert } =
+      createConversionFixture();
+    const otherDirectory = path.join(
+      rootDirectory,
+      "src/registry/example-directory"
+    );
+    fs.copySync(templateDirectory, otherDirectory);
+    expect(convert).toThrow(
+      `Legacy template ${otherDirectory} conflicts with generated layout ID "example-directory" from ${templateDirectory}`
+    );
+  });
+
+  it("identifies reserved template aliases and the source directory", () => {
+    const { rootDirectory, templateDirectory, convert } =
+      createConversionFixture();
+    const renamedDirectory = path.join(rootDirectory, "src/registry/yext-main");
+    fs.renameSync(templateDirectory, renamedDirectory);
+    expect(convert).toThrow(`${renamedDirectory} normalizes to "main"`);
+    expect(convert).toThrow(
+      "Reserved IDs: main, directory, locator, edit. Rename the template directory."
+    );
+  });
+});

packages/visual-editor/src/cli/commands/internal/convertTemplatesToSectionLibrary.ts

@@ -12,6 +12,8 @@ import {
 import { exportDirectoryLocatorSectionLibrary } from "./exportDirectoryLocatorSectionLibrary.ts";
 
 const SAFE_ID = /^[A-Za-z0-9][A-Za-z0-9_-]*$/;
+const ID_REQUIREMENTS =
+  "must start with a letter or number and contain only letters, numbers, underscores, and hyphens";
 const VERTICALS = new Set(verticals);
 const PURPOSES = new Set(purposes);
 const REQUIRED_BASE_SECTIONS = new Set(["Directory", "Locator"]);
@@ -225,24 +227,28 @@ const readLegacyTemplates = (rootDirectory: string): LegacyTemplate[] => {
   if (templateDirectories.length === 0) {
     throw new Error(`No legacy templates found in ${registryDirectory}`);
   }
-  const normalizedTemplateIds = new Set<string>();
+  const normalizedTemplateIds = new Map<string, string>();
   return templateDirectories.map((templateDirectory) => {
     if (!SAFE_ID.test(templateDirectory)) {
-      throw new Error(`Template ID is not valid: ${templateDirectory}`);
+      throw new Error(
+        `Template ID is not valid: ${path.join(registryDirectory, templateDirectory)}. Template IDs ${ID_REQUIREMENTS}.`
+      );
     }
     const templateId = removeYextPrefix(templateDirectory);
     if (!SAFE_ID.test(templateId)) {
-      throw new Error(`Template ID is not valid: ${templateDirectory}`);
+      throw new Error(
+        `Template ID is not valid: ${path.join(registryDirectory, templateDirectory)}. After removing the yext prefix, ${JSON.stringify(templateId)} ${ID_REQUIREMENTS}.`
+      );
     }
     if (normalizedTemplateIds.has(templateId)) {
       throw new Error(
-        `Template IDs collide after removing the yext- prefix: ${templateId}`
+        `Template IDs collide after removing the yext prefix: ${path.join(registryDirectory, normalizedTemplateIds.get(templateId)!)} and ${path.join(registryDirectory, templateDirectory)} both normalize to ${JSON.stringify(templateId)}.`
       );
     }
-    normalizedTemplateIds.add(templateId);
+    normalizedTemplateIds.set(templateId, templateDirectory);
     if (RESERVED_LAYOUT_IDS.has(templateId)) {
       throw new Error(
-        `Template ID is reserved for a generated template alias: ${templateId}`
+        `Template ID is reserved for a generated template alias: ${path.join(registryDirectory, templateDirectory)} normalizes to ${JSON.stringify(templateId)}. Reserved IDs: ${Array.from(RESERVED_LAYOUT_IDS).join(", ")}. Rename the template directory.`
       );
     }
     return readLegacyTemplate(
@@ -653,14 +659,38 @@ const readBaseLibrary = (libraryDirectory: string): BaseLibrary => {
         metadataPath,
         `layout metadata for ${entry.name}`
       );
-      if (
-        !isRecord(metadata) ||
-        !["ENTITY", "DIRECTORY", "LOCATOR"].includes(metadata.pageSetType) ||
-        typeof metadata.id !== "string" ||
-        metadata.id !== entry.name ||
-        !SAFE_ID.test(metadata.id)
-      ) {
-        throw new Error(`Base layout metadata is not valid: ${metadataPath}`);
+      const issues: string[] = [];
+      if (!isRecord(metadata)) {
+        issues.push("metadata must be a JSON object.");
+      } else {
+        if (
+          !["ENTITY", "DIRECTORY", "LOCATOR"].includes(metadata.pageSetType)
+        ) {
+          issues.push(
+            `pageSetType must be ENTITY, DIRECTORY, or LOCATOR; received ${JSON.stringify(metadata.pageSetType)}.`
+          );
+        }
+        if (typeof metadata.id !== "string") {
+          issues.push(
+            `id must be a string; received ${JSON.stringify(metadata.id)}.`
+          );
+        } else {
+          if (metadata.id !== entry.name) {
+            issues.push(
+              `id must match directory name ${JSON.stringify(entry.name)}; received ${JSON.stringify(metadata.id)}.`
+            );
+          }
+          if (!SAFE_ID.test(metadata.id)) {
+            issues.push(
+              `id ${ID_REQUIREMENTS}; received ${JSON.stringify(metadata.id)}.`
+            );
+          }
+        }
+      }
+      if (issues.length > 0) {
+        throw new Error(
+          `Base layout metadata is not valid: ${metadataPath}\n${issues.map((issue) => `  - ${issue}`).join("\n")}`
+        );
       }
       const defaultLayoutPath = path.join(directory, "defaultLayout.json");
       const defaultLayout = readJson(
@@ -738,15 +768,14 @@ const buildConversion = (
   const componentIds = new Set(components.keys());
   const directoryLayoutId = `${firstTemplate.templateId}-directory`;
   const locatorLayoutId = `${firstTemplate.templateId}-locator`;
-  if (
-    templates.some(
-      (template) =>
-        template.templateId === directoryLayoutId ||
-        template.templateId === locatorLayoutId
-    )
-  ) {
+  const conflictingTemplate = templates.find(
+    (template) =>
+      template.templateId === directoryLayoutId ||
+      template.templateId === locatorLayoutId
+  );
+  if (conflictingTemplate) {
     throw new Error(
-      "A legacy template ID conflicts with the generated Directory or Locator layout ID"
+      `Legacy template ${conflictingTemplate.directory} conflicts with generated layout ID ${JSON.stringify(conflictingTemplate.templateId)} from ${firstTemplate.directory}. Rename the conflicting template directory.`
     );
   }
   for (const template of templates) {
@@ -848,14 +877,17 @@ const readMetadataList = <T extends string>(
   if (value === undefined) {
     return [];
   }
-  if (
-    !Array.isArray(value) ||
-    value.some(
-      (item) => typeof item !== "string" || !allowedValues.has(item as T)
-    )
-  ) {
+  if (!Array.isArray(value)) {
+    throw new Error(
+      `${sourcePath} ${property} must be an array of supported strings; received ${JSON.stringify(value)}. Allowed values: ${Array.from(allowedValues).join(", ")}.`
+    );
+  }
+  const invalidValues = value.filter(
+    (item) => typeof item !== "string" || !allowedValues.has(item as T)
+  );
+  if (invalidValues.length > 0) {
     throw new Error(
-      `${sourcePath} ${property} must contain supported string values`
+      `${sourcePath} ${property} contains unsupported values: ${invalidValues.map((item) => JSON.stringify(item)).join(", ")}. Allowed values: ${Array.from(allowedValues).join(", ")}.`
     );
   }
   return value as T[];
@@ -866,14 +898,26 @@ function validateDefaultLayout(
   sourcePath: string,
   description: string
 ): asserts defaultLayout is JsonRecord {
-  if (
-    !isRecord(defaultLayout) ||
-    !isRecord(defaultLayout.root) ||
-    !isRecord(defaultLayout.zones) ||
-    !Array.isArray(defaultLayout.content)
-  ) {
+  const issues: string[] = [];
+  if (!isRecord(defaultLayout)) {
+    issues.push("default layout must be a JSON object.");
+  } else {
+    for (const field of ["root", "zones"] as const) {
+      if (!isRecord(defaultLayout[field])) {
+        issues.push(
+          `${field} must be a JSON object; received ${JSON.stringify(defaultLayout[field])}.`
+        );
+      }
+    }
+    if (!Array.isArray(defaultLayout.content)) {
+      issues.push(
+        `content must be an array; received ${JSON.stringify(defaultLayout.content)}.`
+      );
+    }
+  }
+  if (issues.length > 0) {
     throw new Error(
-      `${description} default layout is not valid: ${sourcePath}`
+      `${description} default layout is not valid: ${sourcePath}\n${issues.map((issue) => `  - ${issue}`).join("\n")}`
     );
   }
 }