diff --git a/.changeset/router-key-validation.md b/.changeset/router-key-validation.md new file mode 100644 index 00000000..a6eaa867 --- /dev/null +++ b/.changeset/router-key-validation.md @@ -0,0 +1,6 @@ +--- +"@valbuild/core": patch +"@valbuild/ui": patch +--- + +Add key validation support to `s.router` (mirroring `s.record`). Pass a string schema as the second argument to attach `.maxLength()`, `.regexp()`, `.validate()`, `.describe()`, etc. to router keys — for example `s.router(nextAppRouter, s.string().maxLength(60).describe("URL slug"), s.object({ ... }))`. Router URL pattern validation continues to run, and both error sources now surface together at the same key path. Validation errors on the same path are now merged instead of overwritten, so router, key and item errors on a key are all reported. The key description is also shown when adding or renaming a route from the sitemap. diff --git a/examples/next/app/blogs/[blog]/page.val.ts b/examples/next/app/blogs/[blog]/page.val.ts index a833f4d7..25156f53 100644 --- a/examples/next/app/blogs/[blog]/page.val.ts +++ b/examples/next/app/blogs/[blog]/page.val.ts @@ -15,7 +15,19 @@ const blogSchema = s.object({ export default c.define( "/app/blogs/[blog]/page.val.ts", - s.router(nextAppRouter, blogSchema), + s + .router( + nextAppRouter, + s.string().describe("The URL of the blog post"), + blogSchema, + ) + .render({ + as: "list", + select: ({ val }) => ({ + title: val.title, + subtitle: val.author, + }), + }), { "/blogs/blog2": { title: "Blog 2", diff --git a/examples/next/content/authors.val.ts b/examples/next/content/authors.val.ts index 8ec0f2d2..9ee6b87d 100644 --- a/examples/next/content/authors.val.ts +++ b/examples/next/content/authors.val.ts @@ -21,6 +21,7 @@ export const schema = s select: ({ val }) => ({ title: val.name, subtitle: val.birthdate, + image: val.image, }), }); diff --git a/packages/core/src/schema/array.ts b/packages/core/src/schema/array.ts index a18e59d1..be6554ed 100644 --- a/packages/core/src/schema/array.ts +++ b/packages/core/src/schema/array.ts @@ -9,10 +9,7 @@ import { SelectorSource } from "../selector"; import { unsafeCreateSourcePath } from "../selector/SelectorProxy"; import { ImageSource } from "../source/image"; import { ModuleFilePath, SourcePath } from "../val"; -import { - ValidationError, - ValidationErrors, -} from "./validation/ValidationError"; +import { ValidationErrors } from "./validation/ValidationError"; export type SerializedArraySchema = { type: "array"; @@ -73,21 +70,12 @@ export class ArraySchema< if (assertRes.data === null) { return false; } - let error: Record = {}; + let error: ValidationErrors = false; for (const [idx, i] of Object.entries(assertRes.data)) { const subPath = unsafeCreateSourcePath(path, Number(idx)); // eslint-disable-next-line @typescript-eslint/no-explicit-any const subError = this.item["executeValidate"](subPath, i as any); - if (subError) { - error = { - ...subError, - ...error, - }; - } - } - - if (Object.keys(error).length === 0) { - return false; + error = this.mergeValidationErrors(error, subError); } return error; } diff --git a/packages/core/src/schema/index.ts b/packages/core/src/schema/index.ts index 6a703950..c9172ae0 100644 --- a/packages/core/src/schema/index.ts +++ b/packages/core/src/schema/index.ts @@ -176,6 +176,35 @@ export abstract class Schema { } as ValidationErrors; } } + + /** + * Merges two sets of validation errors path-wise: errors on the same path are + * concatenated, not overwritten. Object spread cannot be used for this, since + * it replaces the array of the colliding path. A record, for example, validates + * the key and the item on the same path, so both sets must survive. + * + * MUTATES! since internal and perf sensitive + */ + protected mergeValidationErrors( + current: ValidationErrors, + incoming: ValidationErrors, + ): ValidationErrors { + if (!incoming) { + return current; + } + if (!current) { + return incoming; + } + for (const pathS in incoming) { + const path = pathS as SourcePath; + if (current[path]) { + current[path] = current[path].concat(incoming[path]); + } else { + current[path] = incoming[path]; + } + } + return current; + } } export type SelectorOfSchema> = diff --git a/packages/core/src/schema/object.ts b/packages/core/src/schema/object.ts index b26593f8..0ad8cec7 100644 --- a/packages/core/src/schema/object.ts +++ b/packages/core/src/schema/object.ts @@ -140,14 +140,7 @@ export class ObjectSchema< ); } else { const subError = schema["executeValidate"](subPath, src[key]); - if (subError && error) { - error = { - ...subError, - ...error, - }; - } else if (subError) { - error = subError; - } + error = this.mergeValidationErrors(error, subError); } } diff --git a/packages/core/src/schema/record.ts b/packages/core/src/schema/record.ts index 7ca5b6eb..85b33593 100644 --- a/packages/core/src/schema/record.ts +++ b/packages/core/src/schema/record.ts @@ -134,9 +134,7 @@ export class RecordSchema< } as ValidationErrors; } const routerValidations = this.getRouterValidations(path, src); - if (routerValidations) { - return routerValidations; - } + error = this.mergeValidationErrors(error, routerValidations); for (const customValidationError of customValidationErrors) { error = this.appendValidationError( error, @@ -202,15 +200,7 @@ export class RecordSchema< ...err, keyError: true, })); - if (error) { - if (error[keyPath]) { - error[keyPath] = [...error[keyPath], ...keyError[keyPath]]; - } else { - error[keyPath] = keyError[keyPath]; - } - } else { - error = keyError; - } + error = this.mergeValidationErrors(error, keyError); } } @@ -227,26 +217,15 @@ export class RecordSchema< } else if (this.mediaOptions) { // Media collection: validate key (path/URL) and entry (metadata) const keyErr = this.validateMediaKey(subPath, key); - if (keyErr) { - error = error ? { ...error, ...keyErr } : keyErr; - } + error = this.mergeValidationErrors(error, keyErr); const entryErr = this.validateMediaEntry(subPath, elem); - if (entryErr) { - error = error ? { ...error, ...entryErr } : entryErr; - } + error = this.mergeValidationErrors(error, entryErr); } else { const subError = this.item["executeValidate"]( subPath, elem as SelectorSource, ); - if (subError && error) { - error = { - ...subError, - ...error, - }; - } else if (subError) { - error = subError; - } + error = this.mergeValidationErrors(error, subError); } }); return error; diff --git a/packages/core/src/schema/router.test.ts b/packages/core/src/schema/router.test.ts index e0629c0f..78318fa3 100644 --- a/packages/core/src/schema/router.test.ts +++ b/packages/core/src/schema/router.test.ts @@ -1,6 +1,8 @@ import { initSchema } from "../initSchema"; import { router } from "./router"; import { nextAppRouter, ValRouter } from "../router"; +import { SourcePath } from "../val"; +import { deserializeSchema } from "./deserialize"; const s = initSchema(); @@ -91,4 +93,228 @@ describe("router", () => { expect(serialized.router).toBe("next-app-router"); expect(serialized.item.type).toBe("object"); }); + + describe("key validation", () => { + // Router that flags any urlPath starting with "/bad" + const strictMockRouter: ValRouter = { + getRouterId: () => "strict-mock-router", + validate: (_moduleFilePath, urlPaths) => + urlPaths + .filter((p) => p.startsWith("/bad")) + .map((p) => ({ + error: { + message: `URL path '${p}' is not a valid route`, + urlPath: p, + expectedPath: null, + }, + })), + }; + + it("should serialize describe on the key schema", () => { + const schema = s.router( + mockRouter, + s.string().describe("URL slug"), + s.object({ title: s.string() }), + ); + + const serialized = schema["executeSerialize"](); + expect(serialized.type).toBe("record"); + expect(serialized.router).toBe("test-router"); + expect(serialized.key).toBeDefined(); + expect(serialized.key?.type).toBe("string"); + if (serialized.key?.type === "string") { + expect(serialized.key.description).toBe("URL slug"); + } + }); + + it("should enforce maxLength on keys (with keyError flag)", () => { + const schema = s.router( + mockRouter, + s.string().maxLength(5), + s.object({ title: s.string() }), + ); + + const result = schema["executeValidate"]("/test.val.ts" as SourcePath, { + "/ok": { title: "Short" }, + "/way-too-long-key": { title: "Too long" }, + }); + + expect(result).not.toBe(false); + if (result !== false) { + const allErrors = Object.values(result).flat(); + const lengthErrors = allErrors.filter( + (e) => e.keyError && e.message.includes("at most 5"), + ); + expect(lengthErrors).toHaveLength(1); + } + }); + + it("should run custom validator on keys", () => { + const schema = s.router( + mockRouter, + s + .string() + .validate((key) => + key.startsWith("/") ? false : "Key must start with '/'", + ), + s.object({ title: s.string() }), + ); + + const result = schema["executeValidate"]("/test.val.ts" as SourcePath, { + "/good": { title: "Good" }, + "bad-no-slash": { title: "Bad" }, + }); + + expect(result).not.toBe(false); + if (result !== false) { + const allErrors = Object.values(result).flat(); + const customErrors = allErrors.filter( + (e) => e.keyError && e.message === "Key must start with '/'", + ); + expect(customErrors).toHaveLength(1); + } + }); + + it("should surface both router pattern and key schema errors at the same key path", () => { + const schema = s.router( + strictMockRouter, + s.string().maxLength(5), + s.object({ title: s.string() }), + ); + + const result = schema["executeValidate"]("/test.val.ts" as SourcePath, { + "/bad-and-too-long": { title: "Fails both" }, + }); + + expect(result).not.toBe(false); + if (result !== false) { + const keyPath = Object.keys(result).find((p) => + p.includes("/bad-and-too-long"), + ); + expect(keyPath).toBeDefined(); + if (keyPath) { + const errsAtKey = result[keyPath as SourcePath]; + // Router pattern error from strictMockRouter + expect( + errsAtKey.some( + (e) => e.keyError && e.message.includes("not a valid route"), + ), + ).toBe(true); + // Key schema maxLength error + expect( + errsAtKey.some( + (e) => e.keyError && e.message.includes("at most 5"), + ), + ).toBe(true); + } + } + }); + + it("should keep item errors on a key that also has router and key schema errors", () => { + // The item is a string, so its errors land on the key path itself: all + // three error sources collide on the same SourcePath + const schema = s.router( + strictMockRouter, + s.string().maxLength(5), + s.string().maxLength(3), + ); + + const result = schema["executeValidate"]("/test.val.ts" as SourcePath, { + "/bad-and-too-long": "item value is too long as well", + }); + + expect(result).not.toBe(false); + if (result !== false) { + const keyPath = Object.keys(result).find((p) => + p.includes("/bad-and-too-long"), + ); + expect(keyPath).toBeDefined(); + if (keyPath) { + const errsAtKey = result[keyPath as SourcePath]; + // Router pattern error + expect( + errsAtKey.some( + (e) => e.keyError && e.message.includes("not a valid route"), + ), + ).toBe(true); + // Key schema error + expect( + errsAtKey.some( + (e) => e.keyError && e.message.includes("at most 5"), + ), + ).toBe(true); + // Item error must not be dropped by the merge + expect( + errsAtKey.some( + (e) => !e.keyError && e.message.includes("at most 3"), + ), + ).toBe(true); + } + } + }); + + it("should still work with the two-arg form (no key schema)", () => { + const schema = s.router(mockRouter, s.object({ title: s.string() })); + const result = schema["executeValidate"]("/test.val.ts" as SourcePath, { + "any-key-is-fine": { title: "Yes" }, + "another-key": { title: "Also fine" }, + }); + expect(result).toBe(false); + + const serialized = schema["executeSerialize"](); + expect(serialized.key).toBeDefined(); + expect(serialized.key?.type).toBe("string"); + }); + + it("should be equivalent to s.record(key, value).router()", () => { + const keySchema = s.string().maxLength(5).describe("URL slug"); + const valueSchema = s.object({ title: s.string() }); + + const viaRouter = s.router(mockRouter, keySchema, valueSchema); + const viaRecord = s.record(keySchema, valueSchema).router(mockRouter); + + const a = viaRouter["executeSerialize"](); + const b = viaRecord["executeSerialize"](); + + expect(a.type).toBe(b.type); + expect(a.router).toBe(b.router); + expect(a.opt).toBe(b.opt); + expect(a.key?.type).toBe(b.key?.type); + if (a.key?.type === "string" && b.key?.type === "string") { + expect(a.key.description).toBe(b.key.description); + } + }); + + it("should round-trip through serialize/deserialize with key validation", () => { + const schema = s.router( + mockRouter, + s.string().maxLength(5).describe("URL slug"), + s.object({ title: s.string() }), + ); + + const serialized = schema["executeSerialize"](); + const deserialized = deserializeSchema(serialized); + + const failing = deserialized["executeValidate"]( + "/test.val.ts" as SourcePath, + { "way-too-long": { title: "X" } }, + ); + expect(failing).not.toBe(false); + if (failing !== false) { + const allErrors = Object.values(failing).flat(); + expect( + allErrors.some((e) => e.keyError && e.message.includes("at most 5")), + ).toBe(true); + } + + const reserialized = deserialized["executeSerialize"](); + expect(reserialized.type).toBe("record"); + if ( + reserialized.type === "record" && + reserialized.key?.type === "string" + ) { + expect(reserialized.key.description).toBe("URL slug"); + } + }); + }); }); diff --git a/packages/core/src/schema/router.ts b/packages/core/src/schema/router.ts index 052fb2d9..51559644 100644 --- a/packages/core/src/schema/router.ts +++ b/packages/core/src/schema/router.ts @@ -5,16 +5,29 @@ import { RecordSchema } from "./record"; import { string } from "./string"; export function router< + K extends Schema, T extends Schema, - Src extends Record>, ->(router: ValRouter, item: T): RecordSchema, Src> { - const keySchema = string(); - const recordSchema = new RecordSchema, Src>( - item, - false, - [], - router, - keySchema, - ); - return recordSchema; +>( + router: ValRouter, + key: K, + item: T, +): RecordSchema, SelectorOfSchema>>; + +export function router>( + router: ValRouter, + item: T, +): RecordSchema, Record>>; + +export function router< + K extends Schema, + T extends Schema, +>( + router: ValRouter, + keyOrItem: K | T, + maybeItem?: T, +): RecordSchema, SelectorOfSchema>> { + if (maybeItem) { + return new RecordSchema(maybeItem, false, [], router, keyOrItem as K); + } + return new RecordSchema(keyOrItem as T, false, [], router, string()); } diff --git a/packages/ui/spa/components/AddRecordPopover.tsx b/packages/ui/spa/components/AddRecordPopover.tsx index dd94bd45..0a779949 100644 --- a/packages/ui/spa/components/AddRecordPopover.tsx +++ b/packages/ui/spa/components/AddRecordPopover.tsx @@ -119,6 +119,9 @@ export function AddRecordPopover({ } const [moduleFilePath, modulePath] = Internal.splitModuleFilePathAndModulePath(path); + // Callers may pass the description explicitly, but fall back to the key schema + // so that call sites which only know the path (the sitemap, for example) show it too + const description = keyDescription ?? schema.key?.description; return ( @@ -135,12 +138,12 @@ export function AddRecordPopover({ -

{keyDescription ? `Add ${keyDescription}` : "Add"}

+

{description ? `Add ${description}` : "Add"}

- {keyDescription && ( -
{keyDescription}
+ {description && ( +
{description}
)} {routePattern ? ( { if ("data" in parentSource && parentSource.data) { return Object.keys(parentSource.data); @@ -134,12 +146,12 @@ export function ChangeRecordPopover({ - {keyDescription ? `Rename ${keyDescription}` : "Rename record"} + {description ? `Rename ${description}` : "Rename record"} - {keyDescription && ( -
{keyDescription}
+ {description && ( +
{description}
)} {routePattern ? (