From 1c503053c3115da0ca194892ffb653c7b897d805 Mon Sep 17 00:00:00 2001 From: Fredrik Ekholdt Date: Wed, 24 Jun 2026 21:25:44 +0200 Subject: [PATCH 01/12] 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 02/12] 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 03/12] 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 04/12] 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 05/12] 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 06/12] 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 07/12] 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 08/12] 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 09/12] 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 10/12] 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 e43ded1fbbd3442b31fa6152d30939706ebe0614 Mon Sep 17 00:00:00 2001 From: Fredrik Ekholdt Date: Sat, 27 Jun 2026 10:21:38 +0200 Subject: [PATCH 11/12] Drop no-op source/schema flags from Service.get options After moving to in-memory val.modules extraction, Service.get only honors `validate`; `source`/`schema` were accepted but ignored. Remove them from the options type and update all call sites so the API is honest. Co-Authored-By: Claude Opus 4.8 --- packages/cli/src/listUnusedFiles.ts | 2 -- packages/cli/src/runValidation.test.ts | 6 +++--- packages/cli/src/runValidation.ts | 6 +----- packages/server/src/Service.ts | 4 ++-- 4 files changed, 6 insertions(+), 12 deletions(-) diff --git a/packages/cli/src/listUnusedFiles.ts b/packages/cli/src/listUnusedFiles.ts index c8bef1f50..bb80866e1 100644 --- a/packages/cli/src/listUnusedFiles.ts +++ b/packages/cli/src/listUnusedFiles.ts @@ -39,8 +39,6 @@ export async function listUnusedFiles({ root }: { root?: string }) { } const valModule = await service.get(moduleId, "" as ModulePath, { validate: true, - source: true, - schema: true, }); // TODO: not sure using validation is the best way to do this, but it works currently. if (valModule.errors) { diff --git a/packages/cli/src/runValidation.test.ts b/packages/cli/src/runValidation.test.ts index 165842196..19edf251a 100644 --- a/packages/cli/src/runValidation.test.ts +++ b/packages/cli/src/runValidation.test.ts @@ -270,7 +270,7 @@ describe("runValidation", () => { const result = await service.get( "/content/basic-gallery-missing-tracked.val.ts" as ModuleFilePath, "" as ModulePath, - { source: true, schema: true, validate: true }, + { validate: true }, ); expect(result.source).not.toHaveProperty( "/public/val/images4/missing.png", @@ -327,7 +327,7 @@ describe("runValidation", () => { const result = await service.get( "/content/basic-gallery-wrong-metadata.val.ts" as ModuleFilePath, "" as ModulePath, - { source: true, schema: true, validate: true }, + { validate: true }, ); expect(result.source).toMatchObject({ "/public/val/images3/image.png": { @@ -361,7 +361,7 @@ describe("runValidation", () => { const result = await service.get( "/content/basic-image.val.ts" as ModuleFilePath, "" as ModulePath, - { source: true, schema: true, validate: true }, + { validate: true }, ); // The schema always emits image:check-metadata when metadata exists // (actual metadata verification happens in the fix handler). diff --git a/packages/cli/src/runValidation.ts b/packages/cli/src/runValidation.ts index d9aafdb44..791d2c5c2 100644 --- a/packages/cli/src/runValidation.ts +++ b/packages/cli/src/runValidation.ts @@ -457,7 +457,7 @@ export async function handleUniqueFolderCheck( const otherModule = await ctx.service.get( otherModuleFilePath, "" as ModulePath, - { source: false, schema: true, validate: false }, + { validate: false }, ); const schema = otherModule.schema as | { type?: string; directory?: string; mediaType?: string } @@ -631,8 +631,6 @@ export async function* runValidation({ const snapshot: SchemaSourceSnapshot = { schemas: {}, sources: {} }; for (const moduleFilePath of registered) { const valModule = await service.get(moduleFilePath, "" as ModulePath, { - source: true, - schema: true, validate: false, }); if (valModule.schema) { @@ -651,8 +649,6 @@ export async function* runValidation({ } const start = Date.now(); const valModule = await service.get(moduleFilePath, "" as ModulePath, { - source: true, - schema: true, validate: true, }); const remoteFiles: Record< diff --git a/packages/server/src/Service.ts b/packages/server/src/Service.ts index 514b7aa29..66bc436a6 100644 --- a/packages/server/src/Service.ts +++ b/packages/server/src/Service.ts @@ -86,9 +86,9 @@ export class Service { async get( moduleFilePath: ModuleFilePath, modulePath: ModulePath, - options?: { validate: boolean; source: boolean; schema: boolean }, + options?: { validate: boolean }, ): Promise { - const opts = options ?? { validate: true, source: true, schema: true }; + const opts = options ?? { validate: true }; const source = this.extracted.sources[moduleFilePath] as Source | undefined; const schema = this.extracted.schemas[moduleFilePath] as | Schema From 94ce8a06940681ec231fd6d0d3aa523237bd5224 Mon Sep 17 00:00:00 2001 From: Fredrik Ekholdt Date: Sat, 27 Jun 2026 10:21:38 +0200 Subject: [PATCH 12/12] Fix misleading modulePathMap TODO and document loadValModules trust boundary - Remove the dangling/inaccurate "+1" TODO in modulePathMap (no +1 happens). - Add a SECURITY note to loadValModules clarifying the vm context is not a sandbox and must only evaluate trusted first-party project files. Co-Authored-By: Claude Opus 4.8 --- packages/server/src/loadValModules.ts | 8 ++++++++ packages/server/src/modulePathMap.ts | 2 +- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/packages/server/src/loadValModules.ts b/packages/server/src/loadValModules.ts index 64c3e0a33..51463a1b7 100644 --- a/packages/server/src/loadValModules.ts +++ b/packages/server/src/loadValModules.ts @@ -18,6 +18,14 @@ import { getCompilerOptions } from "./getCompilerOptions"; * `extractValModules` uses. * * Mirrors the pattern already used by the CLI's `evalValConfigFile`. + * + * SECURITY: The `vm` context is NOT a security sandbox. It deliberately exposes + * `process` and a `require` that falls back to the real Node resolver (so user + * modules share the same `@valbuild/core` instance). This loader must therefore + * only ever be used to evaluate the project's own first-party, trusted files + * (`val.modules` and the local `*.val.ts`/`val.config.ts` it imports) — i.e. the + * same trust level as running the project's build. It must never be used to + * evaluate untrusted or third-party modules. */ export function loadValModules(projectRoot: string): ValModules { const valModulesPath = findValModulesPath(projectRoot); diff --git a/packages/server/src/modulePathMap.ts b/packages/server/src/modulePathMap.ts index 604cf6567..ca4025fce 100644 --- a/packages/server/src/modulePathMap.ts +++ b/packages/server/src/modulePathMap.ts @@ -136,7 +136,7 @@ function traverseCallExpression( 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]) {