From 13c75e6371265de4a14a7e3a0019b232b6f7fa78 Mon Sep 17 00:00:00 2001 From: Fredrik Ekholdt Date: Wed, 24 Jun 2026 17:06:05 +0200 Subject: [PATCH 01/16] Possible to store files under public/ not only public/val --- packages/cli/src/runValidation.ts | 6 +++--- packages/core/src/schema/files.ts | 2 +- packages/core/src/schema/images.ts | 12 ++++++------ packages/core/src/source/image.ts | 2 +- packages/core/src/source/remote.ts | 4 ++-- packages/ui/spa/search/search.worker.ts | 2 +- packages/ui/spa/utils/getFilenameFromRef.ts | 10 +++++----- 7 files changed, 19 insertions(+), 19 deletions(-) diff --git a/packages/cli/src/runValidation.ts b/packages/cli/src/runValidation.ts index 4c27f2b64..8b08db20c 100644 --- a/packages/cli/src/runValidation.ts +++ b/packages/cli/src/runValidation.ts @@ -335,12 +335,12 @@ export async function handleRemoteFileUpload( const relativeFilePath = path .relative(ctx.projectRoot, filePath) .split(path.sep) - .join("/") as `public/val/${string}`; + .join("/") as `public/${string}`; - if (!relativeFilePath.startsWith("public/val/")) { + if (!relativeFilePath.startsWith("public")) { return { success: false, - errorMessage: `File path must be within the public/val/ directory (e.g. public/val/path/to/file.txt). Got: ${relativeFilePath}`, + errorMessage: `File path must be within the public/ directory (e.g. public/path/to/file.txt). Got: ${relativeFilePath}`, }; } diff --git a/packages/core/src/schema/files.ts b/packages/core/src/schema/files.ts index ef645ac76..c705fe0b7 100644 --- a/packages/core/src/schema/files.ts +++ b/packages/core/src/schema/files.ts @@ -60,7 +60,7 @@ export const files = ( Schema, Record > => { - const directory = options.directory ?? "/public/val"; + const directory = options.directory ?? "/public"; const itemSchema = new ObjectSchema( { mimeType: new StringSchema({}, false) }, false, diff --git a/packages/core/src/schema/images.ts b/packages/core/src/schema/images.ts index 38b308002..c4aab059b 100644 --- a/packages/core/src/schema/images.ts +++ b/packages/core/src/schema/images.ts @@ -20,7 +20,7 @@ export type ImagesOptions = { /** * The accepted mime type pattern. Must be an image type (e.g., "image/png", "image/webp", "image/*") */ - accept: Accept; + accept?: Accept; /** * The directory where images should be stored. * Must start with "/public" (e.g., "/public/val/images") @@ -92,14 +92,14 @@ type ImagesItemSrc = { * ``` */ export const images = ( - options: ImagesOptions, + options?: ImagesOptions, ): RecordSchema< ObjectSchema, Schema, Record > => { - const directory = options.directory ?? "/public/val"; - const altSchema = options.alt ?? string().nullable(); + const directory = options?.directory ?? "/public/val"; + const altSchema = options?.alt ?? string().nullable(); const itemSchema = new ObjectSchema( { width: new NumberSchema(undefined, false), @@ -111,9 +111,9 @@ export const images = ( ) as ObjectSchema; return new RecordSchema(itemSchema, false, [], null, null, { type: "images", - accept: options.accept, + accept: options?.accept ?? "image/*", directory, - remote: options.remote ?? false, + remote: options?.remote ?? false, altSchema, }); }; diff --git a/packages/core/src/source/image.ts b/packages/core/src/source/image.ts index 465db5a6c..97f9bcca7 100644 --- a/packages/core/src/source/image.ts +++ b/packages/core/src/source/image.ts @@ -19,7 +19,7 @@ export type ImageSource< export const initImage = (config?: ValConfig) => { // eslint-disable-next-line @typescript-eslint/no-unused-vars - const fileDirectory = config?.files?.directory ?? "/public/val"; + const fileDirectory = config?.files?.directory ?? "/public"; type FileDirectory = typeof fileDirectory; diff --git a/packages/core/src/source/remote.ts b/packages/core/src/source/remote.ts index 56b25130e..ec5991906 100644 --- a/packages/core/src/source/remote.ts +++ b/packages/core/src/source/remote.ts @@ -31,7 +31,7 @@ export const initRemote = (config?: ValConfig) => { }; export type RemoteRef = - `${string}/file/p/${string}/v/${string}/h/${string}/f/${string}/p/public/val/${string}`; + `${string}/file/p/${string}/v/${string}/h/${string}/f/${string}/p/public/${string}`; export function createRemoteRef( remoteHost: string, @@ -47,7 +47,7 @@ export function createRemoteRef( coreVersion: string; validationHash: string; fileHash: string; - filePath: `public/val/${string}`; + filePath: `public/${string}`; bucket: string; }, ): RemoteRef { diff --git a/packages/ui/spa/search/search.worker.ts b/packages/ui/spa/search/search.worker.ts index c7a2f3348..7594185ae 100644 --- a/packages/ui/spa/search/search.worker.ts +++ b/packages/ui/spa/search/search.worker.ts @@ -69,7 +69,7 @@ function buildIndex( ) { const filename = source[FILE_REF_PROP] as string; // Extract just the filename from the path - const filenameOnly = filename.replace("/public/val/", ""); + const filenameOnly = filename.replace("/public", ""); const metadata = source?.metadata; const alt = metadata && typeof metadata === "object" && "alt" in metadata diff --git a/packages/ui/spa/utils/getFilenameFromRef.ts b/packages/ui/spa/utils/getFilenameFromRef.ts index 1ff766857..13017628a 100644 --- a/packages/ui/spa/utils/getFilenameFromRef.ts +++ b/packages/ui/spa/utils/getFilenameFromRef.ts @@ -10,7 +10,7 @@ function cleanRefPath(ref: string): string { /** * Extract a human-readable filename from a file ref (local path or remote URL). - * Handles both remote refs (via splitRemoteRef) and plain `/public/val/...` paths. + * Handles both remote refs (via splitRemoteRef) and plain `/public/...` paths. */ export function getFilenameFromRef(ref: string): string { const cleanPath = cleanRefPath(ref); @@ -19,10 +19,10 @@ export function getFilenameFromRef(ref: string): string { /** * Parse a file ref into its constituent parts: a clean path, filename, and - * folder (the path relative to `/public/val`). + * folder (the path relative to `/public`). * - * The `folder` strips the standard `/public/val` prefix so it shows a - * concise location like `/images` instead of `/public/val/images`. + * The `folder` strips the standard `/public` prefix so it shows a + * concise location like `/images` instead of `/public/images`. */ export function getRefParts(ref: string): { cleanPath: string; @@ -32,6 +32,6 @@ export function getRefParts(ref: string): { const cleanPath = cleanRefPath(ref); const filename = cleanPath.split("/").pop() || cleanPath; const folder = - cleanPath.replace("/public/val", "").replace(/\/[^/]+$/, "") || "/"; + cleanPath.replace("/public", "").replace(/\/[^/]+$/, "") || "/"; return { cleanPath, filename, folder }; } From e1d694f566b8633297cd1b1d9a355d7e3d8a659c Mon Sep 17 00:00:00 2001 From: Fredrik Ekholdt Date: Wed, 24 Jun 2026 17:45:05 +0200 Subject: [PATCH 02/16] Allow files in public dir, not only public/val dir. --- packages/cli/src/cli.ts | 2 +- packages/cli/src/listUnusedFiles.ts | 11 ++++++++++- packages/cli/src/runValidation.ts | 2 +- packages/cli/src/utils/evalValConfigFile.ts | 10 +++++++--- packages/core/src/initVal.ts | 2 +- packages/core/src/remote/splitRemoteRef.ts | 6 +++--- packages/core/src/schema/files.ts | 2 +- packages/core/src/source/image.ts | 2 +- packages/server/src/checkRemoteRef.ts | 6 +++--- packages/server/src/createFixPatch.ts | 4 ++-- packages/shared/src/internal/SharedValConfig.ts | 10 +++++++++- .../richtext/conversion/remirrorToRichTextSource.ts | 2 +- packages/ui/spa/components/fields/FileField.tsx | 2 +- packages/ui/spa/components/fields/ModuleGallery.tsx | 2 +- 14 files changed, 42 insertions(+), 21 deletions(-) diff --git a/packages/cli/src/cli.ts b/packages/cli/src/cli.ts index 2be3aaa7e..14af857ef 100644 --- a/packages/cli/src/cli.ts +++ b/packages/cli/src/cli.ts @@ -43,7 +43,7 @@ async function main(): Promise { Command: list-unused-files Description: EXPERIMENTAL. - List files that are in public/val but not in use by any Val module. + List files that are in the configured files directory (files.directory, default public/val) but not in use by any Val module. This is useful for cleaning up unused files. Options: --root [root], -r [root] Set project root directory (default process.cwd()) diff --git a/packages/cli/src/listUnusedFiles.ts b/packages/cli/src/listUnusedFiles.ts index 47976e574..eeccdce75 100644 --- a/packages/cli/src/listUnusedFiles.ts +++ b/packages/cli/src/listUnusedFiles.ts @@ -8,11 +8,20 @@ import { import { createService } from "@valbuild/server"; import { glob } from "fast-glob"; import path from "path"; +import { evalValConfigFile } from "./utils/evalValConfigFile"; export async function listUnusedFiles({ root }: { root?: string }) { - const managedDir = "public/val"; const projectRoot = root ? path.resolve(root) : process.cwd(); + const valConfigFile = + (await evalValConfigFile(projectRoot, "val.config.ts")) || + (await evalValConfigFile(projectRoot, "val.config.js")); + // Strip the leading "/" so it is relative to the project root (e.g. "public/val"). + const managedDir = (valConfigFile?.files?.directory ?? "/public/val").replace( + /^\//, + "", + ); + const service = await createService(projectRoot, {}); const valFiles: string[] = await glob("**/*.val.{js,ts}", { diff --git a/packages/cli/src/runValidation.ts b/packages/cli/src/runValidation.ts index 8b08db20c..4758df2ac 100644 --- a/packages/cli/src/runValidation.ts +++ b/packages/cli/src/runValidation.ts @@ -337,7 +337,7 @@ export async function handleRemoteFileUpload( .split(path.sep) .join("/") as `public/${string}`; - if (!relativeFilePath.startsWith("public")) { + if (!relativeFilePath.startsWith("public/")) { return { success: false, errorMessage: `File path must be within the public/ directory (e.g. public/path/to/file.txt). Got: ${relativeFilePath}`, diff --git a/packages/cli/src/utils/evalValConfigFile.ts b/packages/cli/src/utils/evalValConfigFile.ts index b488f0aff..6686f164b 100644 --- a/packages/cli/src/utils/evalValConfigFile.ts +++ b/packages/cli/src/utils/evalValConfigFile.ts @@ -13,9 +13,13 @@ const ValConfigSchema = z.object({ .object({ directory: z .string() - .refine((val): val is `/public/val` => val.startsWith("/public/val"), { - message: "files.directory must start with '/public/val'", - }), + .refine( + (val): val is `/public` | `/public/${string}` => + val === "/public" || val.startsWith("/public/"), + { + message: "files.directory must start with '/public'", + }, + ), }) .optional(), gitCommit: z.string().optional(), diff --git a/packages/core/src/initVal.ts b/packages/core/src/initVal.ts index 873e6b96c..ff738279b 100644 --- a/packages/core/src/initVal.ts +++ b/packages/core/src/initVal.ts @@ -18,7 +18,7 @@ export type ValConstructor = { unstable_getPath: typeof getPath; }; -export type ConfigDirectory = `/public/val`; +export type ConfigDirectory = `/public` | `/public/${string}`; export type ValConfig = { project?: string; diff --git a/packages/core/src/remote/splitRemoteRef.ts b/packages/core/src/remote/splitRemoteRef.ts index 13078db2f..a7b61e53b 100644 --- a/packages/core/src/remote/splitRemoteRef.ts +++ b/packages/core/src/remote/splitRemoteRef.ts @@ -10,7 +10,7 @@ export function splitRemoteRef(ref: string): version: string; validationHash: string; fileHash: string; - filePath: `public/val/${string}`; + filePath: `public/${string}`; } | { status: "error"; @@ -29,7 +29,7 @@ export function splitRemoteRef(ref: string): error: "Invalid remote ref: " + ref, }; } - if (match[7].indexOf("public/val/") !== 0) { + if (match[7].indexOf("public/") !== 0) { return { status: "error", error: "Invalid remote ref: " + ref, @@ -44,6 +44,6 @@ export function splitRemoteRef(ref: string): version: match[4], validationHash: match[5], fileHash: match[6], - filePath: match[7] as `public/val/${string}`, + filePath: match[7] as `public/${string}`, }; } diff --git a/packages/core/src/schema/files.ts b/packages/core/src/schema/files.ts index c705fe0b7..ef645ac76 100644 --- a/packages/core/src/schema/files.ts +++ b/packages/core/src/schema/files.ts @@ -60,7 +60,7 @@ export const files = ( Schema, Record > => { - const directory = options.directory ?? "/public"; + const directory = options.directory ?? "/public/val"; const itemSchema = new ObjectSchema( { mimeType: new StringSchema({}, false) }, false, diff --git a/packages/core/src/source/image.ts b/packages/core/src/source/image.ts index 97f9bcca7..465db5a6c 100644 --- a/packages/core/src/source/image.ts +++ b/packages/core/src/source/image.ts @@ -19,7 +19,7 @@ export type ImageSource< export const initImage = (config?: ValConfig) => { // eslint-disable-next-line @typescript-eslint/no-unused-vars - const fileDirectory = config?.files?.directory ?? "/public"; + const fileDirectory = config?.files?.directory ?? "/public/val"; type FileDirectory = typeof fileDirectory; diff --git a/packages/server/src/checkRemoteRef.ts b/packages/server/src/checkRemoteRef.ts index 0b6e17994..c377b0b9d 100644 --- a/packages/server/src/checkRemoteRef.ts +++ b/packages/server/src/checkRemoteRef.ts @@ -49,10 +49,10 @@ export async function checkRemoteRef( error: `File path is missing in remote ref: ${ref}`, }; } - if (!relativeFilePath.startsWith("public/val/")) { + if (!relativeFilePath.startsWith("public/")) { return { status: "error", - error: `File path must be within the public/val/ directory (e.g. public/val/path/to/file.txt). Got: ${relativeFilePath}`, + error: `File path must be within the public/ directory (e.g. public/path/to/file.txt). Got: ${relativeFilePath}`, }; } const coreVersion = Internal.VERSION.core || "unknown"; @@ -148,7 +148,7 @@ export async function checkRemoteRef( } const newFileExt = Internal.mimeTypeToFileExt(updatedMetadata.mimeType); const newFilePath = (relativeFilePath.slice(0, -fileExt.length) + - newFileExt) as `public/val/${string}`; + newFileExt) as `public/${string}`; return { status: "fix-required", metadata: updatedMetadata, diff --git a/packages/server/src/createFixPatch.ts b/packages/server/src/createFixPatch.ts index 3b6e2eeb6..765f65104 100644 --- a/packages/server/src/createFixPatch.ts +++ b/packages/server/src/createFixPatch.ts @@ -317,11 +317,11 @@ export async function createFixPatch( }); continue; } - if (!filePath.startsWith("public/val/")) { + if (!filePath.startsWith("public/")) { remainingErrors.push({ ...validationError, message: - "Unexpected error while downloading remote (invalid file path - must start with public/val/)", + "Unexpected error while downloading remote (invalid file path - must start with public/)", fixes: undefined, }); continue; diff --git a/packages/shared/src/internal/SharedValConfig.ts b/packages/shared/src/internal/SharedValConfig.ts index fde829620..e576605f9 100644 --- a/packages/shared/src/internal/SharedValConfig.ts +++ b/packages/shared/src/internal/SharedValConfig.ts @@ -13,7 +13,15 @@ export const SharedValConfig: z.ZodSchema< root: z.string().optional(), files: z .object({ - directory: z.literal("/public/val"), + directory: z + .string() + .refine( + (val): val is `/public` | `/public/${string}` => + val === "/public" || val.startsWith("/public/"), + { + message: "files.directory must start with '/public'", + }, + ), }) .optional(), gitCommit: z.string().optional(), diff --git a/packages/shared/src/internal/richtext/conversion/remirrorToRichTextSource.ts b/packages/shared/src/internal/richtext/conversion/remirrorToRichTextSource.ts index 9b58cbca9..90260c508 100644 --- a/packages/shared/src/internal/richtext/conversion/remirrorToRichTextSource.ts +++ b/packages/shared/src/internal/richtext/conversion/remirrorToRichTextSource.ts @@ -413,7 +413,7 @@ function convertImageNode( remoteFileHash, textEncoder, ), - filePath: filePath.slice(1) as `public/val/${string}`, + filePath: filePath.slice(1) as `public/${string}`, }) : (filePath as `/public/${string}`); if (existingFilesEntry) { diff --git a/packages/ui/spa/components/fields/FileField.tsx b/packages/ui/spa/components/fields/FileField.tsx index 56fb09de1..e320ba5ab 100644 --- a/packages/ui/spa/components/fields/FileField.tsx +++ b/packages/ui/spa/components/fields/FileField.tsx @@ -85,7 +85,7 @@ export async function createFilePatch( textEncoder, ), fileHash: remoteFileHash, - filePath: `${(directory ?? "/public/val").slice(1) as `public/val/${string}`}/${newFilePath}`, + filePath: `${(directory ?? "/public/val").slice(1) as `public/${string}`}/${newFilePath}`, }) : filePath; return { diff --git a/packages/ui/spa/components/fields/ModuleGallery.tsx b/packages/ui/spa/components/fields/ModuleGallery.tsx index 646ede370..9b613f1f1 100644 --- a/packages/ui/spa/components/fields/ModuleGallery.tsx +++ b/packages/ui/spa/components/fields/ModuleGallery.tsx @@ -302,7 +302,7 @@ export function ModuleGallery({ bucket: remoteData.bucket, validationHash, fileHash: remoteFileHash, - filePath: `${directory.slice(1) as `public/val/${string}`}/${newFilename}`, + filePath: `${directory.slice(1) as `public/${string}`}/${newFilename}`, }); isRemote = true; } else { From 869bc9d35670f31868bb89ae090132b6edce6701 Mon Sep 17 00:00:00 2001 From: Fredrik Ekholdt Date: Wed, 24 Jun 2026 17:48:20 +0200 Subject: [PATCH 03/16] Changeset --- .changeset/public-dir-not-only-public-val.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/public-dir-not-only-public-val.md diff --git a/.changeset/public-dir-not-only-public-val.md b/.changeset/public-dir-not-only-public-val.md new file mode 100644 index 000000000..03255cea9 --- /dev/null +++ b/.changeset/public-dir-not-only-public-val.md @@ -0,0 +1,5 @@ +--- +"@valbuild/core": patch +--- + +Allow storing files anywhere under `/public`, not only `/public/val`. Config validation, remote refs, and the studio file/image fields now accept any `/public/...` directory (the default remains `/public/val`). Also removed the `files` property from `SharedValConfig` — the per-schema `s.files`/`s.images` directory is now the source of truth. From 957aacc4f54edd6eb3a7736514fff8165cc5f05a Mon Sep 17 00:00:00 2001 From: Fredrik Ekholdt Date: Wed, 24 Jun 2026 20:24:58 +0200 Subject: [PATCH 04/16] Remove "files" directory option --- packages/shared/src/internal/SharedValConfig.ts | 16 ++-------------- packages/ui/spa/components/fields/FileField.tsx | 2 +- packages/ui/spa/components/fields/ImageField.tsx | 2 +- .../ui/spa/components/fields/RichTextField.tsx | 2 +- 4 files changed, 5 insertions(+), 17 deletions(-) diff --git a/packages/shared/src/internal/SharedValConfig.ts b/packages/shared/src/internal/SharedValConfig.ts index e576605f9..41d0f09b4 100644 --- a/packages/shared/src/internal/SharedValConfig.ts +++ b/packages/shared/src/internal/SharedValConfig.ts @@ -2,7 +2,8 @@ import { ValConfig } from "@valbuild/core"; import { z } from "zod"; export const SharedValConfig: z.ZodSchema< - ValConfig & { + // The files directory is not shared - the per-schema s.files/s.images directory is the source of truth. + Omit & { // We are adding URLs and other server only config options here contentHostUrl?: string; } @@ -11,19 +12,6 @@ export const SharedValConfig: z.ZodSchema< appHostUrl: z.string().optional(), project: z.string().optional(), root: z.string().optional(), - files: z - .object({ - directory: z - .string() - .refine( - (val): val is `/public` | `/public/${string}` => - val === "/public" || val.startsWith("/public/"), - { - message: "files.directory must start with '/public'", - }, - ), - }) - .optional(), gitCommit: z.string().optional(), gitBranch: z.string().optional(), defaultTheme: z.enum(["dark", "light"]).optional(), diff --git a/packages/ui/spa/components/fields/FileField.tsx b/packages/ui/spa/components/fields/FileField.tsx index e320ba5ab..0648ae3fd 100644 --- a/packages/ui/spa/components/fields/FileField.tsx +++ b/packages/ui/spa/components/fields/FileField.tsx @@ -438,7 +438,7 @@ export function FileField({ metadata, type, remoteData, - moduleDirectory ?? config.files?.directory, + moduleDirectory, !!referencedModule, ) .then(({ patch, filePath }) => { diff --git a/packages/ui/spa/components/fields/ImageField.tsx b/packages/ui/spa/components/fields/ImageField.tsx index 322fe6205..3ce68e94c 100644 --- a/packages/ui/spa/components/fields/ImageField.tsx +++ b/packages/ui/spa/components/fields/ImageField.tsx @@ -223,7 +223,7 @@ export function ImageField({ addAndUploadPatchWithFileOps, addModuleFilePatch, remoteData, - directory: moduleDirectory ?? config.files?.directory, + directory: moduleDirectory, referencedModule, existingAlt, }); diff --git a/packages/ui/spa/components/fields/RichTextField.tsx b/packages/ui/spa/components/fields/RichTextField.tsx index bf80fd556..99ae7bb93 100644 --- a/packages/ui/spa/components/fields/RichTextField.tsx +++ b/packages/ui/spa/components/fields/RichTextField.tsx @@ -152,7 +152,7 @@ export function RichTextField({ config, ]); - const imageDirectory = imageModuleDirectory ?? config?.files?.directory; + const imageDirectory = imageModuleDirectory; const onImageUpload = useMemo(() => { if (!hasImageEnabled) return undefined; From 1c503053c3115da0ca194892ffb653c7b897d805 Mon Sep 17 00:00:00 2001 From: Fredrik Ekholdt Date: Wed, 24 Jun 2026 21:25:44 +0200 Subject: [PATCH 05/16] Replace quickjs based validation with node:vm running val.modules --- .../cli/src/__fixtures__/basic/val.config.ts | 4 +- .../cli/src/__fixtures__/basic/val.modules.ts | 16 ++ packages/cli/src/listUnusedFiles.ts | 5 + packages/cli/src/runValidation.ts | 15 +- packages/cli/src/validate.ts | 6 + packages/react/src/internal/index.ts | 2 +- packages/react/src/stega/index.ts | 2 +- packages/server/package.json | 1 - packages/server/src/Service.ts | 132 ++++++------ packages/server/src/ValQuickJSRuntime.ts | 191 ----------------- packages/server/src/loadValModules.ts | 196 ++++++++++++++++++ packages/server/src/patchValFile.ts | 3 - packages/server/src/readValFile.test.ts | 59 ------ packages/server/src/readValFile.ts | 158 -------------- pnpm-lock.yaml | 10 +- 15 files changed, 314 insertions(+), 486 deletions(-) create mode 100644 packages/cli/src/__fixtures__/basic/val.modules.ts delete mode 100644 packages/server/src/ValQuickJSRuntime.ts create mode 100644 packages/server/src/loadValModules.ts delete mode 100644 packages/server/src/readValFile.test.ts delete mode 100644 packages/server/src/readValFile.ts diff --git a/packages/cli/src/__fixtures__/basic/val.config.ts b/packages/cli/src/__fixtures__/basic/val.config.ts index 8db86c966..fd8aef37b 100644 --- a/packages/cli/src/__fixtures__/basic/val.config.ts +++ b/packages/cli/src/__fixtures__/basic/val.config.ts @@ -1,5 +1,5 @@ import { initVal } from "@valbuild/core"; -const { s, c } = initVal(); +const { s, c, config } = initVal(); -export { s, c }; +export { s, c, config }; diff --git a/packages/cli/src/__fixtures__/basic/val.modules.ts b/packages/cli/src/__fixtures__/basic/val.modules.ts new file mode 100644 index 000000000..b500c3bd0 --- /dev/null +++ b/packages/cli/src/__fixtures__/basic/val.modules.ts @@ -0,0 +1,16 @@ +import { modules } from "@valbuild/core"; +import { config } from "./val.config"; + +export default modules(config, [ + { def: () => import("./content/basic-valid.val") }, + { def: () => import("./content/basic-errors.val") }, + { def: () => import("./content/basic-files.val") }, + { def: () => import("./content/basic-image.val") }, + { def: () => import("./content/basic-image-from-gallery.val") }, + { def: () => import("./content/basic-image-from-galleries.val") }, + { def: () => import("./content/basic-gallery.val") }, + { def: () => import("./content/basic-gallery-2.val") }, + { def: () => import("./content/basic-gallery-fail-on-non-unique-dir.val") }, + { def: () => import("./content/basic-gallery-missing-tracked.val") }, + { def: () => import("./content/basic-gallery-wrong-metadata.val") }, +]); diff --git a/packages/cli/src/listUnusedFiles.ts b/packages/cli/src/listUnusedFiles.ts index eeccdce75..c8bef1f50 100644 --- a/packages/cli/src/listUnusedFiles.ts +++ b/packages/cli/src/listUnusedFiles.ts @@ -23,6 +23,7 @@ export async function listUnusedFiles({ root }: { root?: string }) { ); const service = await createService(projectRoot, {}); + const registered = new Set(service.getModuleFilePaths()); const valFiles: string[] = await glob("**/*.val.{js,ts}", { ignore: ["node_modules/**"], @@ -32,6 +33,10 @@ export async function listUnusedFiles({ root }: { root?: string }) { const filesUsedByVal: string[] = []; async function pushFilesUsedByVal(file: string) { const moduleId = `/${file}` as ModuleFilePath; // TODO: check if this always works? (Windows?) + if (!registered.has(moduleId)) { + // Not registered in val.modules - skip (e.g. reusable schema fragments). + return; + } const valModule = await service.get(moduleId, "" as ModulePath, { validate: true, source: true, diff --git a/packages/cli/src/runValidation.ts b/packages/cli/src/runValidation.ts index 4758df2ac..4016546ed 100644 --- a/packages/cli/src/runValidation.ts +++ b/packages/cli/src/runValidation.ts @@ -117,6 +117,7 @@ export type ValidationEvent = fixable: boolean; } | { type: "unknown-fix"; sourcePath: string; fixes: string[] } + | { type: "unregistered-module"; file: string } | { type: "fix-applied"; file: string; sourcePath: string } | { type: "fatal-error"; file: string; message: string } | { type: "remote-uploading"; ref: string } @@ -604,14 +605,18 @@ export async function* runValidation({ const service = await createService(projectRoot, {}, fs); + // Modules registered in the project's val.modules. Files found on disk that + // are not registered here are not validated (a warning is emitted instead). + const registered = new Set(service.getModuleFilePaths()); + let errors = 0; // Build a single schema/source snapshot up front so the shared resolver // can resolve keyof:check-keys / router:check-route references that span - // multiple val files. + // multiple val files. Use the full registry so cross-module references + // resolve even against modules not in the validated subset. const snapshot: SchemaSourceSnapshot = { schemas: {}, sources: {} }; - for (const file of valFiles) { - const moduleFilePath = `/${file}` as ModuleFilePath; + for (const moduleFilePath of registered) { const valModule = await service.get(moduleFilePath, "" as ModulePath, { source: true, schema: true, @@ -627,6 +632,10 @@ export async function* runValidation({ async function* validateFile(file: string): AsyncGenerator { const moduleFilePath = `/${file}` as ModuleFilePath; // TODO: check if this always works? (Windows?) + if (!registered.has(moduleFilePath)) { + yield { type: "unregistered-module", file }; + return; + } const start = Date.now(); const valModule = await service.get(moduleFilePath, "" as ModulePath, { source: true, diff --git a/packages/cli/src/validate.ts b/packages/cli/src/validate.ts index 8f37d4404..e7f425c11 100644 --- a/packages/cli/src/validate.ts +++ b/packages/cli/src/validate.ts @@ -114,6 +114,12 @@ export async function validate({ event.sourcePath, ); break; + case "unregistered-module": + console.log( + picocolors.yellow("⚠"), + `/${event.file} is not registered in val.modules - skipping`, + ); + break; case "fix-applied": console.log( picocolors.yellow("⚠"), diff --git a/packages/react/src/internal/index.ts b/packages/react/src/internal/index.ts index 1a1d051fb..c7d7ba5c8 100644 --- a/packages/react/src/internal/index.ts +++ b/packages/react/src/internal/index.ts @@ -1,2 +1,2 @@ -// NOTE: the exports of this file needs to be kept in sync with ValQuickJSRuntime +// NOTE: the exports of this file needs to be kept in sync with the stubs in loadValModules export { ValRichText } from "./ValRichText"; diff --git a/packages/react/src/stega/index.ts b/packages/react/src/stega/index.ts index 5ae7858f8..4eed78de6 100644 --- a/packages/react/src/stega/index.ts +++ b/packages/react/src/stega/index.ts @@ -1,4 +1,4 @@ -// NOTE: the exports of this file needs to be kept in sync with ValQuickJSRuntime +// NOTE: the exports of this file needs to be kept in sync with the stubs in loadValModules export { autoTagJSX } from "./autoTagJSX"; export { stegaEncode, diff --git a/packages/server/package.json b/packages/server/package.json index 81f5e29d2..59a291060 100644 --- a/packages/server/package.json +++ b/packages/server/package.json @@ -33,7 +33,6 @@ "chokidar": "^5.0.0", "image-size": "^2.0.2", "minimatch": "^10.1.1", - "quickjs-emscripten": "^0.21.1", "sucrase": "^3.35.1", "typescript": "^5.9.3", "zod": "^4.3.5", diff --git a/packages/server/src/Service.ts b/packages/server/src/Service.ts index 9b2c677cd..514b7aa29 100644 --- a/packages/server/src/Service.ts +++ b/packages/server/src/Service.ts @@ -1,9 +1,5 @@ -import { newQuickJSWASMModule, QuickJSRuntime } from "quickjs-emscripten"; import { patchValFile } from "./patchValFile"; -import { readValFile } from "./readValFile"; import { Patch } from "@valbuild/core/patch"; -import { ValModuleLoader } from "./ValModuleLoader"; -import { newValQuickJSRuntime } from "./ValQuickJSRuntime"; import { ValSourceFileHandler } from "./ValSourceFileHandler"; import ts from "typescript"; import { getCompilerOptions } from "./getCompilerOptions"; @@ -16,8 +12,13 @@ import { Internal, SourcePath, Schema, + SelectorSource, + SerializedSchema, + Source, + extractValModules, } from "@valbuild/core"; import path from "path"; +import { loadValModules } from "./loadValModules"; export type ServiceOptions = { /** @@ -50,7 +51,6 @@ export async function createService( } }, }, - loader?: ValModuleLoader, ): Promise { const compilerOptions = getCompilerOptions(projectRoot, host); const sourceFileHandler = new ValSourceFileHandler( @@ -58,81 +58,98 @@ export async function createService( compilerOptions, host, ); - const module = await newQuickJSWASMModule(); - const runtime = await newValQuickJSRuntime( - module, - loader || - new ValModuleLoader( - projectRoot, - compilerOptions, - sourceFileHandler, - host, - opts.disableCache === undefined - ? process.env.NODE_ENV === "development" - ? false - : true - : opts.disableCache, - ), - ); - return new Service(projectRoot, sourceFileHandler, runtime); + const valModules = loadValModules(projectRoot); + const extracted = await extractValModules(valModules); + return new Service(projectRoot, sourceFileHandler, extracted); } +type ExtractedModules = Awaited>; + export class Service { readonly projectRoot: string; constructor( projectRoot: string, readonly sourceFileHandler: ValSourceFileHandler, - private readonly runtime: QuickJSRuntime, + private readonly extracted: ExtractedModules, ) { this.projectRoot = projectRoot; } + /** + * The module file paths that are registered in the project's val.modules. + */ + getModuleFilePaths(): ModuleFilePath[] { + return Object.keys(this.extracted.sources) as ModuleFilePath[]; + } + async get( moduleFilePath: ModuleFilePath, modulePath: ModulePath, options?: { validate: boolean; source: boolean; schema: boolean }, ): Promise { - const valModule = await readValFile( - moduleFilePath, - this.projectRoot, - this.runtime, - options ?? { validate: true, source: true, schema: true }, + const opts = options ?? { validate: true, source: true, schema: true }; + const source = this.extracted.sources[moduleFilePath] as Source | undefined; + const schema = this.extracted.schemas[moduleFilePath] as + | Schema + | undefined; + const serializedSchema = this.extracted.serializedSchemas[ + moduleFilePath + ] as SerializedSchema | undefined; + + const moduleError = this.extracted.moduleErrors.find( + (e) => e.path === moduleFilePath, ); - if (valModule.source && valModule.schema) { - const resolved = Internal.resolvePath( - modulePath, - valModule.source, - valModule.schema, - ); - const sourcePath = ( - resolved.path - ? [moduleFilePath, resolved.path].join(".") - : moduleFilePath - ) as SourcePath; + if ( + source === undefined || + schema === undefined || + serializedSchema === undefined + ) { + return { + path: moduleFilePath as string as SourcePath, + errors: { + invalidModulePath: moduleFilePath, + fatal: [ + { + message: + moduleError?.message ?? + `Module '${moduleFilePath}' was not found in val.modules`, + }, + ], + }, + }; + } + + const validation = opts.validate + ? schema["executeValidate"]( + moduleFilePath as string as SourcePath, + source as SelectorSource, + ) + : false; + + const resolved = Internal.resolvePath(modulePath, source, serializedSchema); + const sourcePath = ( + resolved.path ? [moduleFilePath, resolved.path].join(".") : moduleFilePath + ) as SourcePath; + + if (!validation && !moduleError) { return { path: sourcePath, - schema: - resolved.schema instanceof Schema - ? resolved.schema["executeSerialize"]() - : resolved.schema, source: resolved.source, - errors: - valModule.errors && valModule.errors.validation - ? { - validation: valModule.errors.validation || undefined, - fatal: valModule.errors.fatal || undefined, - } - : valModule.errors - ? { - fatal: valModule.errors.fatal || undefined, - } - : false, + schema: resolved.schema, + errors: false, }; - } else { - return valModule; } + return { + path: sourcePath, + source: resolved.source, + schema: resolved.schema, + errors: { + validation: validation || undefined, + fatal: moduleError ? [{ message: moduleError.message }] : undefined, + }, + }; } async patch(moduleFilePath: ModuleFilePath, patch: Patch): Promise { @@ -141,11 +158,10 @@ export class Service { this.projectRoot, patch, this.sourceFileHandler, - this.runtime, ); } dispose() { - this.runtime.dispose(); + // No-op: the vm-based loader holds no disposable resources. } } diff --git a/packages/server/src/ValQuickJSRuntime.ts b/packages/server/src/ValQuickJSRuntime.ts deleted file mode 100644 index c85ae3e91..000000000 --- a/packages/server/src/ValQuickJSRuntime.ts +++ /dev/null @@ -1,191 +0,0 @@ -import { JSModuleNormalizeResult, QuickJSWASMModule } from "quickjs-emscripten"; -import { ValModuleLoader } from "./ValModuleLoader"; -import { VAL_APP_ID, VAL_CSS_PATH, VAL_OVERLAY_ID } from "@valbuild/ui"; - -export async function newValQuickJSRuntime( - quickJSModule: Pick, - moduleLoader: ValModuleLoader, - { - maxStackSize = 1024 * 20, // maximum stack size that works: 1024 * 640 * 8 - memoryLimit = 1024 * 640, // 640 mbs - }: { - maxStackSize?: number; - memoryLimit?: number; - } = {}, -) { - const runtime = quickJSModule.newRuntime(); - - runtime.setMaxStackSize(maxStackSize); - runtime.setMemoryLimit(memoryLimit); - - runtime.setModuleLoader( - (modulePath) => { - try { - // Special cases to avoid loading the React packages since currently React does not have a ESM build: - // TODO: this is not stable, find a better way to do this - if (modulePath === "@valbuild/react") { - return { - value: - "export const useVal = () => { throw Error(`Cannot use 'useVal' in this type of file`) }; export function ValProvider() { throw Error(`Cannot use 'ValProvider' in this type of file`) }; export function ValRichText() { throw Error(`Cannot use 'ValRichText' in this type of file`)};", - }; - } - if (modulePath === "@valbuild/react/internal") { - return { - value: ` -const useVal = () => { throw Error('Cannot use \\'useVal\\' in this type of file') }; -export function ValProvider() { throw Error('Cannot use \\'ValProvider\\' in this type of file') }; -export function ValRichText() { throw Error('Cannot use \\'ValRichText\\' in this type of file')};`, - }; - } - if (modulePath === "@valbuild/ui") { - return { - value: ` -export const ValOverlay = () => { - throw Error("Cannot use 'ValOverlay' in this type of file") -}; -export const VAL_CSS_PATH = "${VAL_CSS_PATH}"; -export const VAL_APP_PATH = "${VAL_CSS_PATH}"; -export const VAL_APP_ID = "${VAL_APP_ID}"; -export const VAL_OVERLAY_ID = "${VAL_OVERLAY_ID}"; -export const IS_DEV = false; -export const VERSION = "0.0.0"; -`, - }; - } - if (modulePath === "@valbuild/react/stega") { - return { - value: - "export const useVal = () => { throw Error(`Cannot use 'useVal' in this type of file`) };export const fetchVal = () => { throw Error(`Cannot use 'fetchVal' in this type of file`) }; export const autoTagJSX = () => { /* ignore */ }; export const stegaClean = () => { throw Error(`Cannot use 'stegaClean' in this type of file`) }; export const stegaDecodeStrings = () => { throw Error(`Cannot use 'stegaDecodeStrings' in this type of file`) }; export const stegaEncode = () => { throw Error(`Cannot use 'stegaEncode' in this type of file`) }; export const raw = () => { throw Error(`Cannot use 'raw' in this type of file`) }; export const attrs = () => { throw Error(`Cannot use 'attrs' in this type of file`) }; ", - }; - } - if (modulePath.startsWith("next/navigation")) { - return { - value: - "export const usePathname = () => { throw Error(`Cannot use 'usePathname' in this type of file`) }; export const useRouter = () => { throw Error(`Cannot use 'useRouter' in this type of file`) }; export default new Proxy({}, { get() { return () => { throw new Error(`Cannot import 'next' in this file`) } } } );", - }; - } - if (modulePath.startsWith("next")) { - return { - value: - "export default new Proxy({}, { get() { return () => { throw new Error(`Cannot import 'next' in this file`) } } } );", - }; - } - if (modulePath.startsWith("react/jsx-runtime")) { - return { - value: - "export const jsx = () => { throw Error(`Cannot use 'jsx' in this type of file`) }; export const Fragment = () => { throw Error(`Cannot use 'Fragment' in this type of file`) }; export default new Proxy({}, { get() { return () => { throw new Error(`Cannot import 'react' in this file`) } } } ); export const jsxs = () => { throw Error(`Cannot use 'jsxs' in this type of file`) };", - }; - } - if (modulePath.startsWith("react")) { - return { - value: ` -export const createContext = () => new Proxy({}, { get() { return () => { throw new Error('Cannot use \\'createContext\\' in this file') } } } ); -export const useTransition = () => { throw Error('Cannot use \\'useTransition\\' in this type of file') }; - -export default new Proxy({}, { - get(target, props) { - // React.createContext might be called on top-level - if (props === 'createContext') { - return createContext; - } - return () => { - throw new Error('Cannot import \\'react\\' in this file'); - } - } -})`, - }; - } - if (modulePath.includes("/ValNextProvider")) { - return { - value: - "export const ValNextProvider = new Proxy({}, { get() { return () => { throw new Error(`Cannot import 'ValNextProvider' in this file`) } } } )", - }; - } - if (modulePath.includes("/ValContext")) { - return { - value: - "export const useValEvents = () => { throw Error(`Cannot use 'useValEvents' in this type of file`) }; export const ValContext = new Proxy({}, { get() { return () => { throw new Error(`Cannot import 'ValContext' in this file`) } } } ) export const ValEvents = new Proxy({}, { get() { return () => { throw new Error(`Cannot import 'ValEvents' in this file`) } } } )", - }; - } - if (modulePath.includes("/ValImage")) { - return { - value: - "export const ValImage = new Proxy({}, { get() { return () => { throw new Error(`Cannot import 'ValImage' in this file`) } } } )", - }; - } - if (modulePath.includes("/ValApp")) { - return { - value: - "export const ValApp = new Proxy({}, { get() { return () => { throw new Error(`Cannot import 'ValApp' in this file`) } } } )", - }; - } - if (modulePath.includes("/ValModulesClient")) { - return { - value: - "export const ValModulesClient = new Proxy({}, { get() { return () => { throw new Error(`Cannot import 'ValModulesClient' in this file`) } } } ); export const useRegisterValModules = () => { throw new Error(`Cannot use 'useRegisterValModules' in this type of file`) };", - }; - } - return { value: moduleLoader.getModule(modulePath) }; - } catch { - return { - error: Error(`Could not resolve module: '${modulePath}'`), - }; - } - }, - (baseModuleName, requestedName): JSModuleNormalizeResult => { - try { - if (requestedName === "@valbuild/react") { - return { value: requestedName }; - } - if (requestedName === "@valbuild/react/stega") { - return { value: requestedName }; - } - if (requestedName === "@valbuild/react/internal") { - return { value: requestedName }; - } - if (requestedName === "@valbuild/ui") { - return { value: requestedName }; - } - if (requestedName.startsWith("next/navigation")) { - return { value: requestedName }; - } - if (requestedName.startsWith("next")) { - return { value: requestedName }; - } - if (requestedName.startsWith("react/jsx-runtime")) { - return { value: requestedName }; - } - if (requestedName.startsWith("react")) { - return { value: requestedName }; - } - if (requestedName.includes("/ValNextProvider")) { - return { value: requestedName }; - } - if (requestedName.includes("/ValContext")) { - return { value: requestedName }; - } - if (requestedName.includes("/ValImage")) { - return { value: requestedName }; - } - if (requestedName.includes("/ValApp")) { - return { value: requestedName }; - } - if (requestedName.includes("/ValModulesClient")) { - return { value: requestedName }; - } - const modulePath = moduleLoader.resolveModulePath( - baseModuleName, - requestedName, - ); - return { value: modulePath }; - } catch (e) { - console.debug( - `Could not resolve ${requestedName} in ${baseModuleName}`, - e, - ); - return { value: requestedName }; - } - }, - ); - return runtime; -} diff --git a/packages/server/src/loadValModules.ts b/packages/server/src/loadValModules.ts new file mode 100644 index 000000000..64c3e0a33 --- /dev/null +++ b/packages/server/src/loadValModules.ts @@ -0,0 +1,196 @@ +import path from "path"; +import fs from "fs"; +import vm from "node:vm"; +import { createRequire } from "node:module"; +import ts from "typescript"; +import type { ValModules } from "@valbuild/core"; +import { getCompilerOptions } from "./getCompilerOptions"; + +/** + * Loads the project's root `val.modules.ts` (or `.js`) using Node's `vm` + * module and returns its default export (a `ValModules` registry). + * + * This is a recursive CommonJS loader: the root modules file and every + * relative `*.val.ts` / `val.config.ts` it (dynamically) imports are + * transpiled to CommonJS and evaluated in a `vm` sandbox. Bare specifiers + * (e.g. `@valbuild/core`) are resolved with the real Node `require` so the + * user modules share the exact same `@valbuild/core` instance that + * `extractValModules` uses. + * + * Mirrors the pattern already used by the CLI's `evalValConfigFile`. + */ +export function loadValModules(projectRoot: string): ValModules { + const valModulesPath = findValModulesPath(projectRoot); + if (!valModulesPath) { + throw Error( + `Could not find 'val.modules.ts' nor 'val.modules.js' in project root: '${projectRoot}'`, + ); + } + const compilerOptions = getCompilerOptions(projectRoot, ts.sys); + const cache: Record }> = {}; + const loaded = loadModule(valModulesPath, cache, compilerOptions); + const valModules = loaded.exports.default; + if (!valModules) { + throw Error( + `Val modules file at path: '${valModulesPath}' must have a default export. Got: ${valModules}`, + ); + } + return valModules as ValModules; +} + +function findValModulesPath(projectRoot: string): string | null { + for (const fileName of ["val.modules.ts", "val.modules.js"]) { + const candidate = path.join(projectRoot, fileName); + if (fs.existsSync(candidate)) { + return candidate; + } + } + return null; +} + +const RESOLVE_EXTENSIONS = [".ts", ".tsx", ".js", ".jsx", ".cjs", ".mjs"]; + +// Specifiers that user val files must not actually use. We stub them so that +// importing is fine, but using a value throws a clear error. Real @valbuild +// packages are resolved via the real require, so when they (legitimately) +// import react/next internally those go through Node, not this stub. +function isStubbedSpecifier(spec: string): boolean { + return ( + spec === "react" || + spec.startsWith("react/") || + spec === "next" || + spec.startsWith("next/") || + spec === "@valbuild/ui" || + spec === "@valbuild/react" || + spec.startsWith("@valbuild/react/") + ); +} + +function makeStub(spec: string): Record { + const throwing = (prop: string) => () => { + throw Error(`Cannot use '${prop}' from '${spec}' in this type of file`); + }; + const handler: ProxyHandler> = { + get(_target, prop) { + if (prop === "__esModule") { + return true; + } + if (typeof prop === "symbol") { + return undefined; + } + // React.createContext is sometimes called at module top-level; return a + // proxy-returning function so evaluation does not crash on import. + if (prop === "createContext") { + return () => new Proxy({}, handler); + } + if (prop === "default") { + return stub; + } + return throwing(prop); + }, + }; + const stub = new Proxy({}, handler); + return stub; +} + +function loadModule( + absPath: string, + cache: Record }>, + compilerOptions: ts.CompilerOptions, +): { exports: Record } { + const cached = cache[absPath]; + if (cached) { + return cached; + } + const code = fs.readFileSync(absPath, "utf-8"); + const transpiled = ts.transpileModule(code, { + compilerOptions: { + target: ts.ScriptTarget.ES2020, + module: ts.ModuleKind.CommonJS, + esModuleInterop: true, + jsx: ts.JsxEmit.ReactJSX, + }, + fileName: absPath, + }); + + const moduleObj: { exports: Record } = { exports: {} }; + // Insert into the cache before evaluating so cyclic imports resolve. + cache[absPath] = moduleObj; + + const dirName = path.dirname(absPath); + const realRequire = createRequire(absPath); + const customRequire = (spec: string): unknown => { + if (isStubbedSpecifier(spec)) { + return makeStub(spec); + } + if (spec.startsWith(".") || path.isAbsolute(spec)) { + const resolved = resolveRelative(dirName, spec); + if (!resolved) { + throw Error(`Could not resolve module '${spec}' from '${absPath}'`); + } + return loadModule(resolved, cache, compilerOptions).exports; + } + // Non-relative specifier: it might be a tsconfig path alias (e.g. "_/val.config") + // pointing at a local source file, or an actual node_modules package. + const tsResolved = ts.resolveModuleName( + spec, + absPath, + compilerOptions, + ts.sys, + ).resolvedModule?.resolvedFileName; + if ( + tsResolved && + !tsResolved.includes("/node_modules/") && + !tsResolved.endsWith(".d.ts") + ) { + return loadModule(tsResolved, cache, compilerOptions).exports; + } + // Real node_modules package – use the real require so user modules share + // the same @valbuild/core instance as extractValModules. + return realRequire(spec); + }; + + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const sandbox: Record = { + exports: moduleObj.exports, + module: moduleObj, + require: customRequire, + __filename: absPath, + __dirname: dirName, + console, + process, + }; + sandbox.global = sandbox; + sandbox.globalThis = sandbox; + + const context = vm.createContext(sandbox); + const script = new vm.Script(transpiled.outputText, { filename: absPath }); + script.runInContext(context); + + return moduleObj; +} + +function resolveRelative(dirName: string, spec: string): string | null { + const base = path.resolve(dirName, spec); + // Exact file (with extension) + if (fs.existsSync(base) && fs.statSync(base).isFile()) { + return base; + } + // Probe extensions (handles `./x.val` -> `./x.val.ts`) + for (const ext of RESOLVE_EXTENSIONS) { + const candidate = base + ext; + if (fs.existsSync(candidate)) { + return candidate; + } + } + // Directory index + if (fs.existsSync(base) && fs.statSync(base).isDirectory()) { + for (const ext of RESOLVE_EXTENSIONS) { + const candidate = path.join(base, "index" + ext); + if (fs.existsSync(candidate)) { + return candidate; + } + } + } + return null; +} diff --git a/packages/server/src/patchValFile.ts b/packages/server/src/patchValFile.ts index 986424179..bfd958efb 100644 --- a/packages/server/src/patchValFile.ts +++ b/packages/server/src/patchValFile.ts @@ -9,7 +9,6 @@ import { } from "./patch/ts/syntax"; import { ValSourceFileHandler } from "./ValSourceFileHandler"; import { derefPatch } from "@valbuild/core"; -import { QuickJSRuntime } from "quickjs-emscripten"; import ts from "typescript"; import { getSyntheticContainingPath } from "./getSyntheticContainingPath"; @@ -26,8 +25,6 @@ export const patchValFile = async ( rootDir: string, patch: Patch, sourceFileHandler: ValSourceFileHandler, - // eslint-disable-next-line @typescript-eslint/no-unused-vars - runtime: QuickJSRuntime, ): Promise => { // const timeId = randomUUID(); // console.time("patchValFile" + timeId); diff --git a/packages/server/src/readValFile.test.ts b/packages/server/src/readValFile.test.ts deleted file mode 100644 index 53fe21772..000000000 --- a/packages/server/src/readValFile.test.ts +++ /dev/null @@ -1,59 +0,0 @@ -import { - TestQuickJSWASMModule, - newQuickJSAsyncWASMModule, -} from "quickjs-emscripten"; -import { readValFile } from "./readValFile"; -import path from "path"; -import { createModuleLoader } from "./ValModuleLoader"; -import { newValQuickJSRuntime } from "./ValQuickJSRuntime"; -import { ModuleFilePath } from "@valbuild/core"; - -const TestCaseDir = "../test/example-projects"; -const TestCases = [ - { name: "basic-next-typescript", ext: ".ts" }, - - { - name: "basic-next-src-typescript", - prefix: "/src", - ext: ".ts", - }, - { name: "basic-next-javascript", ext: ".js" }, - { name: "typescript-description-files", ext: ".ts" }, -]; - -describe("read val file", () => { - // We cannot, currently use TestQuickJSWASMModule - let QuickJS: TestQuickJSWASMModule; - - beforeEach(async () => { - QuickJS = new TestQuickJSWASMModule(await newQuickJSAsyncWASMModule()); - }); - - afterEach(() => { - QuickJS.disposeAll(); - QuickJS.assertNoMemoryAllocated(); - }); - - test.each(TestCases)("read basic val file from: $name", async (testCase) => { - const rootDir = path.resolve(__dirname, TestCaseDir, testCase.name); - const loader = createModuleLoader(rootDir); - const testRuntime = await newValQuickJSRuntime(QuickJS, loader, { - maxStackSize: 1024 * 640, - memoryLimit: 1024 * 640, - }); - const result = await readValFile( - ((testCase.prefix ? testCase.prefix : "") + - "/pages/blogs.val" + - testCase.ext) as ModuleFilePath, - rootDir, - testRuntime, - { - schema: true, - source: true, - validate: true, - }, - ); - expect(result).toHaveProperty("source"); - expect(result).toHaveProperty("schema"); - }); -}); diff --git a/packages/server/src/readValFile.ts b/packages/server/src/readValFile.ts deleted file mode 100644 index f55dd356a..000000000 --- a/packages/server/src/readValFile.ts +++ /dev/null @@ -1,158 +0,0 @@ -import { ModuleFilePath, SourcePath } from "@valbuild/core"; -import { QuickJSRuntime } from "quickjs-emscripten"; -import { SerializedModuleContent } from "./SerializedModuleContent"; -import { getSyntheticContainingPath } from "./getSyntheticContainingPath"; - -export const readValFile = async ( - moduleFilePath: ModuleFilePath, - rootDirPath: string, - runtime: QuickJSRuntime, - options: { validate: boolean; source: boolean; schema: boolean }, -): Promise => { - const context = runtime.newContext(); - - // avoid failures when console.log is called - const logHandle = context.newFunction("log", () => { - // do nothing - }); - const consoleHandle = context.newObject(); - context.setProp(consoleHandle, "log", logHandle); - context.setProp(context.global, "console", consoleHandle); - - consoleHandle.dispose(); - logHandle.dispose(); - - // avoid failures when process.env is called - const envHandle = context.newObject(); - const processHandle = context.newObject(); - context.setProp(processHandle, "env", envHandle); - context.setProp(context.global, "process", processHandle); - - const optionsHandle = context.newObject(); - if (options) { - if (options.validate !== undefined) { - context.setProp( - optionsHandle, - "validate", - context.newNumber(+options.validate), - ); - } - if (options.source !== undefined) { - context.setProp( - optionsHandle, - "source", - context.newNumber(+options.source), - ); - } - if (options.schema !== undefined) { - context.setProp( - optionsHandle, - "schema", - context.newNumber(+options.schema), - ); - } - } - context.setProp(context.global, "__VAL_OPTIONS__", optionsHandle); - - envHandle.dispose(); - processHandle.dispose(); - optionsHandle.dispose(); - - try { - const modulePath = `.${moduleFilePath - .replace(".val.js", ".val") - .replace(".val.ts", ".val") - .replace(".val.tsx", ".val") - .replace(".val.jsx", ".val")}`; - const code = `import * as valModule from ${JSON.stringify(modulePath)}; -import { Internal } from "@valbuild/core"; - -globalThis.valModule = { - path: valModule?.default && Internal.getValPath(valModule?.default), - schema: !!globalThis['__VAL_OPTIONS__'].schema ? valModule?.default && Internal.getSchema(valModule?.default)?.["executeSerialize"]() : undefined, - source: !!globalThis['__VAL_OPTIONS__'].source ? valModule?.default && Internal.getSource(valModule?.default) : undefined, - validation: !!globalThis['__VAL_OPTIONS__'].validate ? valModule?.default && (Internal.validate ? Internal.validate(valModule.default, Internal.getValPath(valModule?.default) || "/", - Internal.getSource(valModule?.default)) : Internal.getSchema(valModule?.default)?.validate( - Internal.getValPath(valModule?.default) || "/", - Internal.getSource(valModule?.default) - )) : undefined, - defaultExport: !!valModule?.default, -}; -`; - const result = context.evalCode( - code, - getSyntheticContainingPath(rootDirPath), - ); - const fatalErrors: string[] = []; - if (result.error) { - const error = result.error.consume(context.dump); - console.error( - `Fatal error reading val file: ${moduleFilePath}. Error: ${error.message}\n`, - error.stack, - ); - return { - path: moduleFilePath as string as SourcePath, - errors: { - invalidModulePath: moduleFilePath as ModuleFilePath, - fatal: [ - { - message: `${error.name || "Unknown error"}: ${ - error.message || "" - }`, - stack: error.stack, - }, - ], - }, - }; - } else { - result.value.dispose(); - const valModule = context - .getProp(context.global, "valModule") - .consume(context.dump); - if ( - // if one of these are set it is a Val module, so must validate - valModule?.path !== undefined || - valModule?.schema !== undefined || - valModule?.source !== undefined - ) { - if (valModule.path !== moduleFilePath) { - fatalErrors.push( - `Wrong c.define path! Expected: '${moduleFilePath}', found: '${valModule.path}'`, - ); - } else if (valModule?.schema === undefined && options.schema) { - fatalErrors.push( - `Expected val path: '${moduleFilePath}' to have a schema`, - ); - } else if (valModule?.source === undefined && options.source) { - fatalErrors.push( - `Expected val path: '${moduleFilePath}' to have a source`, - ); - } - } - let errors: SerializedModuleContent["errors"] = false; - if (fatalErrors.length > 0) { - errors = { - invalidModulePath: - valModule.path !== moduleFilePath - ? (moduleFilePath as ModuleFilePath) - : undefined, - fatal: fatalErrors.map((message) => ({ message })), - }; - } - if (valModule?.validation) { - errors = { - ...(errors ? errors : {}), - validation: valModule.validation, - }; - } - return { - path: valModule.path || moduleFilePath, // NOTE: we use path here, since SerializedModuleContent (maybe bad name?) can be used for whole modules as well as subparts of modules - source: valModule.source, - schema: valModule.schema, - errors, - }; - } - } finally { - context.dispose(); - } -}; diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 1355c69b5..7df2b6904 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -409,9 +409,6 @@ importers: minimatch: specifier: ^10.1.1 version: 10.1.1 - quickjs-emscripten: - specifier: ^0.21.1 - version: 0.21.2 sucrase: specifier: ^3.35.1 version: 3.35.1 @@ -8603,9 +8600,6 @@ packages: resolution: {integrity: sha512-WuyALRjWPDGtt/wzJiadO5AXY+8hZ80hVpe6MyivgraREW751X3SbhRvG3eLKOYN+8VEvqLcf3wdnt44Z4S4SA==} engines: {node: '>=10'} - quickjs-emscripten@0.21.2: - resolution: {integrity: sha512-fTujYfS1jmNUi4A5Tlr9SkeEQoFH5aVEXKbs/PV8t/MZ3DLb7B4fnlM7ESd/K/o5UGNnNECVgossKdEtS8fmFg==} - ramda@0.29.0: resolution: {integrity: sha512-BBea6L67bYLtdbOqfp8f58fPMqEwx0doL+pAi8TZyp2YWz8R9G8z9x75CZI8W+ftqhFHCpEX2cRnUUXK130iKA==} @@ -9383,7 +9377,7 @@ packages: tar@6.2.1: resolution: {integrity: sha512-DZ4yORTwrbTj/7MZYq2w+/ZFdI6OZ/f9SFHR+71gIVUZhOQPHzVCLpvRnPgyaMpfWxxk/4ONva3GQSyNIKRv6A==} engines: {node: '>=10'} - deprecated: Old versions of tar are not supported, and contain widely publicized security vulnerabilities, which have been fixed in the current version. Please update. Support for old versions may be purchased (at exorbitant rates) by contacting i@izs.me + deprecated: Old versions of tar are not supported, and contain widely publicized security vulnerabilities, which have been fixed in the current version. Please update. Support for old versions may be purchased (at exhorbitant rates) by contacting i@izs.me telejson@7.2.0: resolution: {integrity: sha512-1QTEcJkJEhc8OnStBx/ILRu5J2p0GjvWsBx56bmZRqnrkdBMUe+nX92jxV+p3dB4CP6PZCdJMQJwCggkNBMzkQ==} @@ -19871,8 +19865,6 @@ snapshots: quick-lru@5.1.1: {} - quickjs-emscripten@0.21.2: {} - ramda@0.29.0: {} randombytes@2.1.0: From aa011c73fd730b503d98101c89e41c9af089dd61 Mon Sep 17 00:00:00 2001 From: Fredrik Ekholdt Date: Wed, 24 Jun 2026 21:42:13 +0200 Subject: [PATCH 06/16] Fix so a top-level s.image() type checks as a ValModule --- packages/core/src/selector/file.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/core/src/selector/file.ts b/packages/core/src/selector/file.ts index 513e4013a..5a134eeef 100644 --- a/packages/core/src/selector/file.ts +++ b/packages/core/src/selector/file.ts @@ -9,8 +9,8 @@ export type FileSelector = url: string; }> & { readonly url: UnknownSelector; - } & Metadata extends undefined - ? {} - : Metadata extends Source - ? { readonly metadata: UnknownSelector } - : {}; + } & (Metadata extends undefined + ? {} + : Metadata extends Source + ? { readonly metadata: UnknownSelector } + : {}); From 800e6c44062ed8e1bd0f6d1e67bc7050131ceb47 Mon Sep 17 00:00:00 2001 From: Fredrik Ekholdt Date: Wed, 24 Jun 2026 21:55:53 +0200 Subject: [PATCH 07/16] Add code frame (source file mapping) in validation errors for cooler and more practical validation errors --- .../cli/src/utils/sourcePathToFileLocation.ts | 143 ++++++++++ packages/cli/src/validate.ts | 35 ++- packages/server/src/index.ts | 2 + packages/server/src/modulePathMap.test.ts | 200 ++++++++++++++ packages/server/src/modulePathMap.ts | 249 ++++++++++++++++++ 5 files changed, 626 insertions(+), 3 deletions(-) create mode 100644 packages/cli/src/utils/sourcePathToFileLocation.ts create mode 100644 packages/server/src/modulePathMap.test.ts create mode 100644 packages/server/src/modulePathMap.ts diff --git a/packages/cli/src/utils/sourcePathToFileLocation.ts b/packages/cli/src/utils/sourcePathToFileLocation.ts new file mode 100644 index 000000000..b33fa348b --- /dev/null +++ b/packages/cli/src/utils/sourcePathToFileLocation.ts @@ -0,0 +1,143 @@ +import path from "path"; +import nodeFs from "fs"; +import ts from "typescript"; +import picocolors from "picocolors"; +import { Internal, type SourcePath } from "@valbuild/core"; +import { + createModulePathMap, + getModulePathRange, + type ModulePathMap, +} from "@valbuild/server"; + +type CachedFile = { lines: string[]; map: ModulePathMap | undefined }; + +/** + * Per-run cache mapping a moduleFilePath to its parsed source. `null` means the + * file could not be read. Each val file is read and parsed at most once. + */ +export type SourceFileCache = Map; + +type Position = { line: number; character: number }; +type Range = { start: Position; end: Position }; + +/** + * Resolves a validation `sourcePath` (e.g. + * `/content/page.val.ts?p="image"."metadata"`) to a clickable + * `relativePath:line:col` location pointing at the offending source literal in + * the val file. + * + * Falls back to the raw `sourcePath` when the file cannot be read or no range + * can be resolved. + */ +export function sourcePathToFileLocation( + sourcePath: string, + projectRoot: string, + cache: SourceFileCache, +): string { + const resolved = resolveRange(sourcePath, projectRoot, cache); + if (!resolved) { + return sourcePath; + } + // TS line/character are 0-indexed; editors/terminals expect 1-indexed. + const { relativeFile, range } = resolved; + return `${relativeFile}:${range.start.line + 1}:${range.start.character + 1}`; +} + +/** + * Renders a Rust-style code frame for the offending `sourcePath`: the line + * above, the offending line, and the line below, with red carets underlining + * the offending span. Returns `undefined` when the location cannot be resolved + * (the caller should then skip the frame). + */ +export function sourcePathToCodeFrame( + sourcePath: string, + projectRoot: string, + cache: SourceFileCache, +): string | undefined { + const resolved = resolveRange(sourcePath, projectRoot, cache); + if (!resolved) { + return undefined; + } + const { lines, range } = resolved; + const startLine = range.start.line; + const firstLine = Math.max(0, startLine - 1); + const lastLine = Math.min(lines.length - 1, startLine + 1); + const gutterWidth = String(lastLine + 1).length; + + const out: string[] = []; + for (let i = firstLine; i <= lastLine; i++) { + const lineText = lines[i] ?? ""; + const gutter = picocolors.dim( + `${String(i + 1).padStart(gutterWidth, " ")} | `, + ); + out.push(`${gutter}${lineText}`); + if (i === startLine) { + const caretStart = range.start.character; + const sameLine = range.end.line === range.start.line; + const caretEnd = sameLine ? range.end.character : lineText.length; + const caretCount = Math.max(1, caretEnd - caretStart); + const emptyGutter = picocolors.dim(`${" ".repeat(gutterWidth)} | `); + out.push( + `${emptyGutter}${" ".repeat(caretStart)}${picocolors.red( + "^".repeat(caretCount), + )}`, + ); + } + } + return out.join("\n"); +} + +function resolveRange( + sourcePath: string, + projectRoot: string, + cache: SourceFileCache, +): { relativeFile: string; lines: string[]; range: Range } | undefined { + const [moduleFilePath, modulePath] = + Internal.splitModuleFilePathAndModulePath(sourcePath as SourcePath); + + const cached = getCachedFile(moduleFilePath, projectRoot, cache); + if (!cached || !cached.map) { + return undefined; + } + + const range = getModulePathRange(modulePath, cached.map); + if (!range) { + return undefined; + } + + return { + relativeFile: moduleFilePath.replace(/^\//, ""), + lines: cached.lines, + range, + }; +} + +function getCachedFile( + moduleFilePath: string, + projectRoot: string, + cache: SourceFileCache, +): CachedFile | null { + const existing = cache.get(moduleFilePath); + if (existing !== undefined) { + return existing; + } + const filePath = path.join(projectRoot, moduleFilePath); + let fileContent: string; + try { + fileContent = nodeFs.readFileSync(filePath, "utf-8"); + } catch { + cache.set(moduleFilePath, null); + return null; + } + const sourceFile = ts.createSourceFile( + filePath, + fileContent, + ts.ScriptTarget.ES2015, + ); + const entry: CachedFile = { + lines: fileContent.split(/\r?\n/), + map: createModulePathMap(sourceFile), + }; + cache.set(moduleFilePath, entry); + return entry; +} diff --git a/packages/cli/src/validate.ts b/packages/cli/src/validate.ts index e7f425c11..78aa18b1b 100644 --- a/packages/cli/src/validate.ts +++ b/packages/cli/src/validate.ts @@ -6,6 +6,11 @@ import { DEFAULT_CONTENT_HOST, DEFAULT_VAL_REMOTE_HOST } from "@valbuild/core"; import { getSettings, uploadRemoteFile } from "@valbuild/server"; import { evalValConfigFile } from "./utils/evalValConfigFile"; import { createDefaultValFSHost, runValidation } from "./runValidation"; +import { + sourcePathToCodeFrame, + sourcePathToFileLocation, + type SourceFileCache, +} from "./utils/sourcePathToFileLocation"; export async function validate({ root, @@ -52,6 +57,23 @@ export async function validate({ const fixedFiles = new Set(); let totalErrors = 0; + // Caches each val file's parsed source so files are read/parsed at most once + // when resolving sourcePaths to file locations and code frames. + const sourceFileCache: SourceFileCache = new Map(); + + // Prints `file:line:col` followed by a Rust-style code frame (when the + // location can be resolved) for an error at the given sourcePath. + const logSourceLocation = (sourcePath: string) => { + const frame = sourcePathToCodeFrame( + sourcePath, + projectRoot, + sourceFileCache, + ); + if (frame !== undefined) { + console.log("\n" + frame); + } + }; + for await (const event of runValidation({ root: projectRoot, fix: !!fix, @@ -93,17 +115,19 @@ export async function validate({ console.log( picocolors.red("✘"), "Got error in", - `${event.sourcePath}:`, + `${sourcePathToFileLocation(event.sourcePath, projectRoot, sourceFileCache)}:`, event.message, ); + logSourceLocation(event.sourcePath); break; case "validation-fixable-error": console.log( event.fixable ? picocolors.yellow("⚠") : picocolors.red("✘"), `Got ${event.fixable ? "fixable " : ""}error in`, - `${event.sourcePath}:`, + `${sourcePathToFileLocation(event.sourcePath, projectRoot, sourceFileCache)}:`, event.message, ); + logSourceLocation(event.sourcePath); break; case "unknown-fix": console.log( @@ -111,8 +135,13 @@ export async function validate({ "Unknown fix", event.fixes, "for", - event.sourcePath, + sourcePathToFileLocation( + event.sourcePath, + projectRoot, + sourceFileCache, + ), ); + logSourceLocation(event.sourcePath); break; case "unregistered-module": console.log( diff --git a/packages/server/src/index.ts b/packages/server/src/index.ts index 40dac0b09..c47906988 100644 --- a/packages/server/src/index.ts +++ b/packages/server/src/index.ts @@ -18,3 +18,5 @@ export { parsePersonalAccessTokenFile, } from "./personalAccessTokens"; export { uploadRemoteFile } from "./uploadRemoteFile"; +export { createModulePathMap, getModulePathRange } from "./modulePathMap"; +export type { ModulePathMap } from "./modulePathMap"; diff --git a/packages/server/src/modulePathMap.test.ts b/packages/server/src/modulePathMap.test.ts new file mode 100644 index 000000000..cae6890f6 --- /dev/null +++ b/packages/server/src/modulePathMap.test.ts @@ -0,0 +1,200 @@ +import ts from "typescript"; +import { createModulePathMap, getModulePathRange } from "./modulePathMap"; +import assert from "assert"; + +describe("Should map source path to line / cols", () => { + test("test 1", () => { + const text = `import type { InferSchemaType } from '@valbuild/next'; +import { s, c } from '../val.config'; + +const commons = { + keepAspectRatio: s.boolean().optional(), + size: s.union(s.literal('xs'), s.literal('md'), s.literal('lg')).optional(), +}; + +export const schema = s.object({ + text: s.string({ minLength: 10 }), + nested: s.object({ + text: s.string({ minLength: 10 }), + }), + testText: s + .richtext({ + a: true, + bold: true, + headings: ['h1', 'h2', 'h3', 'h4', 'h5', 'h6'], + lineThrough: true, + italic: true, + link: true, + img: true, + ul: true, + ol: true, + }) + .optional(), + testUnion: s.union( + 'type', + s.object({ + ...commons, + type: s.literal('singleImage'), + image: s.image().optional(), + }), + s.object({ + ...commons, + type: s.literal('doubleImage'), + image1: s.image().optional(), + image2: s.image().optional(), + }) + ), +}); +export type TestContent = InferSchemaType; + +export default c.define( + '/oj/test.val.ts', // <- NOTE: this must be the same path as the file + schema, + { + testText: [], + text: 'hei', + nested: { + text: 'hei', + }, + testUnion: { + type: 'singleImage', + keepAspectRatio: true, + size: 'xs', + image: c.image('/public/Screenshot 2023-11-30 at 20.20.11_dbcdb.png'), + }, + } +); +`; + const sourceFile = ts.createSourceFile( + "./oj/test.val.ts", + text, + ts.ScriptTarget.ES2015, + ); + + const modulePathMap = createModulePathMap(sourceFile); + + assert(!!modulePathMap, "modulePathMap is undefined"); + + console.log(getModulePathRange('"text"', modulePathMap)); + assert.deepStrictEqual(getModulePathRange('"text"', modulePathMap), { + end: { character: 6, line: 48 }, + start: { character: 2, line: 48 }, + }); + console.log(getModulePathRange('"nested"."text"', modulePathMap)); + assert.deepStrictEqual( + getModulePathRange('"nested"."text"', modulePathMap), + { end: { character: 8, line: 50 }, start: { character: 4, line: 50 } }, + ); + }); + + test("test 2", () => { + const text = `import { s, c } from '../val.config'; + +const commons = { + keepAspectRatio: s.boolean().optional(), + size: s.union(s.literal('xs'), s.literal('md'), s.literal('lg')).optional(), +}; + +export const schema = s.object({ + ingress: s.string({ maxLength: 1 }), + theme: s.string().raw(), + header: s.string(), + image: s.image(), +}); + +export default c.define('/content/aboutUs.val.ts', schema, { + ingress: + 'Vi elsker å bytestgge digitale tjenester som betyr noe for folk, helt fra bunn av, og helt ferdig. Vi tror på iterative utviklingsprosesser, tverrfaglige team, designdrevet produktutvikling og brukersentrerte designmetoder.', + header: 'SPESIALISTER PÅ DIGITAL PRODUKTUTVIKLING', + image: c.image( + '/public/368032148_1348297689148655_444423253678040057_n_64374.png', + { + width: 1283, + height: 1121, + } + ), +}); +`; + const sourceFile = ts.createSourceFile( + "./oj/test.val.ts", + text, + ts.ScriptTarget.ES2015, + ); + + const modulePathMap = createModulePathMap(sourceFile); + assert(!!modulePathMap, "modulePathMap is undefined"); + + console.log(modulePathMap); + // console.log(getModulePathRange('"ingress"', modulePathMap)); + assert.deepStrictEqual(getModulePathRange('"ingress"', modulePathMap), { + start: { line: 15, character: 2 }, + end: { line: 15, character: 9 }, + }); + }); + + test("test 3", () => { + const text = `import { s, c } from '../val.config'; + +export const schema = s.object({ + first: s.array(s.object({ second: s.record(s.array(s.string()))})) +}); + +export default c.define('/content', schema, { + first: [{ second: { a: ['a', 'b'] } }] +}); +`; + const sourceFile = ts.createSourceFile( + "./content.val.ts", + text, + ts.ScriptTarget.ES2015, + ); + + const modulePathMap = createModulePathMap(sourceFile); + assert(!!modulePathMap, "modulePathMap is undefined"); + + // console.log(getModulePathRange('"first".0."second"."a".1', modulePathMap)); + assert.deepStrictEqual( + getModulePathRange('"first".0."second"."a".1', modulePathMap), + { + start: { line: 7, character: 31 }, + end: { line: 7, character: 34 }, + }, + ); + }); + + test("should handle invalid/malformed module paths gracefully", () => { + const text = `import { s, c } from '../val.config'; + +export const schema = s.object({ + text: s.string(), +}); + +export default c.define('/content', schema, { + text: 'hello' +}); +`; + const sourceFile = ts.createSourceFile( + "./content.val.ts", + text, + ts.ScriptTarget.ES2015, + ); + + const modulePathMap = createModulePathMap(sourceFile); + assert(!!modulePathMap, "modulePathMap is undefined"); + + // These should return undefined instead of throwing + assert.strictEqual(getModulePathRange("", modulePathMap), undefined); + assert.strictEqual(getModulePathRange("invalid", modulePathMap), undefined); + assert.strictEqual(getModulePathRange(".", modulePathMap), undefined); + assert.strictEqual(getModulePathRange("..", modulePathMap), undefined); + assert.strictEqual(getModulePathRange("foo.bar", modulePathMap), undefined); + assert.strictEqual( + getModulePathRange(undefined as unknown as string, modulePathMap), + undefined, + ); + assert.strictEqual( + getModulePathRange(null as unknown as string, modulePathMap), + undefined, + ); + }); +}); diff --git a/packages/server/src/modulePathMap.ts b/packages/server/src/modulePathMap.ts new file mode 100644 index 000000000..5d622a7b1 --- /dev/null +++ b/packages/server/src/modulePathMap.ts @@ -0,0 +1,249 @@ +import ts from "typescript"; +import { Internal, type ModulePath } from "@valbuild/core"; + +export type ModulePathMap = { + [modulePath: string]: { + children: ModulePathMap; + start: { + line: number; + character: number; + }; + end: { + line: number; + character: number; + }; + }; +}; + +export function getModulePathRange( + modulePath: string, + modulePathMap: ModulePathMap, +) { + // Handle empty or invalid module paths gracefully + if (!modulePath || typeof modulePath !== "string") { + return undefined; + } + + let segments: string[]; + try { + // Quote-aware splitter that correctly handles keys containing dots + // (e.g. file refs like `"/public/val/images/logo.png"`), unlike a naive + // split on ".". Throws on malformed input (e.g. unbalanced quotes). + segments = Internal.splitModulePath(modulePath as ModulePath); + } catch { + // Return undefined if the module path is malformed. This can happen when + // there are upstream errors in schema serialization. + return undefined; + } + + if (segments.length === 0) { + return undefined; + } + + let range = modulePathMap[segments[0]]; + for (const pathSegment of segments.slice(1)) { + if (!range) { + break; + } + range = range?.children?.[pathSegment]; + } + return ( + range?.start && + range?.end && { + start: range.start, + end: range.end, + } + ); +} + +export function createModulePathMap( + sourceFile: ts.SourceFile, +): ModulePathMap | undefined { + for (const child of sourceFile + .getChildren() + .flatMap((child) => child.getChildren())) { + if (ts.isExportAssignment(child)) { + const contentNode = + child.expression && + ts.isCallExpression(child.expression) && + child.expression.arguments[2]; + + if (contentNode) { + return traverse(contentNode, sourceFile); + } + } + } +} + +function traverse( + node: ts.Expression, + sourceFile: ts.SourceFile, +): ModulePathMap | undefined { + if (ts.isStringLiteral(node) || ts.isNumericLiteral(node)) { + const tsEnd = sourceFile.getLineAndCharacterOfPosition(node.end); + const start = { + line: tsEnd.line, + character: tsEnd.character - node.getWidth(sourceFile), + }; + const end = { + line: tsEnd.line, + character: tsEnd.character, + }; + return { + "": { + children: {}, + start, + end, + }, + }; + } + if (ts.isObjectLiteralExpression(node)) { + return traverseObjectLiteral(node, sourceFile); + } + if (ts.isArrayLiteralExpression(node)) { + return traverseArrayLiteral(node, sourceFile); + } + if (ts.isCallExpression(node)) { + return traverseCallExpression(node, sourceFile); + } +} + +function traverseCallExpression( + node: ts.CallExpression, + sourceFile: ts.SourceFile, +): ModulePathMap | undefined { + if (ts.isPropertyAccessExpression(node.expression)) { + if ( + node.expression.expression.getText(sourceFile) === "c" && + (node.expression.name.getText(sourceFile) === "file" || + node.expression.name.getText(sourceFile) === "image") + ) { + const val = { + children: {}, + start: sourceFile.getLineAndCharacterOfPosition( + node.getStart(sourceFile), + ), // TODO: We do + 1 to line up the diagnostics error exactly below a normal + end: sourceFile.getLineAndCharacterOfPosition(node.getEnd()), + }; + if (node.arguments[0]) { + const firstArgEnd = sourceFile.getLineAndCharacterOfPosition( + node.arguments[0].end, + ); + const _ref = { + children: {}, + start: { + line: firstArgEnd.line, + character: + firstArgEnd.character - node.arguments[0].getWidth(sourceFile), + }, + end: { + line: firstArgEnd.line, + character: firstArgEnd.character, + }, + }; + if (!node.arguments[1]) { + return { + val, + _ref, + }; + } + const metadataEnd = sourceFile.getLineAndCharacterOfPosition( + node.arguments[1].end, + ); + return { + val, + _ref, + metadata: { + children: {}, + start: { + line: metadataEnd.line, + character: + metadataEnd.character - node.arguments[1].getWidth(sourceFile), + }, + end: { + line: metadataEnd.line, + character: metadataEnd.character, + }, + }, + }; + } + } + } +} + +function traverseArrayLiteral( + node: ts.ArrayLiteralExpression, + sourceFile: ts.SourceFile, +): ModulePathMap { + return node.elements.reduce((acc, element, index) => { + if (ts.isExpression(element)) { + const tsEnd = sourceFile.getLineAndCharacterOfPosition(element.end); + const start = { + line: tsEnd.line, + character: tsEnd.character - element.getWidth(sourceFile), + }; + const end = { + line: tsEnd.line, + character: tsEnd.character, + }; + return { + ...acc, + [index]: { + children: traverse(element, sourceFile), + start, + end, + }, + }; + } + return acc; + }, {}); +} + +function traverseObjectLiteral( + node: ts.ObjectLiteralExpression, + sourceFile: ts.SourceFile, +): ModulePathMap { + return node.properties.reduce((acc, property) => { + if (ts.isPropertyAssignment(property)) { + const key = + property.name && + (ts.isIdentifier(property.name) || ts.isStringLiteral(property.name)) && + property.name.text; + const value = property.initializer; + if (key) { + const tsEnd = sourceFile.getLineAndCharacterOfPosition( + property.name.getEnd(), + ); + const start = { + line: tsEnd.line, + character: tsEnd.character - property.name.getWidth(sourceFile), + }; + const end = { + line: tsEnd.line, + character: tsEnd.character, + }; + const val = { + children: {}, + start: sourceFile.getLineAndCharacterOfPosition( + property.initializer.getStart(sourceFile), + ), + end: sourceFile.getLineAndCharacterOfPosition( + property.initializer.getEnd(), + ), + }; + return { + ...acc, + [key]: { + children: { + val, + ...traverse(value, sourceFile), + }, + start, + end, + }, + }; + } + } + return acc; + }, {}); +} From e298eed4a8b6562d491c5c8f5731b247653e2e8a Mon Sep 17 00:00:00 2001 From: Fredrik Ekholdt Date: Wed, 24 Jun 2026 22:07:43 +0200 Subject: [PATCH 08/16] Show code frame validation errors on key or value depending on the validation error --- packages/cli/src/runValidation.ts | 22 +++++++++- .../cli/src/utils/sourcePathToFileLocation.ts | 9 +++-- packages/cli/src/validate.ts | 40 +++++++++++++------ packages/server/src/modulePathMap.test.ts | 11 +++++ packages/server/src/modulePathMap.ts | 22 ++++++++-- 5 files changed, 82 insertions(+), 22 deletions(-) diff --git a/packages/cli/src/runValidation.ts b/packages/cli/src/runValidation.ts index 4016546ed..e865ab899 100644 --- a/packages/cli/src/runValidation.ts +++ b/packages/cli/src/runValidation.ts @@ -62,6 +62,8 @@ export type ValidationError = { message: string; value?: unknown; fixes?: ValidationFix[]; + // True when the error is about an object/record key rather than its value. + keyError?: boolean; }; export type FixHandlerContext = { @@ -109,14 +111,25 @@ export type ValidationEvent = errorCount: number; durationMs: number; } - | { type: "validation-error"; sourcePath: string; message: string } + | { + type: "validation-error"; + sourcePath: string; + message: string; + keyError?: boolean; + } | { type: "validation-fixable-error"; sourcePath: string; message: string; fixable: boolean; + keyError?: boolean; + } + | { + type: "unknown-fix"; + sourcePath: string; + fixes: string[]; + keyError?: boolean; } - | { type: "unknown-fix"; sourcePath: string; fixes: string[] } | { type: "unregistered-module"; file: string } | { type: "fix-applied"; file: string; sourcePath: string } | { type: "fatal-error"; file: string; message: string } @@ -679,6 +692,7 @@ export async function* runValidation({ type: "validation-error", sourcePath, message: v.message, + ...(v.keyError ? { keyError: true } : {}), }; continue; } @@ -692,6 +706,7 @@ export async function* runValidation({ type: "unknown-fix", sourcePath, fixes: v.fixes, + ...(v.keyError ? { keyError: true } : {}), }; fileErrors += 1; continue; @@ -737,6 +752,7 @@ export async function* runValidation({ type: "validation-error", sourcePath, message: result.errorMessage ?? "Unknown error", + ...(v.keyError ? { keyError: true } : {}), }; fileErrors += 1; continue; @@ -769,6 +785,7 @@ export async function* runValidation({ sourcePath, message: v.message, fixable: true, + ...(v.keyError ? { keyError: true } : {}), }; } @@ -779,6 +796,7 @@ export async function* runValidation({ sourcePath, message: e.message, fixable: !!(e.fixes && e.fixes.length), + ...(e.keyError ? { keyError: true } : {}), }; } } diff --git a/packages/cli/src/utils/sourcePathToFileLocation.ts b/packages/cli/src/utils/sourcePathToFileLocation.ts index b33fa348b..cfbdbceae 100644 --- a/packages/cli/src/utils/sourcePathToFileLocation.ts +++ b/packages/cli/src/utils/sourcePathToFileLocation.ts @@ -33,8 +33,9 @@ export function sourcePathToFileLocation( sourcePath: string, projectRoot: string, cache: SourceFileCache, + target: "key" | "value" = "value", ): string { - const resolved = resolveRange(sourcePath, projectRoot, cache); + const resolved = resolveRange(sourcePath, projectRoot, cache, target); if (!resolved) { return sourcePath; } @@ -53,8 +54,9 @@ export function sourcePathToCodeFrame( sourcePath: string, projectRoot: string, cache: SourceFileCache, + target: "key" | "value" = "value", ): string | undefined { - const resolved = resolveRange(sourcePath, projectRoot, cache); + const resolved = resolveRange(sourcePath, projectRoot, cache, target); if (!resolved) { return undefined; } @@ -91,6 +93,7 @@ function resolveRange( sourcePath: string, projectRoot: string, cache: SourceFileCache, + target: "key" | "value", ): { relativeFile: string; lines: string[]; range: Range } | undefined { const [moduleFilePath, modulePath] = Internal.splitModuleFilePathAndModulePath(sourcePath as SourcePath); @@ -100,7 +103,7 @@ function resolveRange( return undefined; } - const range = getModulePathRange(modulePath, cached.map); + const range = getModulePathRange(modulePath, cached.map, target); if (!range) { return undefined; } diff --git a/packages/cli/src/validate.ts b/packages/cli/src/validate.ts index 78aa18b1b..5150c2cb2 100644 --- a/packages/cli/src/validate.ts +++ b/packages/cli/src/validate.ts @@ -61,13 +61,31 @@ export async function validate({ // when resolving sourcePaths to file locations and code frames. const sourceFileCache: SourceFileCache = new Map(); - // Prints `file:line:col` followed by a Rust-style code frame (when the - // location can be resolved) for an error at the given sourcePath. - const logSourceLocation = (sourcePath: string) => { + // Resolves a sourcePath to `file:line:col (key|value)`, pointing the carets at + // the key when the error is about a key and the value otherwise. Falls back to + // the raw sourcePath (no label) when the location cannot be resolved. + const formatLocation = (sourcePath: string, keyError?: boolean) => { + const target = keyError ? "key" : "value"; + const location = sourcePathToFileLocation( + sourcePath, + projectRoot, + sourceFileCache, + target, + ); + if (location === sourcePath) { + return location; + } + return `${location} (${target})`; + }; + + // Prints a Rust-style code frame (when the location can be resolved) for an + // error at the given sourcePath, underlining the key or the value. + const logSourceLocation = (sourcePath: string, keyError?: boolean) => { const frame = sourcePathToCodeFrame( sourcePath, projectRoot, sourceFileCache, + keyError ? "key" : "value", ); if (frame !== undefined) { console.log("\n" + frame); @@ -115,19 +133,19 @@ export async function validate({ console.log( picocolors.red("✘"), "Got error in", - `${sourcePathToFileLocation(event.sourcePath, projectRoot, sourceFileCache)}:`, + `${formatLocation(event.sourcePath, event.keyError)}:`, event.message, ); - logSourceLocation(event.sourcePath); + logSourceLocation(event.sourcePath, event.keyError); break; case "validation-fixable-error": console.log( event.fixable ? picocolors.yellow("⚠") : picocolors.red("✘"), `Got ${event.fixable ? "fixable " : ""}error in`, - `${sourcePathToFileLocation(event.sourcePath, projectRoot, sourceFileCache)}:`, + `${formatLocation(event.sourcePath, event.keyError)}:`, event.message, ); - logSourceLocation(event.sourcePath); + logSourceLocation(event.sourcePath, event.keyError); break; case "unknown-fix": console.log( @@ -135,13 +153,9 @@ export async function validate({ "Unknown fix", event.fixes, "for", - sourcePathToFileLocation( - event.sourcePath, - projectRoot, - sourceFileCache, - ), + formatLocation(event.sourcePath, event.keyError), ); - logSourceLocation(event.sourcePath); + logSourceLocation(event.sourcePath, event.keyError); break; case "unregistered-module": console.log( diff --git a/packages/server/src/modulePathMap.test.ts b/packages/server/src/modulePathMap.test.ts index cae6890f6..bf28e9a63 100644 --- a/packages/server/src/modulePathMap.test.ts +++ b/packages/server/src/modulePathMap.test.ts @@ -85,6 +85,17 @@ export default c.define( getModulePathRange('"nested"."text"', modulePathMap), { end: { character: 8, line: 50 }, start: { character: 4, line: 50 } }, ); + + // target "value" points at the value ('hei') instead of the key (text) + assert.deepStrictEqual( + getModulePathRange('"text"', modulePathMap, "value"), + { end: { character: 13, line: 48 }, start: { character: 8, line: 48 } }, + ); + // explicit "key" matches the default + assert.deepStrictEqual( + getModulePathRange('"text"', modulePathMap, "key"), + getModulePathRange('"text"', modulePathMap), + ); }); test("test 2", () => { diff --git a/packages/server/src/modulePathMap.ts b/packages/server/src/modulePathMap.ts index 5d622a7b1..604cf6567 100644 --- a/packages/server/src/modulePathMap.ts +++ b/packages/server/src/modulePathMap.ts @@ -18,6 +18,13 @@ export type ModulePathMap = { export function getModulePathRange( modulePath: string, modulePathMap: ModulePathMap, + // Which part of an object/record member to point at. For an object property + // the resolved node's own range is the *key* (property name); the *value* + // range is stored under `children.val`. Array elements, leaf literals and + // `c.image`/`c.file` `_ref`/`metadata` nodes have no `val` child, so "value" + // falls back to the node's own range for those. Defaults to "key" to preserve + // existing callers. + target: "key" | "value" = "key", ) { // Handle empty or invalid module paths gracefully if (!modulePath || typeof modulePath !== "string") { @@ -47,11 +54,18 @@ export function getModulePathRange( } range = range?.children?.[pathSegment]; } + + if (!range) { + return undefined; + } + + const valueRange = target === "value" ? range.children?.val : undefined; + const resolved = valueRange ?? range; return ( - range?.start && - range?.end && { - start: range.start, - end: range.end, + resolved.start && + resolved.end && { + start: resolved.start, + end: resolved.end, } ); } From fe82d2074aa0ad5097cd665b9f66d708b5a97670 Mon Sep 17 00:00:00 2001 From: Fredrik Ekholdt Date: Wed, 24 Jun 2026 22:18:02 +0200 Subject: [PATCH 09/16] Show code frames for validation fixes as well --- packages/cli/src/runValidation.ts | 4 +++- packages/server/src/createFixPatch.ts | 16 ++++++++++++++-- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/packages/cli/src/runValidation.ts b/packages/cli/src/runValidation.ts index e865ab899..d9aafdb44 100644 --- a/packages/cli/src/runValidation.ts +++ b/packages/cli/src/runValidation.ts @@ -793,7 +793,9 @@ export async function* runValidation({ fileErrors += 1; yield { type: "validation-fixable-error", - sourcePath, + // Gallery checks expand into per-entry errors that point at + // the individual entry; fall back to the record sourcePath. + sourcePath: e.sourcePath ?? sourcePath, message: e.message, fixable: !!(e.fixes && e.fixes.length), ...(e.keyError ? { keyError: true } : {}), diff --git a/packages/server/src/createFixPatch.ts b/packages/server/src/createFixPatch.ts index 765f65104..208733e8f 100644 --- a/packages/server/src/createFixPatch.ts +++ b/packages/server/src/createFixPatch.ts @@ -22,6 +22,14 @@ import { getValidationErrorFileRef } from "./getValidationErrorFileRef"; import path from "path"; import { checkRemoteRef, downloadFileFromRemote } from "./checkRemoteRef"; +// A remaining error may optionally carry a more specific `sourcePath` than the +// one the fix was created from. This is used by gallery checks, where a single +// record-level fix expands into per-entry errors that should point at the +// individual entry (e.g. `?p="/public/val/logo.png"`) rather than the record. +export type FixPatchRemainingError = ValidationError & { + sourcePath?: SourcePath; +}; + // TODO: find a better name? transformFixesToPatch? export async function createFixPatch( config: { projectRoot: string; remoteHost: string }, @@ -36,8 +44,10 @@ export async function createFixPatch( }, moduleSource?: Source, moduleSchema?: SerializedSchema, -): Promise<{ patch: Patch; remainingErrors: ValidationError[] } | undefined> { - const remainingErrors: ValidationError[] = []; +): Promise< + { patch: Patch; remainingErrors: FixPatchRemainingError[] } | undefined +> { + const remainingErrors: FixPatchRemainingError[] = []; const patch: Patch = []; for (const fix of validationError.fixes || []) { if (fix === "image:check-metadata" || fix === "image:add-metadata") { @@ -419,6 +429,7 @@ export async function createFixPatch( remainingErrors.push({ ...validationError, message: `Image metadata for '${entryKey}' is incorrect (width: ${stored.width ?? ""} vs ${actualMetadata.width}, height: ${stored.height ?? ""} vs ${actualMetadata.height}, mimeType: ${stored.mimeType ?? ""} vs ${actualMetadata.mimeType}). Use --fix to update.`, + sourcePath: Internal.createValPathOfItem(sourcePath, entryKey), }); } } @@ -441,6 +452,7 @@ export async function createFixPatch( remainingErrors.push({ ...validationError, message: `File metadata for '${entryKey}' has incorrect mimeType: '${stored.mimeType ?? ""}' vs '${actualMetadata.mimeType}'. Use --fix to update.`, + sourcePath: Internal.createValPathOfItem(sourcePath, entryKey), }); } } From 1416bee4393a47539bd69a2ad75e6521b9b03cde Mon Sep 17 00:00:00 2001 From: Fredrik Ekholdt Date: Wed, 24 Jun 2026 22:18:18 +0200 Subject: [PATCH 10/16] Changeset --- .changeset/validation-error-code-frames.md | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 .changeset/validation-error-code-frames.md diff --git a/.changeset/validation-error-code-frames.md b/.changeset/validation-error-code-frames.md new file mode 100644 index 000000000..4e2ac9493 --- /dev/null +++ b/.changeset/validation-error-code-frames.md @@ -0,0 +1,8 @@ +--- +"@valbuild/server": patch +"@valbuild/cli": patch +--- + +Validation errors from the CLI now point at the offending location in the `.val.ts` file. Each error shows a clickable `file:line:col` and a code frame (the line above, the offending line, and the line below) with carets underlining the exact source. The carets target the **value** by default and the **key** when the error is about an object/record key (`keyError`), and the output is labelled `(key)` / `(value)` accordingly. Gallery metadata errors now point at the specific record entry instead of the whole module. + +The module-path → source-range utility used for this (`createModulePathMap`, `getModulePathRange`, `ModulePathMap`) is now exported from `@valbuild/server`. From d10bd502b5e173182ba74163ddfa72d8945a7c9f Mon Sep 17 00:00:00 2001 From: Fredrik Ekholdt Date: Wed, 24 Jun 2026 22:18:42 +0200 Subject: [PATCH 11/16] In s.files and s.images the validation code frame error should be on key (looks nice IMO) --- packages/core/src/schema/record.ts | 13 ++++++++++++- packages/server/src/createFixPatch.ts | 6 ++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/packages/core/src/schema/record.ts b/packages/core/src/schema/record.ts index 7ca5b6ebf..7b1aa3794 100644 --- a/packages/core/src/schema/record.ts +++ b/packages/core/src/schema/record.ts @@ -225,13 +225,17 @@ export class RecordSchema< src, ); } else if (this.mediaOptions) { - // Media collection: validate key (path/URL) and entry (metadata) + // Media collection: validate key (path/URL) and entry (metadata). + // Gallery entries are keyed by their file path and the metadata is + // derived, so surface entry errors on the key rather than the value. const keyErr = this.validateMediaKey(subPath, key); if (keyErr) { + this.markKeyErrorsAtPath(keyErr, subPath); error = error ? { ...error, ...keyErr } : keyErr; } const entryErr = this.validateMediaEntry(subPath, elem); if (entryErr) { + this.markKeyErrorsAtPath(entryErr, subPath); error = error ? { ...error, ...entryErr } : entryErr; } } else { @@ -256,6 +260,13 @@ export class RecordSchema< return url.startsWith("https://") || url.startsWith("http://"); } + /** Marks the validation errors reported at `path` as key errors (in place). */ + private markKeyErrorsAtPath(errors: ValidationErrors, path: SourcePath) { + if (errors && errors[path]) { + errors[path] = errors[path].map((err) => ({ ...err, keyError: true })); + } + } + private validateMediaKey(path: SourcePath, key: string): ValidationErrors { if (!this.mediaOptions) { return false; diff --git a/packages/server/src/createFixPatch.ts b/packages/server/src/createFixPatch.ts index 208733e8f..a458b6552 100644 --- a/packages/server/src/createFixPatch.ts +++ b/packages/server/src/createFixPatch.ts @@ -430,6 +430,9 @@ export async function createFixPatch( ...validationError, message: `Image metadata for '${entryKey}' is incorrect (width: ${stored.width ?? ""} vs ${actualMetadata.width}, height: ${stored.height ?? ""} vs ${actualMetadata.height}, mimeType: ${stored.mimeType ?? ""} vs ${actualMetadata.mimeType}). Use --fix to update.`, sourcePath: Internal.createValPathOfItem(sourcePath, entryKey), + // Gallery entries are keyed by their file path; surface the + // error on the key rather than the derived metadata value. + keyError: true, }); } } @@ -453,6 +456,9 @@ export async function createFixPatch( ...validationError, message: `File metadata for '${entryKey}' has incorrect mimeType: '${stored.mimeType ?? ""}' vs '${actualMetadata.mimeType}'. Use --fix to update.`, sourcePath: Internal.createValPathOfItem(sourcePath, entryKey), + // Gallery entries are keyed by their file path; surface the + // error on the key rather than the derived metadata value. + keyError: true, }); } } From a481fe43253526d05d85beb917ebaeb56d38429e Mon Sep 17 00:00:00 2001 From: Fredrik Ekholdt Date: Wed, 24 Jun 2026 22:28:26 +0200 Subject: [PATCH 12/16] Add watch mode to validate --- packages/cli/package.json | 1 + packages/cli/src/cli.ts | 11 + packages/cli/src/validate.ts | 482 +++++++++++++++++++++-------------- pnpm-lock.yaml | 9 +- 4 files changed, 305 insertions(+), 198 deletions(-) diff --git a/packages/cli/package.json b/packages/cli/package.json index adea840d4..7c3491e1f 100644 --- a/packages/cli/package.json +++ b/packages/cli/package.json @@ -27,6 +27,7 @@ "@valbuild/server": "workspace:*", "@valbuild/shared": "workspace:*", "chalk": "^5.6.2", + "chokidar": "^5.0.0", "cors": "^2.8.5", "fast-glob": "^3.3.3", "meow": "^9.0.0", diff --git a/packages/cli/src/cli.ts b/packages/cli/src/cli.ts index 14af857ef..8e76e2d84 100644 --- a/packages/cli/src/cli.ts +++ b/packages/cli/src/cli.ts @@ -28,6 +28,7 @@ async function main(): Promise { Options: --root [root], -r [root] Set project root directory (default process.cwd()) --fix [fix] Attempt to fix validation errors + --watch, -w Re-validate on changes to val.config, val.modules and *.val files Command: login @@ -62,6 +63,10 @@ async function main(): Promise { fix: { type: "boolean", }, + watch: { + type: "boolean", + alias: "w", + }, noEslint: { type: "boolean", }, @@ -112,9 +117,15 @@ async function main(): Promise { if (flags.managedDir) { return error(`Command "validate" does not support --managedDir flag`); } + if (flags.watch && flags.fix) { + return error( + `Command "validate" does not support --watch together with --fix`, + ); + } return validate({ root: flags.root, fix: flags.fix, + watch: flags.watch, }); default: return error(`Unknown command "${input.join(" ")}"`); diff --git a/packages/cli/src/validate.ts b/packages/cli/src/validate.ts index 5150c2cb2..578dd4605 100644 --- a/packages/cli/src/validate.ts +++ b/packages/cli/src/validate.ts @@ -15,221 +15,313 @@ import { export async function validate({ root, fix, + watch, }: { root?: string; fix?: boolean; + watch?: boolean; }) { const projectRoot = root ? path.resolve(root) : process.cwd(); - const valConfigFile = - (await evalValConfigFile(projectRoot, "val.config.ts")) || - (await evalValConfigFile(projectRoot, "val.config.js")); - - const resolvedValConfigFile = valConfigFile - ? { - ...valConfigFile, - project: process.env.VAL_PROJECT || valConfigFile.project, - } - : process.env.VAL_PROJECT - ? { project: process.env.VAL_PROJECT } - : undefined; - - console.log( - picocolors.greenBright( - `Validating project${resolvedValConfigFile?.project ? ` '${picocolors.inverse(resolvedValConfigFile.project)}'` : ""}...`, - ), - ); - - const valFiles: string[] = await glob("**/*.val.{js,ts}", { - ignore: ["node_modules/**"], - cwd: projectRoot, - }); - - console.log(picocolors.greenBright(`Found ${valFiles.length} files...`)); - - let prettier; + let prettier: typeof import("prettier") | undefined; try { - prettier = (await import("prettier")).default; + prettier = await import("prettier"); } catch { console.log("Prettier not found, skipping formatting"); } - const fixedFiles = new Set(); - let totalErrors = 0; - - // Caches each val file's parsed source so files are read/parsed at most once - // when resolving sourcePaths to file locations and code frames. - const sourceFileCache: SourceFileCache = new Map(); - - // Resolves a sourcePath to `file:line:col (key|value)`, pointing the carets at - // the key when the error is about a key and the value otherwise. Falls back to - // the raw sourcePath (no label) when the location cannot be resolved. - const formatLocation = (sourcePath: string, keyError?: boolean) => { - const target = keyError ? "key" : "value"; - const location = sourcePathToFileLocation( - sourcePath, - projectRoot, - sourceFileCache, - target, + // Runs a single validation pass over the project and returns the number of + // errors found. Re-reads config and val files each call so it always reflects + // the latest state on disk (used both for one-shot and watch mode). + async function runOnce(): Promise { + const valConfigFile = + (await evalValConfigFile(projectRoot, "val.config.ts")) || + (await evalValConfigFile(projectRoot, "val.config.js")); + + const resolvedValConfigFile = valConfigFile + ? { + ...valConfigFile, + project: process.env.VAL_PROJECT || valConfigFile.project, + } + : process.env.VAL_PROJECT + ? { project: process.env.VAL_PROJECT } + : undefined; + + console.log( + picocolors.greenBright( + `Validating project${resolvedValConfigFile?.project ? ` '${picocolors.inverse(resolvedValConfigFile.project)}'` : ""}...`, + ), ); - if (location === sourcePath) { - return location; + + const valFiles: string[] = await glob("**/*.val.{js,ts}", { + ignore: ["node_modules/**"], + cwd: projectRoot, + }); + + console.log(picocolors.greenBright(`Found ${valFiles.length} files...`)); + + const fixedFiles = new Set(); + let totalErrors = 0; + + // Caches each val file's parsed source so files are read/parsed at most once + // per pass when resolving sourcePaths to file locations and code frames. + const sourceFileCache: SourceFileCache = new Map(); + + // Resolves a sourcePath to `file:line:col (key|value)`, pointing the carets + // at the key when the error is about a key and the value otherwise. Falls + // back to the raw sourcePath (no label) when it cannot be resolved. + const formatLocation = (sourcePath: string, keyError?: boolean) => { + const target = keyError ? "key" : "value"; + const location = sourcePathToFileLocation( + sourcePath, + projectRoot, + sourceFileCache, + target, + ); + if (location === sourcePath) { + return location; + } + return `${location} (${target})`; + }; + + // Prints a Rust-style code frame (when the location can be resolved) for an + // error at the given sourcePath, underlining the key or the value. + const logSourceLocation = (sourcePath: string, keyError?: boolean) => { + const frame = sourcePathToCodeFrame( + sourcePath, + projectRoot, + sourceFileCache, + keyError ? "key" : "value", + ); + if (frame !== undefined) { + console.log("\n" + frame); + } + }; + + for await (const event of runValidation({ + root: projectRoot, + fix: !!fix, + valFiles, + project: resolvedValConfigFile?.project, + remote: { + remoteHost: process.env.VAL_REMOTE_HOST || DEFAULT_VAL_REMOTE_HOST, + getSettings: (projectName, options) => + getSettings(projectName, options), + uploadFile: (project, bucket, fileHash, fileExt, fileBuffer, options) => + uploadRemoteFile( + process.env.VAL_CONTENT_URL || DEFAULT_CONTENT_HOST, + project, + bucket, + fileHash, + fileExt ?? "", + fileBuffer, + options, + ), + }, + fs: createDefaultValFSHost(), + })) { + switch (event.type) { + case "file-valid": + console.log( + picocolors.green("✔"), + event.file, + "is valid (" + event.durationMs + "ms)", + ); + break; + case "file-error-count": + console.log( + picocolors.red("✘"), + `${event.file} contains ${event.errorCount} error${event.errorCount > 1 ? "s" : ""}`, + " (" + event.durationMs + "ms)", + ); + totalErrors += event.errorCount; + break; + case "validation-error": + console.log( + picocolors.red("✘"), + "Got error in", + `${formatLocation(event.sourcePath, event.keyError)}:`, + event.message, + ); + logSourceLocation(event.sourcePath, event.keyError); + break; + case "validation-fixable-error": + console.log( + event.fixable ? picocolors.yellow("⚠") : picocolors.red("✘"), + `Got ${event.fixable ? "fixable " : ""}error in`, + `${formatLocation(event.sourcePath, event.keyError)}:`, + event.message, + ); + logSourceLocation(event.sourcePath, event.keyError); + break; + case "unknown-fix": + console.log( + picocolors.red("✘"), + "Unknown fix", + event.fixes, + "for", + formatLocation(event.sourcePath, event.keyError), + ); + logSourceLocation(event.sourcePath, event.keyError); + break; + case "unregistered-module": + console.log( + picocolors.yellow("⚠"), + `/${event.file} is not registered in val.modules - skipping`, + ); + break; + case "fix-applied": + console.log( + picocolors.yellow("⚠"), + "Applied fix for", + event.sourcePath, + ); + fixedFiles.add(event.file); + break; + case "fatal-error": + console.log( + picocolors.red("✘"), + event.file, + "is invalid:", + event.message, + ); + break; + case "remote-uploading": + console.log( + picocolors.yellow("⚠"), + `Uploading remote file: '${event.ref}'...`, + ); + break; + case "remote-uploaded": + console.log( + picocolors.green("✔"), + `Completed upload of remote file: '${event.ref}'`, + ); + break; + case "remote-already-uploaded": + console.log( + picocolors.yellow("⚠"), + `Remote file ${event.filePath} already uploaded`, + ); + break; + case "remote-downloading": + console.log( + picocolors.yellow("⚠"), + `Downloading remote file in ${event.sourcePath}...`, + ); + break; + case "summary-errors": + case "summary-success": + break; + } } - return `${location} (${target})`; - }; - // Prints a Rust-style code frame (when the location can be resolved) for an - // error at the given sourcePath, underlining the key or the value. - const logSourceLocation = (sourcePath: string, keyError?: boolean) => { - const frame = sourcePathToCodeFrame( - sourcePath, - projectRoot, - sourceFileCache, - keyError ? "key" : "value", - ); - if (frame !== undefined) { - console.log("\n" + frame); + // Run prettier on files that had fixes applied + if (prettier) { + for (const file of fixedFiles) { + const filePath = path.join(projectRoot, file); + const fileContent = await fs.readFile(filePath, "utf-8"); + const formattedContent = await prettier.format(fileContent, { + filepath: filePath, + }); + await fs.writeFile(filePath, formattedContent); + } } - }; - for await (const event of runValidation({ - root: projectRoot, - fix: !!fix, - valFiles, - project: resolvedValConfigFile?.project, - remote: { - remoteHost: process.env.VAL_REMOTE_HOST || DEFAULT_VAL_REMOTE_HOST, - getSettings: (projectName, options) => getSettings(projectName, options), - uploadFile: (project, bucket, fileHash, fileExt, fileBuffer, options) => - uploadRemoteFile( - process.env.VAL_CONTENT_URL || DEFAULT_CONTENT_HOST, - project, - bucket, - fileHash, - fileExt ?? "", - fileBuffer, - options, - ), - }, - fs: createDefaultValFSHost(), - })) { - switch (event.type) { - case "file-valid": - console.log( - picocolors.green("✔"), - event.file, - "is valid (" + event.durationMs + "ms)", - ); - break; - case "file-error-count": - console.log( - picocolors.red("✘"), - `${event.file} contains ${event.errorCount} error${event.errorCount > 1 ? "s" : ""}`, - " (" + event.durationMs + "ms)", - ); - totalErrors += event.errorCount; - break; - case "validation-error": - console.log( - picocolors.red("✘"), - "Got error in", - `${formatLocation(event.sourcePath, event.keyError)}:`, - event.message, - ); - logSourceLocation(event.sourcePath, event.keyError); - break; - case "validation-fixable-error": - console.log( - event.fixable ? picocolors.yellow("⚠") : picocolors.red("✘"), - `Got ${event.fixable ? "fixable " : ""}error in`, - `${formatLocation(event.sourcePath, event.keyError)}:`, - event.message, - ); - logSourceLocation(event.sourcePath, event.keyError); - break; - case "unknown-fix": - console.log( - picocolors.red("✘"), - "Unknown fix", - event.fixes, - "for", - formatLocation(event.sourcePath, event.keyError), - ); - logSourceLocation(event.sourcePath, event.keyError); - break; - case "unregistered-module": - console.log( - picocolors.yellow("⚠"), - `/${event.file} is not registered in val.modules - skipping`, - ); - break; - case "fix-applied": - console.log( - picocolors.yellow("⚠"), - "Applied fix for", - event.sourcePath, - ); - fixedFiles.add(event.file); - break; - case "fatal-error": - console.log( - picocolors.red("✘"), - event.file, - "is invalid:", - event.message, - ); - break; - case "remote-uploading": - console.log( - picocolors.yellow("⚠"), - `Uploading remote file: '${event.ref}'...`, - ); - break; - case "remote-uploaded": - console.log( - picocolors.green("✔"), - `Completed upload of remote file: '${event.ref}'`, - ); - break; - case "remote-already-uploaded": - console.log( - picocolors.yellow("⚠"), - `Remote file ${event.filePath} already uploaded`, - ); - break; - case "remote-downloading": - console.log( - picocolors.yellow("⚠"), - `Downloading remote file in ${event.sourcePath}...`, - ); - break; - case "summary-errors": - case "summary-success": - break; + if (totalErrors > 0) { + console.log( + picocolors.red("✘"), + "Got", + totalErrors, + "error" + (totalErrors > 1 ? "s" : ""), + ); + } else { + console.log(picocolors.green("✔"), "No validation errors found"); } + return totalErrors; } - // Run prettier on files that had fixes applied - if (prettier) { - for (const file of fixedFiles) { - const filePath = path.join(projectRoot, file); - const fileContent = await fs.readFile(filePath, "utf-8"); - const formattedContent = await prettier.format(fileContent, { - filepath: filePath, - }); - await fs.writeFile(filePath, formattedContent); + if (!watch) { + const totalErrors = await runOnce(); + if (totalErrors > 0) { + process.exit(1); } + return; } - if (totalErrors > 0) { - console.log( - picocolors.red("✘"), - "Got", - totalErrors, - "error" + (totalErrors > 1 ? "s" : ""), - ); - process.exit(1); - } else { - console.log(picocolors.green("✔"), "No validation errors found"); - } + await watchAndValidate(projectRoot, runOnce); +} + +// Directory names anywhere in the path that should never be watched. +const WATCH_IGNORED = /(^|[\\/])(node_modules|\.git|dist)([\\/]|$)/; + +function isRelevantValFile(filePath: string): boolean { + const base = path.basename(filePath); + return ( + base === "val.modules.ts" || + base === "val.modules.js" || + base === "val.config.ts" || + base === "val.config.js" || + /\.val\.(ts|js)$/.test(base) + ); +} + +async function watchAndValidate( + projectRoot: string, + runOnce: () => Promise, +) { + // Initial pass. + await runOnce(); + const watchingMessage = picocolors.dim( + "Watching for changes... (Ctrl+C to exit)", + ); + console.log(watchingMessage); + + // chokidar 5 is ESM-only; load it dynamically (like prettier above). + const { watch } = await import("chokidar"); + + let running = false; + let pending = false; + let debounceTimer: ReturnType | undefined; + + const triggerRun = async () => { + if (running) { + pending = true; + return; + } + running = true; + // Clear the screen (and scrollback) so only the latest result shows. + process.stdout.write("\x1b[2J\x1b[3J\x1b[H"); + console.log(picocolors.cyanBright("Re-validating...")); + try { + await runOnce(); + } catch (err) { + console.error(err); + } + console.log(watchingMessage); + running = false; + if (pending) { + pending = false; + void triggerRun(); + } + }; + + const watcher = watch(projectRoot, { + ignoreInitial: true, + ignored: (watchedPath: string) => WATCH_IGNORED.test(watchedPath), + }); + + watcher.on("all", (_event, changedPath) => { + if (!isRelevantValFile(changedPath)) { + return; + } + if (debounceTimer) { + clearTimeout(debounceTimer); + } + debounceTimer = setTimeout(() => void triggerRun(), 150); + }); + + process.on("SIGINT", () => { + void watcher.close().then(() => process.exit(0)); + }); } diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 7df2b6904..d6294df3a 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -141,6 +141,9 @@ importers: chalk: specifier: ^5.6.2 version: 5.6.2 + chokidar: + specifier: ^5.0.0 + version: 5.0.0 cors: specifier: ^2.8.5 version: 2.8.6 @@ -16292,7 +16295,7 @@ snapshots: eslint: 8.32.0 eslint-import-resolver-node: 0.3.9 eslint-import-resolver-typescript: 3.10.1(eslint-plugin-import@2.32.0(@typescript-eslint/parser@8.53.1(eslint@9.39.2(jiti@1.21.7))(typescript@5.9.3))(eslint@8.32.0))(eslint@8.32.0) - eslint-plugin-import: 2.32.0(@typescript-eslint/parser@5.62.0(eslint@8.32.0)(typescript@5.1.3))(eslint-import-resolver-typescript@3.10.1)(eslint@8.32.0) + eslint-plugin-import: 2.32.0(@typescript-eslint/parser@5.62.0(eslint@8.32.0)(typescript@5.1.3))(eslint-import-resolver-typescript@3.10.1(eslint-plugin-import@2.32.0(@typescript-eslint/parser@8.53.1(eslint@9.39.2(jiti@1.21.7))(typescript@5.9.3))(eslint@8.32.0))(eslint@8.32.0))(eslint@8.32.0) eslint-plugin-jsx-a11y: 6.10.2(eslint@8.32.0) eslint-plugin-react: 7.37.5(eslint@8.32.0) eslint-plugin-react-hooks: 4.6.2(eslint@8.32.0) @@ -16338,7 +16341,7 @@ snapshots: tinyglobby: 0.2.15 unrs-resolver: 1.11.1 optionalDependencies: - eslint-plugin-import: 2.32.0(@typescript-eslint/parser@5.62.0(eslint@8.32.0)(typescript@5.1.3))(eslint-import-resolver-typescript@3.10.1)(eslint@8.32.0) + eslint-plugin-import: 2.32.0(@typescript-eslint/parser@5.62.0(eslint@8.32.0)(typescript@5.1.3))(eslint-import-resolver-typescript@3.10.1(eslint-plugin-import@2.32.0(@typescript-eslint/parser@8.53.1(eslint@9.39.2(jiti@1.21.7))(typescript@5.9.3))(eslint@8.32.0))(eslint@8.32.0))(eslint@8.32.0) transitivePeerDependencies: - supports-color @@ -16393,7 +16396,7 @@ snapshots: - eslint-import-resolver-webpack - supports-color - eslint-plugin-import@2.32.0(@typescript-eslint/parser@5.62.0(eslint@8.32.0)(typescript@5.1.3))(eslint-import-resolver-typescript@3.10.1)(eslint@8.32.0): + eslint-plugin-import@2.32.0(@typescript-eslint/parser@5.62.0(eslint@8.32.0)(typescript@5.1.3))(eslint-import-resolver-typescript@3.10.1(eslint-plugin-import@2.32.0(@typescript-eslint/parser@8.53.1(eslint@9.39.2(jiti@1.21.7))(typescript@5.9.3))(eslint@8.32.0))(eslint@8.32.0))(eslint@8.32.0): dependencies: '@rtsao/scc': 1.1.0 array-includes: 3.1.9 From 295cd1e86e8905fe9f96aee65388db7df0cc078c Mon Sep 17 00:00:00 2001 From: Fredrik Ekholdt Date: Thu, 25 Jun 2026 08:18:28 +0200 Subject: [PATCH 13/16] Reverse order of validation output (fixable at the end) --- .../cli/src/utils/sourcePathToFileLocation.ts | 25 ++ packages/cli/src/validate.ts | 274 +++++++++++++----- 2 files changed, 221 insertions(+), 78 deletions(-) diff --git a/packages/cli/src/utils/sourcePathToFileLocation.ts b/packages/cli/src/utils/sourcePathToFileLocation.ts index cfbdbceae..ec648856d 100644 --- a/packages/cli/src/utils/sourcePathToFileLocation.ts +++ b/packages/cli/src/utils/sourcePathToFileLocation.ts @@ -44,6 +44,31 @@ export function sourcePathToFileLocation( return `${relativeFile}:${range.start.line + 1}:${range.start.character + 1}`; } +/** + * Resolves a validation `sourcePath` to its individual location parts: the + * relative file path and the 1-indexed line/column of the offending literal. + * Returns `undefined` when the location cannot be resolved (the caller should + * then fall back to the raw `sourcePath`). + */ +export function sourcePathToLocationParts( + sourcePath: string, + projectRoot: string, + cache: SourceFileCache, + target: "key" | "value" = "value", +): { relativeFile: string; line: number; character: number } | undefined { + const resolved = resolveRange(sourcePath, projectRoot, cache, target); + if (!resolved) { + return undefined; + } + // TS line/character are 0-indexed; editors/terminals expect 1-indexed. + const { relativeFile, range } = resolved; + return { + relativeFile, + line: range.start.line + 1, + character: range.start.character + 1, + }; +} + /** * Renders a Rust-style code frame for the offending `sourcePath`: the line * above, the offending line, and the line below, with red carets underlining diff --git a/packages/cli/src/validate.ts b/packages/cli/src/validate.ts index 578dd4605..beb839aec 100644 --- a/packages/cli/src/validate.ts +++ b/packages/cli/src/validate.ts @@ -8,10 +8,25 @@ import { evalValConfigFile } from "./utils/evalValConfigFile"; import { createDefaultValFSHost, runValidation } from "./runValidation"; import { sourcePathToCodeFrame, - sourcePathToFileLocation, + sourcePathToLocationParts, type SourceFileCache, } from "./utils/sourcePathToFileLocation"; +type Diagnostic = { + // "fixable" => ⚠ (run --fix), "error" => ✘ + severity: "fixable" | "error"; + sourcePath: string; + message: string; + keyError?: boolean; +}; + +type ModuleReport = { + // Relative file path (no leading slash). + file: string; + durationMs: number; + diagnostics: Diagnostic[]; +}; + export async function validate({ root, fix, @@ -67,35 +82,29 @@ export async function validate({ // per pass when resolving sourcePaths to file locations and code frames. const sourceFileCache: SourceFileCache = new Map(); - // Resolves a sourcePath to `file:line:col (key|value)`, pointing the carets - // at the key when the error is about a key and the value otherwise. Falls - // back to the raw sourcePath (no label) when it cannot be resolved. - const formatLocation = (sourcePath: string, keyError?: boolean) => { - const target = keyError ? "key" : "value"; - const location = sourcePathToFileLocation( - sourcePath, - projectRoot, - sourceFileCache, - target, - ); - if (location === sourcePath) { - return location; - } - return `${location} (${target})`; - }; + // Diagnostics are buffered per module (keyed by relative file path) so we can + // render them grouped and prioritised after the run, rather than streaming + // them out interleaved. Transient progress (remote/fix-applied) still streams + // live below. + const reports = new Map(); + const valid: { file: string; durationMs: number }[] = []; + const skipped: string[] = []; - // Prints a Rust-style code frame (when the location can be resolved) for an - // error at the given sourcePath, underlining the key or the value. - const logSourceLocation = (sourcePath: string, keyError?: boolean) => { - const frame = sourcePathToCodeFrame( - sourcePath, - projectRoot, - sourceFileCache, - keyError ? "key" : "value", - ); - if (frame !== undefined) { - console.log("\n" + frame); + // Relative file path (no leading slash), matching the code frame's + // relativeFile so headers, diagnostics and frames all agree. + const relFile = (file: string) => file.replace(/^\//, ""); + // The module a sourcePath/file belongs to is the part before the `?p=...`. + const moduleOf = (sourcePathOrFile: string) => + relFile(sourcePathOrFile.split("?")[0]); + + const reportFor = (file: string): ModuleReport => { + const key = relFile(file); + let report = reports.get(key); + if (!report) { + report = { file: key, durationMs: 0, diagnostics: [] }; + reports.set(key, report); } + return report; }; for await (const event of runValidation({ @@ -122,53 +131,49 @@ export async function validate({ })) { switch (event.type) { case "file-valid": - console.log( - picocolors.green("✔"), - event.file, - "is valid (" + event.durationMs + "ms)", - ); + valid.push({ + file: relFile(event.file), + durationMs: event.durationMs, + }); break; case "file-error-count": - console.log( - picocolors.red("✘"), - `${event.file} contains ${event.errorCount} error${event.errorCount > 1 ? "s" : ""}`, - " (" + event.durationMs + "ms)", - ); + reportFor(event.file).durationMs = event.durationMs; totalErrors += event.errorCount; break; case "validation-error": - console.log( - picocolors.red("✘"), - "Got error in", - `${formatLocation(event.sourcePath, event.keyError)}:`, - event.message, - ); - logSourceLocation(event.sourcePath, event.keyError); + reportFor(moduleOf(event.sourcePath)).diagnostics.push({ + severity: "error", + sourcePath: event.sourcePath, + message: event.message, + ...(event.keyError ? { keyError: true } : {}), + }); break; case "validation-fixable-error": - console.log( - event.fixable ? picocolors.yellow("⚠") : picocolors.red("✘"), - `Got ${event.fixable ? "fixable " : ""}error in`, - `${formatLocation(event.sourcePath, event.keyError)}:`, - event.message, - ); - logSourceLocation(event.sourcePath, event.keyError); + reportFor(moduleOf(event.sourcePath)).diagnostics.push({ + severity: event.fixable ? "fixable" : "error", + sourcePath: event.sourcePath, + message: event.message, + ...(event.keyError ? { keyError: true } : {}), + }); break; case "unknown-fix": - console.log( - picocolors.red("✘"), - "Unknown fix", - event.fixes, - "for", - formatLocation(event.sourcePath, event.keyError), - ); - logSourceLocation(event.sourcePath, event.keyError); + reportFor(moduleOf(event.sourcePath)).diagnostics.push({ + severity: "error", + sourcePath: event.sourcePath, + message: `Unknown fix: ${event.fixes.join(", ")}`, + ...(event.keyError ? { keyError: true } : {}), + }); break; case "unregistered-module": - console.log( - picocolors.yellow("⚠"), - `/${event.file} is not registered in val.modules - skipping`, - ); + skipped.push(event.file); + break; + case "fatal-error": + // No sourcePath for fatal errors; group by file, render message only. + reportFor(event.file).diagnostics.push({ + severity: "error", + sourcePath: event.file, + message: event.message, + }); break; case "fix-applied": console.log( @@ -178,14 +183,6 @@ export async function validate({ ); fixedFiles.add(event.file); break; - case "fatal-error": - console.log( - picocolors.red("✘"), - event.file, - "is invalid:", - event.message, - ); - break; case "remote-uploading": console.log( picocolors.yellow("⚠"), @@ -216,6 +213,76 @@ export async function validate({ } } + // Renders a module's diagnostics under a single left "│" gutter bar, with the + // file name on top and blank gutter lines for air. Output flows least- to + // most-actionable top-to-bottom, so fixable diagnostics are shown last (at the + // bottom, nearest the prompt). + const renderModule = (report: ModuleReport) => { + const bar = picocolors.dim("│"); + const diagnostics = [...report.diagnostics].sort((a, b) => + a.severity === b.severity ? 0 : a.severity === "fixable" ? 1 : -1, + ); + const fixableCount = diagnostics.filter( + (d) => d.severity === "fixable", + ).length; + const total = diagnostics.length; + const hasError = fixableCount < total; + const symbol = hasError ? picocolors.red("✘") : picocolors.yellow("⚠"); + let label: string; + if (fixableCount === total) { + label = `${fixableCount} fixable`; + } else if (fixableCount > 0) { + label = `${total} error${total > 1 ? "s" : ""} (${fixableCount} fixable)`; + } else { + label = `${total} error${total > 1 ? "s" : ""}`; + } + console.log( + `${picocolors.bold(report.file)} ${symbol} ${label} ${picocolors.dim( + `(${report.durationMs}ms)`, + )}`, + ); + for (const d of diagnostics) { + const target = d.keyError ? "key" : "value"; + const dsym = + d.severity === "fixable" + ? picocolors.yellow("⚠") + : picocolors.red("✘"); + const suffix = + d.severity === "fixable" + ? picocolors.dim(" · fixable, run --fix") + : ""; + const parts = sourcePathToLocationParts( + d.sourcePath, + projectRoot, + sourceFileCache, + target, + ); + console.log(bar); + if (parts) { + console.log( + `${bar} ${dsym} ${parts.line}:${parts.character} (${target})${suffix}`, + ); + console.log(`${bar} ${d.message}`); + } else { + console.log(`${bar} ${dsym} ${d.message}${suffix}`); + } + const frame = sourcePathToCodeFrame( + d.sourcePath, + projectRoot, + sourceFileCache, + target, + ); + if (frame !== undefined) { + console.log(bar); + for (const frameLine of frame.split("\n")) { + console.log(`${bar} ${frameLine}`); + } + } + } + console.log(bar); + console.log(""); + }; + // Run prettier on files that had fixes applied if (prettier) { for (const file of fixedFiles) { @@ -228,15 +295,66 @@ export async function validate({ } } - if (totalErrors > 0) { + // Render the grouped report least- to most-actionable, top-to-bottom, so the + // most important things end up at the bottom nearest the prompt: valid files + // and skipped modules first, then error-only modules, then fixable modules. + const allReports = [...reports.values()]; + const fixableModules = allReports.filter((r) => + r.diagnostics.some((d) => d.severity === "fixable"), + ); + const errorModules = allReports.filter( + (r) => !r.diagnostics.some((d) => d.severity === "fixable"), + ); + + for (const v of valid) { + console.log( + picocolors.green("✔"), + picocolors.dim(`${v.file} valid (${v.durationMs}ms)`), + ); + } + for (const file of skipped) { console.log( - picocolors.red("✘"), - "Got", - totalErrors, - "error" + (totalErrors > 1 ? "s" : ""), + picocolors.yellow("⚠"), + picocolors.dim(`/${file} is not registered in val.modules - skipping`), ); + } + + if (allReports.length > 0) { + console.log(""); + } + for (const report of [...errorModules, ...fixableModules]) { + renderModule(report); + } + + const fixableTotal = allReports.reduce( + (n, r) => + n + r.diagnostics.filter((d) => d.severity === "fixable").length, + 0, + ); + if (totalErrors > 0) { + let summary = `${totalErrors} error${totalErrors > 1 ? "s" : ""}`; + if (fixableTotal > 0) { + summary += ` (${fixableTotal} fixable)`; + } + summary += ` across ${allReports.length} file${ + allReports.length > 1 ? "s" : "" + }`; + if (valid.length > 0) { + summary += ` · ${valid.length} valid`; + } + if (skipped.length > 0) { + summary += ` · ${skipped.length} skipped`; + } + console.log(picocolors.red("✘"), summary); } else { - console.log(picocolors.green("✔"), "No validation errors found"); + let summary = "No validation errors found"; + if (valid.length > 0) { + summary += ` · ${valid.length} valid`; + } + if (skipped.length > 0) { + summary += ` · ${skipped.length} skipped`; + } + console.log(picocolors.green("✔"), summary); } return totalErrors; } From 6fbfd2e077e11c8944d2efb45c3e31660301e60f Mon Sep 17 00:00:00 2001 From: Fredrik Ekholdt Date: Thu, 25 Jun 2026 08:18:46 +0200 Subject: [PATCH 14/16] Make all error clickable in validation output --- packages/cli/src/validate.ts | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/packages/cli/src/validate.ts b/packages/cli/src/validate.ts index beb839aec..cdd959fcc 100644 --- a/packages/cli/src/validate.ts +++ b/packages/cli/src/validate.ts @@ -247,10 +247,6 @@ export async function validate({ d.severity === "fixable" ? picocolors.yellow("⚠") : picocolors.red("✘"); - const suffix = - d.severity === "fixable" - ? picocolors.dim(" · fixable, run --fix") - : ""; const parts = sourcePathToLocationParts( d.sourcePath, projectRoot, @@ -259,12 +255,13 @@ export async function validate({ ); console.log(bar); if (parts) { + // `file:line:col` (no key/value label) so VS Code's terminal links it. console.log( - `${bar} ${dsym} ${parts.line}:${parts.character} (${target})${suffix}`, + `${bar} ${dsym} ${parts.relativeFile}:${parts.line}:${parts.character}`, ); console.log(`${bar} ${d.message}`); } else { - console.log(`${bar} ${dsym} ${d.message}${suffix}`); + console.log(`${bar} ${dsym} ${d.message}`); } const frame = sourcePathToCodeFrame( d.sourcePath, @@ -278,6 +275,11 @@ export async function validate({ console.log(`${bar} ${frameLine}`); } } + if (d.severity === "fixable") { + console.log( + `${bar} ${picocolors.dim("→ run with --fix to apply")}`, + ); + } } console.log(bar); console.log(""); From e957647f521052a3b1c375fcb3c787fa2ab7ff60 Mon Sep 17 00:00:00 2001 From: Fredrik Ekholdt Date: Thu, 25 Jun 2026 18:08:32 +0000 Subject: [PATCH 15/16] Remove noisy test / example validation errors --- examples/next/app/blogs/[blog]/page.val.ts | 2 +- examples/next/app/page.val.ts | 2 +- examples/next/content/media.val.ts | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/examples/next/app/blogs/[blog]/page.val.ts b/examples/next/app/blogs/[blog]/page.val.ts index f9544a619..23a2ee8a4 100644 --- a/examples/next/app/blogs/[blog]/page.val.ts +++ b/examples/next/app/blogs/[blog]/page.val.ts @@ -3,7 +3,7 @@ import authorsVal from "../../../content/authors.val"; import { linkSchema } from "../../../components/link.val"; const blogSchema = s.object({ - title: s.string().maxLength(3), + title: s.string(), content: s.richtext({ inline: { a: s.route(), diff --git a/examples/next/app/page.val.ts b/examples/next/app/page.val.ts index 9415c0844..5f2b33295 100644 --- a/examples/next/app/page.val.ts +++ b/examples/next/app/page.val.ts @@ -65,7 +65,7 @@ export default c.define("/app/page.val.ts", s.router(nextAppRouter, schema), { link: "/blogs/blog1", hero: { title: "Content as code", - image: c.image("/public/val/logo_7adc7.png", { + image: c.image("/public/val/images/logo.png", { width: 944, height: 944, mimeType: "image/png", diff --git a/examples/next/content/media.val.ts b/examples/next/content/media.val.ts index ef551bce2..75b6f59ba 100644 --- a/examples/next/content/media.val.ts +++ b/examples/next/content/media.val.ts @@ -9,8 +9,8 @@ export default c.define( }), { "/public/val/images/logo.png": { - width: 800, - height: 600, + width: 944, + height: 944, mimeType: "image/png", alt: "An example image", }, From 21c430ef2c98d2bee88cd20cef84c3787c485b2d Mon Sep 17 00:00:00 2001 From: Fredrik Ekholdt Date: Sat, 27 Jun 2026 09:57:08 +0200 Subject: [PATCH 16/16] Fix broken remote files for s.images / s.files --- .../remote-gallery-upload-validation.md | 10 + .../basic-gallery-remote-existing.val.ts | 20 ++ .../basic/content/basic-gallery-remote.val.ts | 19 ++ .../basic/public/val/images-remote/image.png | Bin 0 -> 67 bytes .../basic/public/val/images-remote2/image.png | Bin 0 -> 67 bytes .../cli/src/__fixtures__/basic/val.modules.ts | 2 + packages/cli/src/runValidation.test.ts | 101 +++++++ packages/cli/src/runValidation.ts | 252 +++++++++++++----- packages/core/src/schema/files.test.ts | 38 ++- packages/core/src/schema/images.test.ts | 38 ++- packages/core/src/schema/record.ts | 16 ++ .../src/schema/validation/ValidationFix.ts | 2 + packages/server/src/createFixPatch.ts | 84 +++++- packages/shared/src/internal/ApiRoutes.ts | 2 + .../validation/partitionValidationErrors.ts | 2 + 15 files changed, 484 insertions(+), 102 deletions(-) create mode 100644 .changeset/remote-gallery-upload-validation.md create mode 100644 packages/cli/src/__fixtures__/basic/content/basic-gallery-remote-existing.val.ts create mode 100644 packages/cli/src/__fixtures__/basic/content/basic-gallery-remote.val.ts create mode 100644 packages/cli/src/__fixtures__/basic/public/val/images-remote/image.png create mode 100644 packages/cli/src/__fixtures__/basic/public/val/images-remote2/image.png diff --git a/.changeset/remote-gallery-upload-validation.md b/.changeset/remote-gallery-upload-validation.md new file mode 100644 index 000000000..4765523ac --- /dev/null +++ b/.changeset/remote-gallery-upload-validation.md @@ -0,0 +1,10 @@ +--- +"@valbuild/core": patch +"@valbuild/server": patch +"@valbuild/shared": patch +"@valbuild/cli": patch +--- + +Remote galleries (`s.images({ remote: true })` / `s.files({ remote: true })`) now report a fixable validation error when an entry is still a local path, mirroring the single-field `s.image().remote()` / `s.file().remote()` behaviour. Running the CLI with `validate --fix` uploads the file to remote storage and rewrites the record key from the local path to the remote URL, keeping the file on disk. The gallery "untracked files" check now recognizes a remote-URL key as covering its on-disk file, so kept files are not flagged. + +Adds two new validation fixes: `images:upload-remote` and `files:upload-remote`. diff --git a/packages/cli/src/__fixtures__/basic/content/basic-gallery-remote-existing.val.ts b/packages/cli/src/__fixtures__/basic/content/basic-gallery-remote-existing.val.ts new file mode 100644 index 000000000..ae186ac7b --- /dev/null +++ b/packages/cli/src/__fixtures__/basic/content/basic-gallery-remote-existing.val.ts @@ -0,0 +1,20 @@ +import { c, s } from "../val.config"; + +export default c.define( + "/content/basic-gallery-remote-existing.val.ts", + s.images({ + directory: "/public/val/images-remote2", + accept: "image/*", + remote: true, + }), + + { + "https://remote.val.build/file/p/proj123/b/01/v/1.0.0/h/abc1/f/def4/p/public/val/images-remote2/image.png": + { + width: 1, + height: 1, + mimeType: "image/png", + alt: null, + }, + }, +); diff --git a/packages/cli/src/__fixtures__/basic/content/basic-gallery-remote.val.ts b/packages/cli/src/__fixtures__/basic/content/basic-gallery-remote.val.ts new file mode 100644 index 000000000..0c0e3a0b8 --- /dev/null +++ b/packages/cli/src/__fixtures__/basic/content/basic-gallery-remote.val.ts @@ -0,0 +1,19 @@ +import { c, s } from "../val.config"; + +export default c.define( + "/content/basic-gallery-remote.val.ts", + s.images({ + directory: "/public/val/images-remote", + accept: "image/*", + remote: true, + }), + + { + "/public/val/images-remote/image.png": { + width: 1, + height: 1, + mimeType: "image/png", + alt: null, + }, + }, +); diff --git a/packages/cli/src/__fixtures__/basic/public/val/images-remote/image.png b/packages/cli/src/__fixtures__/basic/public/val/images-remote/image.png new file mode 100644 index 0000000000000000000000000000000000000000..252d9502d8573d033e633f5e377d81bebf8afd36 GIT binary patch literal 67 zcmeAS@N?(olHy`uVBq!ia0vp^j35jm7|ip2ssJf2PZ!6K3dZCFAe)JSvAC2`0?1 import("./content/basic-gallery-fail-on-non-unique-dir.val") }, { def: () => import("./content/basic-gallery-missing-tracked.val") }, { def: () => import("./content/basic-gallery-wrong-metadata.val") }, + { def: () => import("./content/basic-gallery-remote.val") }, + { def: () => import("./content/basic-gallery-remote-existing.val") }, ]); diff --git a/packages/cli/src/runValidation.test.ts b/packages/cli/src/runValidation.test.ts index 165842196..56b0b42ff 100644 --- a/packages/cli/src/runValidation.test.ts +++ b/packages/cli/src/runValidation.test.ts @@ -383,4 +383,105 @@ describe("runValidation", () => { service.dispose(); } }); + + test("reports upload-remote error for local entry in a remote gallery", async () => { + const events: ValidationEvent[] = []; + + for await (const event of runValidation({ + root: tmpDir, + fix: false, + valFiles: ["content/basic-gallery-remote.val.ts"], + project: undefined, + remote: mockRemote, + fs: createDefaultValFSHost(), + })) { + events.push(event); + } + + expect(events.at(-1)).toEqual({ + type: "summary-errors", + count: expect.any(Number), + }); + const errors = events.filter((e) => e.type === "validation-error"); + expect( + errors.some( + (e) => + "message" in e && + (e.message as string).includes("needs to be uploaded"), + ), + ).toBe(true); + }); + + test("uploads local entry in a remote gallery and rewrites the key when fix is true", async () => { + const uploadRemote: IValRemote = { + remoteHost: DEFAULT_VAL_REMOTE_HOST, + getSettings: async () => ({ + success: true, + data: { + publicProjectId: "pubproj", + remoteFileBuckets: [{ bucket: "01" }], + }, + }), + uploadFile: async () => ({ success: true }), + }; + // Upload requires a personal access token on disk. + fs.mkdirSync(path.join(tmpDir, ".val"), { recursive: true }); + fs.writeFileSync( + path.join(tmpDir, ".val", "pat.json"), + JSON.stringify({ pat: "test-pat" }), + ); + + const events: ValidationEvent[] = []; + for await (const event of runValidation({ + root: tmpDir, + fix: true, + valFiles: ["content/basic-gallery-remote.val.ts"], + project: "test/project", + remote: uploadRemote, + fs: createDefaultValFSHost(), + })) { + events.push(event); + } + + expect(events.some((e) => e.type === "remote-uploaded")).toBe(true); + expect(events.some((e) => e.type === "fix-applied")).toBe(true); + + const service = await createService(tmpDir, {}, createDefaultValFSHost()); + try { + const result = await service.get( + "/content/basic-gallery-remote.val.ts" as ModuleFilePath, + "" as ModulePath, + { source: true, schema: true, validate: false }, + ); + const source = result.source as Record; + // The local-path key is replaced by a remote URL key (file kept on disk). + expect(source).not.toHaveProperty("/public/val/images-remote/image.png"); + const keys = Object.keys(source); + expect(keys).toHaveLength(1); + expect(keys[0]).toContain("pubproj"); + expect(keys[0]).toContain("public/val/images-remote/image.png"); + } finally { + service.dispose(); + } + }); + + test("does not flag a kept-on-disk file behind a remote gallery key", async () => { + const events: ValidationEvent[] = []; + + for await (const event of runValidation({ + root: tmpDir, + fix: false, + valFiles: ["content/basic-gallery-remote-existing.val.ts"], + project: undefined, + remote: mockRemote, + fs: createDefaultValFSHost(), + })) { + events.push(event); + } + + // The file kept on disk under the remote URL key must not be reported as + // untracked, and the remote URL key must not be reported as missing. + expect(events.at(-1)).toEqual({ type: "summary-success" }); + expect(events.filter((e) => e.type === "validation-error")).toHaveLength(0); + }); }); diff --git a/packages/cli/src/runValidation.ts b/packages/cli/src/runValidation.ts index d9aafdb44..09b4ea0f8 100644 --- a/packages/cli/src/runValidation.ts +++ b/packages/cli/src/runValidation.ts @@ -181,45 +181,17 @@ export async function handleFileMetadata( return { success: true, shouldApplyPatch: true }; } -export async function handleRemoteFileUpload( +// Shared upload core used by both the single-field (handleRemoteFileUpload) +// and gallery (handleRemoteGalleryFileUpload) handlers. The two differ only in +// how they derive the local file ref, metadata and serialized image/file +// schema; everything from auth through upload is identical. +async function uploadRemoteFileCore( ctx: FixHandlerContext, + fileRef: string, + metadata: Record | undefined, + schema: SerializedImageSchema | SerializedFileSchema, ): Promise { - if (!ctx.fix) { - return { - success: false, - errorMessage: `Remote file ${ctx.sourcePath} needs to be uploaded (use --fix to upload)`, - }; - } - - const [, modulePath] = Internal.splitModuleFilePathAndModulePath( - ctx.sourcePath, - ); - - if (!ctx.valModule.source || !ctx.valModule.schema) { - return { - success: false, - errorMessage: `Could not resolve source or schema for ${ctx.sourcePath}`, - }; - } - - const resolvedRemoteFileAtSourcePath = Internal.resolvePath( - modulePath, - ctx.valModule.source, - ctx.valModule.schema, - ); - - // eslint-disable-next-line @typescript-eslint/no-explicit-any - const fileRefProp = (resolvedRemoteFileAtSourcePath.source as any)?.[ - FILE_REF_PROP - ]; - if (!fileRefProp) { - return { - success: false, - errorMessage: `Expected file to be defined at: ${ctx.sourcePath} but no file was found`, - }; - } - - const filePath = path.join(ctx.projectRoot, fileRefProp); + const filePath = path.join(ctx.projectRoot, fileRef); if (!ctx.fs.fileExists(filePath)) { return { success: false, @@ -259,26 +231,6 @@ export async function handleRemoteFileUpload( }; } - if (!resolvedRemoteFileAtSourcePath.schema) { - return { - success: false, - errorMessage: `Cannot upload remote file: schema not found for ${ctx.sourcePath}`, - }; - } - - const actualRemoteFileSource = resolvedRemoteFileAtSourcePath.source; - const fileSourceMetadata = Internal.isFile(actualRemoteFileSource) - ? actualRemoteFileSource.metadata - : undefined; - const resolveRemoteFileSchema = resolvedRemoteFileAtSourcePath.schema; - - if (!resolveRemoteFileSchema) { - return { - success: false, - errorMessage: `Could not resolve schema for remote file: ${ctx.sourcePath}`, - }; - } - const projectName = ctx.project; let publicProjectId = ctx.publicProjectId; let remoteFileBuckets = ctx.remoteFileBuckets; @@ -317,16 +269,6 @@ export async function handleRemoteFileUpload( }; } - if ( - resolveRemoteFileSchema.type !== "image" && - resolveRemoteFileSchema.type !== "file" - ) { - return { - success: false, - errorMessage: `The schema is the remote is neither image nor file: ${ctx.sourcePath}`, - }; - } - remoteFilesCounter += 1; const bucket = remoteFileBuckets[remoteFilesCounter % remoteFileBuckets.length]; @@ -361,10 +303,6 @@ export async function handleRemoteFileUpload( const fileHash = Internal.remote.getFileHash(fileBuffer); const coreVersion = Internal.VERSION.core || "unknown"; const fileExt = getFileExt(filePath); - const schema = resolveRemoteFileSchema as - | SerializedImageSchema - | SerializedFileSchema; - const metadata = fileSourceMetadata; const ref = Internal.remote.createRemoteRef(ctx.remote.remoteHost, { publicProjectId, coreVersion, @@ -399,7 +337,7 @@ export async function handleRemoteFileUpload( ctx.remoteFiles[ctx.sourcePath] = { ref, - metadata: fileSourceMetadata, + metadata, }; return { @@ -415,6 +353,156 @@ export async function handleRemoteFileUpload( }; } +export async function handleRemoteFileUpload( + ctx: FixHandlerContext, +): Promise { + if (!ctx.fix) { + return { + success: false, + errorMessage: `Remote file ${ctx.sourcePath} needs to be uploaded (use --fix to upload)`, + }; + } + + const [, modulePath] = Internal.splitModuleFilePathAndModulePath( + ctx.sourcePath, + ); + + if (!ctx.valModule.source || !ctx.valModule.schema) { + return { + success: false, + errorMessage: `Could not resolve source or schema for ${ctx.sourcePath}`, + }; + } + + const resolvedRemoteFileAtSourcePath = Internal.resolvePath( + modulePath, + ctx.valModule.source, + ctx.valModule.schema, + ); + + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const fileRefProp = (resolvedRemoteFileAtSourcePath.source as any)?.[ + FILE_REF_PROP + ]; + if (!fileRefProp) { + return { + success: false, + errorMessage: `Expected file to be defined at: ${ctx.sourcePath} but no file was found`, + }; + } + + const resolveRemoteFileSchema = resolvedRemoteFileAtSourcePath.schema; + if (!resolveRemoteFileSchema) { + return { + success: false, + errorMessage: `Cannot upload remote file: schema not found for ${ctx.sourcePath}`, + }; + } + + if ( + resolveRemoteFileSchema.type !== "image" && + resolveRemoteFileSchema.type !== "file" + ) { + return { + success: false, + errorMessage: `The schema is the remote is neither image nor file: ${ctx.sourcePath}`, + }; + } + + const actualRemoteFileSource = resolvedRemoteFileAtSourcePath.source; + const fileSourceMetadata = Internal.isFile(actualRemoteFileSource) + ? actualRemoteFileSource.metadata + : undefined; + + return uploadRemoteFileCore( + ctx, + fileRefProp, + fileSourceMetadata, + resolveRemoteFileSchema, + ); +} + +// Gallery (s.images({ remote: true }) / s.files({ remote: true })) upload. +// Unlike a single image/file field, a gallery entry is keyed by its local file +// path and the value is bare metadata (no FileSource), so we derive the file +// ref from the key and synthesize the image/file schema from the record's +// media options. +export async function handleRemoteGalleryFileUpload( + ctx: FixHandlerContext, +): Promise { + if (!ctx.fix) { + return { + success: false, + errorMessage: `Remote file ${ctx.sourcePath} needs to be uploaded (use --fix to upload)`, + }; + } + + const fix = ctx.validationError.fixes?.[0]; + const mediaType = fix === "files:upload-remote" ? "file" : "image"; + + // The gallery entry is keyed by its local file path; validateMediaKey carries + // that key as the error value. + const fileRef = ctx.validationError.value; + if (typeof fileRef !== "string") { + return { + success: false, + errorMessage: `Expected a local file path for gallery entry at ${ctx.sourcePath}`, + }; + } + + const [, modulePath] = Internal.splitModuleFilePathAndModulePath( + ctx.sourcePath, + ); + + if (!ctx.valModule.source || !ctx.valModule.schema) { + return { + success: false, + errorMessage: `Could not resolve source or schema for ${ctx.sourcePath}`, + }; + } + + const resolved = Internal.resolvePath( + modulePath, + ctx.valModule.source, + ctx.valModule.schema, + ); + const entrySource = resolved.source; + const metadata = + entrySource && + typeof entrySource === "object" && + !Array.isArray(entrySource) + ? (entrySource as Record) + : undefined; + + // The gallery item schema is an ObjectSchema, not an image/file schema, so + // synthesize the serialized image/file schema (matching how single fields + // serialize) for the remote ref's validation hash. + const moduleSchema = ctx.valModule.schema; + const accept = + moduleSchema.type === "record" ? moduleSchema.accept : undefined; + const directory = + moduleSchema.type === "record" ? moduleSchema.directory : undefined; + const schema: SerializedImageSchema | SerializedFileSchema = + mediaType === "image" + ? { + type: "image", + opt: false, + options: { + ...(accept ? { accept } : {}), + ...(directory ? { directory } : {}), + }, + } + : { + type: "file", + opt: false, + options: { + ...(accept ? { accept } : {}), + }, + }; + + return uploadRemoteFileCore(ctx, fileRef, metadata, schema); +} + export async function handleRemoteFileDownload( ctx: FixHandlerContext, ): Promise { @@ -479,6 +567,17 @@ export async function handleUniqueFolderCheck( return { success: true }; } +// Maps a gallery key to its on-disk local path. Remote galleries key uploaded +// entries by a remote URL while keeping the file on disk; everything else is +// already a local path and is returned unchanged. +function remoteKeyToLocalPath(key: string): string { + const remoteRefRes = Internal.remote.splitRemoteRef(key); + if (remoteRefRes.status === "success") { + return `/${remoteRefRes.filePath}`; + } + return key; +} + export async function handleCheckAllFiles( ctx: FixHandlerContext, ): Promise { @@ -500,7 +599,12 @@ export async function handleCheckAllFiles( errorMessage: `Could not get source for ${ctx.sourcePath}`, }; } - const trackedFiles = new Set(Object.keys(source as Record)); + // Gallery entries are keyed by their file path. Remote galleries key uploaded + // entries by a remote URL, but the file is kept on disk at its local path, so + // normalize remote-URL keys back to that local path for the on-disk checks. + const trackedFiles = new Set( + Object.keys(source as Record).map(remoteKeyToLocalPath), + ); // Check that all tracked files exist on disk const missingTrackedFiles = [...trackedFiles].filter((f) => { @@ -558,6 +662,8 @@ export const currentFixHandlers: Record< "file:add-metadata": handleFileMetadata, "image:upload-remote": handleRemoteFileUpload, "file:upload-remote": handleRemoteFileUpload, + "images:upload-remote": handleRemoteGalleryFileUpload, + "files:upload-remote": handleRemoteGalleryFileUpload, "image:download-remote": handleRemoteFileDownload, "file:download-remote": handleRemoteFileDownload, "image:check-remote": handleRemoteFileCheck, diff --git a/packages/core/src/schema/files.test.ts b/packages/core/src/schema/files.test.ts index 0d18e1db3..821229f5f 100644 --- a/packages/core/src/schema/files.test.ts +++ b/packages/core/src/schema/files.test.ts @@ -5,12 +5,15 @@ import { files, FilesEntryMetadata, SerializedFilesSchema } from "./files"; function filterCheckErrors( result: | false - | Record + | Record | undefined, ) { if (!result) return result; const checkFixes = ["files:check-unique-folder", "files:check-all-files"]; - const filtered: Record = {}; + const filtered: Record< + string, + { message: string; fixes?: string[]; value?: unknown }[] + > = {}; for (const [key, errors] of Object.entries(result)) { const nonCheck = errors.filter( (e) => !e.fixes?.some((f) => checkFixes.includes(f)), @@ -289,7 +292,7 @@ describe("FilesSchema", () => { expect(filterCheckErrors(result)).toBeFalsy(); }); - test("should accept local paths when remote is enabled", () => { + test("should flag local paths as upload-remote when remote is enabled", () => { const schema = files({ accept: "application/pdf", directory: "/public/val/documents", @@ -300,19 +303,18 @@ describe("FilesSchema", () => { }, }; const result = schema["executeValidate"]("path" as SourcePath, src); - // Should not have path errors + // A local path in a remote gallery is a fixable error (upload to remote). const filteredResult3 = filterCheckErrors(result); - if (filteredResult3) { - const errors = Object.values(filteredResult3 as object).flat(); - const hasPathError = errors.some( - (e: { message: string }) => - e.message.includes("directory") || e.message.includes("Remote"), - ); - expect(hasPathError).toBe(false); - } + expect(filteredResult3).toBeTruthy(); + const errors = Object.values(filteredResult3 as object).flat(); + const uploadError = errors.find((e: { fixes?: string[] }) => + e.fixes?.includes("files:upload-remote"), + ); + expect(uploadError).toBeTruthy(); + expect(uploadError?.value).toBe("/public/val/documents/local.pdf"); }); - test("should accept mixed remote and local when remote is enabled", () => { + test("should flag only local paths when mixing remote and local", () => { const schema = files({ accept: "application/pdf", directory: "/public/val/documents", @@ -327,7 +329,15 @@ describe("FilesSchema", () => { }, }; const result = schema["executeValidate"]("path" as SourcePath, src); - expect(filterCheckErrors(result)).toBeFalsy(); + const filtered = filterCheckErrors(result); + expect(filtered).toBeTruthy(); + const errors = Object.values(filtered as object).flat(); + // Exactly the local entry is flagged for upload; the remote URL is valid. + const uploadErrors = errors.filter((e: { fixes?: string[] }) => + e.fixes?.includes("files:upload-remote"), + ); + expect(uploadErrors).toHaveLength(1); + expect(uploadErrors[0]?.value).toBe("/public/val/documents/local.pdf"); }); test("should reject invalid remote URLs", () => { diff --git a/packages/core/src/schema/images.test.ts b/packages/core/src/schema/images.test.ts index f5af9d3e4..b568d3405 100644 --- a/packages/core/src/schema/images.test.ts +++ b/packages/core/src/schema/images.test.ts @@ -6,12 +6,15 @@ import { string } from "./string"; function filterCheckErrors( result: | false - | Record + | Record | undefined, ) { if (!result) return result; const checkFixes = ["images:check-unique-folder", "images:check-all-files"]; - const filtered: Record = {}; + const filtered: Record< + string, + { message: string; fixes?: string[]; value?: unknown }[] + > = {}; for (const [key, errors] of Object.entries(result)) { const nonCheck = errors.filter( (e) => !e.fixes?.some((f) => checkFixes.includes(f)), @@ -334,7 +337,7 @@ describe("ImagesSchema", () => { expect(filterCheckErrors(result)).toBeFalsy(); }); - test("should accept local paths when remote is enabled", () => { + test("should flag local paths as upload-remote when remote is enabled", () => { const schema = images({ accept: "image/webp", directory: "/public/val/images", @@ -348,19 +351,18 @@ describe("ImagesSchema", () => { }, }; const result = schema["executeValidate"]("path" as SourcePath, src); - // Should not have path errors + // A local path in a remote gallery is a fixable error (upload to remote). const filteredResult2 = filterCheckErrors(result); - if (filteredResult2) { - const errors = Object.values(filteredResult2 as object).flat(); - const hasPathError = errors.some( - (e: { message: string }) => - e.message.includes("directory") || e.message.includes("Remote"), - ); - expect(hasPathError).toBe(false); - } + expect(filteredResult2).toBeTruthy(); + const errors = Object.values(filteredResult2 as object).flat(); + const uploadError = errors.find((e: { fixes?: string[] }) => + e.fixes?.includes("images:upload-remote"), + ); + expect(uploadError).toBeTruthy(); + expect(uploadError?.value).toBe("/public/val/images/local.webp"); }); - test("should accept mixed remote and local when remote is enabled", () => { + test("should flag only local paths when mixing remote and local", () => { const schema = images({ accept: "image/webp", directory: "/public/val/images", @@ -381,7 +383,15 @@ describe("ImagesSchema", () => { }, }; const result = schema["executeValidate"]("path" as SourcePath, src); - expect(filterCheckErrors(result)).toBeFalsy(); + const filtered = filterCheckErrors(result); + expect(filtered).toBeTruthy(); + const errors = Object.values(filtered as object).flat(); + // Exactly the local entry is flagged for upload; the remote URL is valid. + const uploadErrors = errors.filter((e: { fixes?: string[] }) => + e.fixes?.includes("images:upload-remote"), + ); + expect(uploadErrors).toHaveLength(1); + expect(uploadErrors[0]?.value).toBe("/public/val/images/local.webp"); }); test("should reject invalid remote URLs", () => { diff --git a/packages/core/src/schema/record.ts b/packages/core/src/schema/record.ts index 7b1aa3794..d52c738b2 100644 --- a/packages/core/src/schema/record.ts +++ b/packages/core/src/schema/record.ts @@ -323,6 +323,22 @@ export class RecordSchema< ], }; } + // Local path in a remote gallery: needs to be uploaded to remote. + const uploadRemoteFix = + type === "images" + ? ("images:upload-remote" as const) + : ("files:upload-remote" as const); + return { + [path]: [ + { + message: `Expected a remote ${ + type === "images" ? "image" : "file" + }, but got a local path. Use Val tooling (CLI --fix, VS Code extension, or Val Studio) to upload it. Got: ${key}`, + value: key, + fixes: [uploadRemoteFix], + }, + ], + }; } else { // When remote is disabled, only accept local paths if (isRemoteUrl) { diff --git a/packages/core/src/schema/validation/ValidationFix.ts b/packages/core/src/schema/validation/ValidationFix.ts index 02a875e34..bb9b336f6 100644 --- a/packages/core/src/schema/validation/ValidationFix.ts +++ b/packages/core/src/schema/validation/ValidationFix.ts @@ -5,12 +5,14 @@ export const ValidationFix = [ "image:download-remote", "image:check-remote", "images:check-remote", + "images:upload-remote", "file:add-metadata", "file:check-metadata", "file:upload-remote", "file:download-remote", "file:check-remote", "files:check-remote", + "files:upload-remote", "keyof:check-keys", "router:check-route", "images:check-unique-folder", diff --git a/packages/server/src/createFixPatch.ts b/packages/server/src/createFixPatch.ts index a458b6552..9dee0876a 100644 --- a/packages/server/src/createFixPatch.ts +++ b/packages/server/src/createFixPatch.ts @@ -30,6 +30,33 @@ export type FixPatchRemainingError = ValidationError & { sourcePath?: SourcePath; }; +// Gallery entries are keyed by their file path. Remote galleries key uploaded +// entries by a remote URL while keeping the file on disk at its local path, so +// normalize a remote-URL key back to that local path for on-disk reads. +function galleryKeyToLocalPath(key: string): string { + const res = Internal.remote.splitRemoteRef(key); + return res.status === "success" ? `/${res.filePath}` : key; +} + +// Patch path of the record that holds a media entry. Media keys are file paths +// that can contain dots, which sourceToPatchPath cannot round-trip, so strip +// the (JSON-encoded) key segment off the entry source path and convert the +// remaining parent path — whose ancestors are plain object keys / array +// indices — instead. +function parentPatchPathOfMediaEntry(sourcePath: SourcePath, entryKey: string) { + const jsonKey = JSON.stringify(entryKey); + let parent: string = sourcePath; + if (parent.endsWith(jsonKey)) { + parent = parent.slice(0, parent.length - jsonKey.length); + } + if (parent.endsWith(".")) { + parent = parent.slice(0, -1); + } else if (parent.endsWith(Internal.ModuleFilePathSep)) { + parent = parent.slice(0, parent.length - Internal.ModuleFilePathSep.length); + } + return sourceToPatchPath(parent as SourcePath); +} + // TODO: find a better name? transformFixesToPatch? export async function createFixPatch( config: { projectRoot: string; remoteHost: string }, @@ -295,6 +322,58 @@ export async function createFixPatch( path: sourceToPatchPath(sourcePath), }); } + } else if ( + fix === "images:upload-remote" || + fix === "files:upload-remote" + ) { + // Gallery entry: the record is keyed by the file path, so uploading to + // remote means renaming the key from the local path to the remote URL + // (remove the old key, add the new one with the same metadata). + const remoteFile = remoteFiles[sourcePath]; + if (!remoteFile) { + remainingErrors.push({ + ...validationError, + message: + "Cannot fix local to remote gallery entry: remote file was not uploaded", + fixes: undefined, + }); + continue; + } + const metadata = remoteFile.metadata as JSONValue | undefined; + if (!metadata) { + remainingErrors.push({ + ...validationError, + message: "Failed to get metadata for remote gallery file", + fixes: undefined, + }); + continue; + } + // The entry is keyed by its (local) file path, which can contain dots + // (e.g. image.png). sourceToPatchPath can't round-trip such keys, so + // build the patch path from the parent record path plus the key (which + // validateMediaKey carries as the error value), like the UI does. + const entryKey = validationError.value; + if (typeof entryKey !== "string") { + remainingErrors.push({ + ...validationError, + message: "Cannot rewrite remote gallery entry: missing entry key", + fixes: undefined, + }); + continue; + } + const parentPath = parentPatchPathOfMediaEntry(sourcePath, entryKey); + const removePath = parentPath.concat(entryKey); + const addPath = parentPath.concat(remoteFile.ref); + if (!isNotRoot(removePath) || !isNotRoot(addPath)) { + remainingErrors.push({ + ...validationError, + message: "Cannot rewrite remote gallery entry at root path", + fixes: undefined, + }); + continue; + } + patch.push({ op: "remove", path: removePath }); + patch.push({ op: "add", path: addPath, value: metadata }); } else if ( fix === "image:download-remote" || fix === "file:download-remote" @@ -386,7 +465,10 @@ export async function createFixPatch( } const gallerySource = moduleSource as Record; for (const [entryKey, storedEntry] of Object.entries(gallerySource)) { - const filename = path.join(config.projectRoot, entryKey); + const filename = path.join( + config.projectRoot, + galleryKeyToLocalPath(entryKey), + ); let buffer: Buffer; try { buffer = fs.readFileSync(filename); diff --git a/packages/shared/src/internal/ApiRoutes.ts b/packages/shared/src/internal/ApiRoutes.ts index 7d75f5591..5fc8fc350 100644 --- a/packages/shared/src/internal/ApiRoutes.ts +++ b/packages/shared/src/internal/ApiRoutes.ts @@ -41,12 +41,14 @@ const ValidationFixZ: z.ZodSchema = z.union([ z.literal("image:upload-remote"), z.literal("image:download-remote"), z.literal("images:check-remote"), + z.literal("images:upload-remote"), z.literal("file:add-metadata"), z.literal("file:check-metadata"), z.literal("file:check-remote"), z.literal("file:upload-remote"), z.literal("file:download-remote"), z.literal("files:check-remote"), + z.literal("files:upload-remote"), z.literal("keyof:check-keys"), z.literal("router:check-route"), z.literal("images:check-unique-folder"), diff --git a/packages/ui/spa/validation/partitionValidationErrors.ts b/packages/ui/spa/validation/partitionValidationErrors.ts index 004567bbf..f5ed1ea1b 100644 --- a/packages/ui/spa/validation/partitionValidationErrors.ts +++ b/packages/ui/spa/validation/partitionValidationErrors.ts @@ -63,12 +63,14 @@ function isSkippableFixCode(fix: ValidationFix): boolean { case "image:download-remote": case "image:check-remote": case "images:check-remote": + case "images:upload-remote": case "file:add-metadata": case "file:check-metadata": case "file:upload-remote": case "file:download-remote": case "file:check-remote": case "files:check-remote": + case "files:upload-remote": case "images:check-unique-folder": case "files:check-unique-folder": case "images:check-all-files":