diff --git a/packages/visual-editor/src/cli/commands/internal/convertTemplatesToSectionLibrary.test.ts b/packages/visual-editor/src/cli/commands/internal/convertTemplatesToSectionLibrary.test.ts index f3d0a6516..5a0f667da 100644 --- a/packages/visual-editor/src/cli/commands/internal/convertTemplatesToSectionLibrary.test.ts +++ b/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." + ); + }); +}); diff --git a/packages/visual-editor/src/cli/commands/internal/convertTemplatesToSectionLibrary.ts b/packages/visual-editor/src/cli/commands/internal/convertTemplatesToSectionLibrary.ts index 2298ace97..5f40da5f2 100644 --- a/packages/visual-editor/src/cli/commands/internal/convertTemplatesToSectionLibrary.ts +++ b/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(); + const normalizedTemplateIds = new Map(); 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 = ( 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")}` ); } }