Skip to content

Commit

Permalink
fix: avoid invoking morphs on nested failures, fix index path seriali…
Browse files Browse the repository at this point in the history
…zation (#1193)
  • Loading branch information
ssalbdivad authored Nov 8, 2024
1 parent 07d448f commit 304c4e2
Show file tree
Hide file tree
Showing 46 changed files with 526 additions and 377 deletions.
2 changes: 1 addition & 1 deletion ark/attest/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@ark/attest",
"version": "0.25.0",
"version": "0.26.0",
"license": "MIT",
"author": {
"name": "David Blass",
Expand Down
3 changes: 1 addition & 2 deletions ark/docs/astro.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,6 @@ export default defineConfig({
vite: {
resolve: {
conditions: ["ark-ts"]
},
publicDir: "public"
}
}
})
2 changes: 1 addition & 1 deletion ark/fs/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@ark/fs",
"version": "0.21.0",
"version": "0.22.0",
"license": "MIT",
"author": {
"name": "David Blass",
Expand Down
4 changes: 0 additions & 4 deletions ark/repo/scratch.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1 @@
import { type } from "arktype"

const t = type({
foo: "string"
})
2 changes: 1 addition & 1 deletion ark/schema/constraint.ts
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ export const constraintKeyParser =
return nodes.sort((l, r) => (l.hash < r.hash ? -1 : 1)) as never
}
const child = ctx.$.node(kind, schema)
return child.hasOpenIntersection() ? [child] : (child as any)
return (child.hasOpenIntersection() ? [child] : child) as never
}

type ConstraintGroupKind = satisfy<NodeKind, "intersection" | "structure">
Expand Down
5 changes: 3 additions & 2 deletions ark/schema/node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {
includes,
isArray,
isEmptyObject,
stringifyPath,
throwError,
type Dict,
type GuardablePredicate,
Expand Down Expand Up @@ -55,7 +56,7 @@ import {
type TraverseAllows,
type TraverseApply
} from "./shared/traversal.ts"
import { isNode, pathToPropString, type arkKind } from "./shared/utils.ts"
import { isNode, type arkKind } from "./shared/utils.ts"
import type { UndeclaredKeyHandling } from "./structure/structure.ts"

export abstract class BaseNode<
Expand Down Expand Up @@ -480,7 +481,7 @@ export type FlatRef<root extends BaseRoot = BaseRoot> = {
}

export const typePathToPropString = (path: array<KeyOrKeyNode>): string =>
pathToPropString(path, {
stringifyPath(path, {
stringifyNonKey: node => node.expression
})

Expand Down
2 changes: 1 addition & 1 deletion ark/schema/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@ark/schema",
"version": "0.21.0",
"version": "0.22.0",
"license": "MIT",
"author": {
"name": "David Blass",
Expand Down
6 changes: 3 additions & 3 deletions ark/schema/shared/disjoint.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
import { isArray, throwParseError, type Key } from "@ark/util"
import { isArray, stringifyPath, throwParseError, type Key } from "@ark/util"
import type { nodeOfKind } from "../kinds.ts"
import type { BaseNode } from "../node.ts"
import type { Domain } from "../roots/domain.ts"
import type { BaseRoot } from "../roots/root.ts"
import type { Prop } from "../structure/prop.ts"
import type { BoundKind } from "./implement.ts"
import { $ark } from "./registry.ts"
import { isNode, pathToPropString } from "./utils.ts"
import { isNode } from "./utils.ts"

export interface DisjointEntry<kind extends DisjointKind = DisjointKind> {
kind: kind
Expand Down Expand Up @@ -66,7 +66,7 @@ export class Disjoint extends Array<DisjointEntry> {
describeReasons(): string {
if (this.length === 1) {
const { path, l, r } = this[0]
const pathString = pathToPropString(path)
const pathString = stringifyPath(path)
return writeUnsatisfiableExpressionError(
`Intersection${
pathString && ` at ${pathString}`
Expand Down
62 changes: 49 additions & 13 deletions ark/schema/shared/errors.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
import {
CastableBase,
ReadonlyArray,
ReadonlyPath,
append,
defineProperties,
stringifyPath,
type array,
type propwiseXor,
type show
} from "@ark/util"
Expand All @@ -10,15 +14,15 @@ import type { Prerequisite, errorContext } from "../kinds.ts"
import type { NodeKind } from "./implement.ts"
import type { StandardSchema } from "./standardSchema.ts"
import type { TraversalContext } from "./traversal.ts"
import { arkKind, pathToPropString, type TraversalPath } from "./utils.ts"
import { arkKind } from "./utils.ts"

export type ArkErrorResult = ArkError | ArkErrors

export class ArkError<
code extends ArkErrorCode = ArkErrorCode
> extends CastableBase<ArkErrorContextInput<code>> {
readonly [arkKind] = "error"
path: TraversalPath
path: ReadonlyPath
data: Prerequisite<code>
private nodeConfig: ResolvedArkConfig[code]
protected input: ArkErrorContextInput<code>
Expand All @@ -35,8 +39,11 @@ export class ArkError<
)
}
this.nodeConfig = ctx.config[this.code] as never
this.path = input.path ?? [...ctx.path]
if (input.relativePath) this.path.push(...input.relativePath)
this.path =
input.relativePath ?
new ReadonlyPath([...ctx.path, ...input.relativePath])
: input.path ? new ReadonlyPath(input.path)
: new ReadonlyPath(ctx.path)
this.data = "data" in input ? input.data : data
}

Expand All @@ -45,7 +52,7 @@ export class ArkError<
}

get propString(): string {
return pathToPropString(this.path)
return stringifyPath(this.path)
}

get expected(): string {
Expand Down Expand Up @@ -87,6 +94,8 @@ export class ArkErrors
}

byPath: Record<string, ArkError> = Object.create(null)
byAncestorPath: Record<string, ArkError[]> = Object.create(null)

count = 0
private mutable: ArkError[] = this as never

Expand All @@ -95,6 +104,19 @@ export class ArkErrors
this._add(error)
}

affectsPath(path: ReadonlyPath): boolean {
if (this.length === 0) return false

return (
// this would occur if there is an existing error at a prefix of path
// e.g. the path is ["foo", "bar"] and there is an error at ["foo"]
path.stringifyAncestors().some(s => s in this.byPath) ||
// this would occur if there is an existing error at a suffix of path
// e.g. the path is ["foo"] and there is an error at ["foo", "bar"]
path.stringify() in this.byAncestorPath
)
}

private _add(error: ArkError): void {
const existing = this.byPath[error.propString]
if (existing) {
Expand All @@ -109,25 +131,37 @@ export class ArkErrors
this.ctx
)
const existingIndex = this.indexOf(existing)
// If existing is found (which it always should be unless this was externally mutated),
// replace it with the new problem intersection. In case it isn't for whatever reason,
// just append the intersection.
this.mutable[existingIndex === -1 ? this.length : existingIndex] =
errorIntersection

this.byPath[error.propString] = errorIntersection
// add the original error here rather than the intersection
// since the intersection is reflected by the array of errors at
// this path
this.addAncestorPaths(error)
} else {
this.byPath[error.propString] = error
this.addAncestorPaths(error)
this.mutable.push(error)
}
this.count++
}

private addAncestorPaths(error: ArkError): void {
error.path.stringifyAncestors().forEach(propString => {
this.byAncestorPath[propString] = append(
this.byAncestorPath[propString],
error
)
})
}

merge(errors: ArkErrors): void {
errors.forEach(e => {
if (this.includes(e)) return
this._add(
new ArkError(
{ ...e, path: [...e.path, ...this.ctx.path] } as never,
{ ...e, path: [...this.ctx.path, ...e.path] } as never,
this.ctx
)
)
Expand All @@ -142,7 +176,6 @@ export class ArkErrors
return this.toString()
}

/** Reference to this ArkErrors array (for Standard Schema compatibility) */
get issues(): this {
return this
}
Expand All @@ -157,7 +190,7 @@ export class ArkErrors
}

export type ArkErrorsMergeOptions = {
relativePath?: TraversalPath
relativePath?: array<PropertyKey>
}

export interface DerivableErrorContext<
Expand All @@ -168,14 +201,17 @@ export interface DerivableErrorContext<
problem: string
message: string
data: Prerequisite<code>
path: TraversalPath
path: array<PropertyKey>
propString: string
}

export type DerivableErrorContextInput<
code extends ArkErrorCode = ArkErrorCode
> = Partial<DerivableErrorContext<code>> &
propwiseXor<{ path?: TraversalPath }, { relativePath?: TraversalPath }>
propwiseXor<
{ path?: array<PropertyKey> },
{ relativePath?: array<PropertyKey> }
>

export type ArkErrorCode = {
[kind in NodeKind]: errorContext<kind> extends null ? never : kind
Expand Down
4 changes: 2 additions & 2 deletions ark/schema/shared/implement.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@ import {
type Entry,
type Json,
type JsonData,
type KeySet,
type arrayIndexOf,
type entryOf,
type keySet,
type keySetOf,
type listable,
type requireKeys,
Expand Down Expand Up @@ -111,7 +111,7 @@ export type CompositeKind = Exclude<NodeKind, PrimitiveKind>

export type OrderedNodeKinds = typeof nodeKinds

export const constraintKeys: keySet<ConstraintKind> = flatMorph(
export const constraintKeys: KeySet<ConstraintKind> = flatMorph(
constraintKinds,
(i, kind) => [kind, 1] as const
)
Expand Down
44 changes: 22 additions & 22 deletions ark/schema/shared/traversal.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import type { array } from "@ark/util"
import { ReadonlyPath, type array } from "@ark/util"
import type { ResolvedArkConfig } from "../config.ts"
import type { Morph } from "../roots/morph.ts"
import {
Expand All @@ -8,10 +8,10 @@ import {
type ArkErrorContextInput,
type ArkErrorInput
} from "./errors.ts"
import { appendPropToPathString, isNode, type TraversalPath } from "./utils.ts"
import { isNode } from "./utils.ts"

export type MorphsAtPath = {
path: TraversalPath
path: ReadonlyPath
morphs: array<Morph>
}

Expand All @@ -21,7 +21,7 @@ export type BranchTraversalContext = {
}

export class TraversalContext {
path: TraversalPath = []
path: PropertyKey[] = []
queuedMorphs: MorphsAtPath[] = []
errors: ArkErrors = new ArkErrors(this)
branches: BranchTraversalContext[] = []
Expand All @@ -42,7 +42,7 @@ export class TraversalContext {

queueMorphs(morphs: array<Morph>): void {
const input: MorphsAtPath = {
path: [...this.path],
path: new ReadonlyPath(this.path),
morphs
}
if (this.currentBranch) this.currentBranch.queuedMorphs.push(input)
Expand Down Expand Up @@ -75,14 +75,13 @@ export class TraversalContext {
for (const { path, morphs } of queuedMorphs) {
// even if we already have an error, apply morphs that are not at a path
// with errors to capture potential validation errors
if (this.pathHasError(path)) continue

if (this.errors.affectsPath(path)) continue
this.applyMorphsAtPath(path, morphs)
}
}
}

private applyMorphsAtPath(path: TraversalPath, morphs: array<Morph>): void {
private applyMorphsAtPath(path: ReadonlyPath, morphs: array<Morph>): void {
const key = path.at(-1)

let parent: any
Expand All @@ -94,7 +93,7 @@ export class TraversalContext {
parent = parent[path[pathIndex]]
}

this.path = path
this.path = [...path]

for (const morph of morphs) {
const morphIsNode = isNode(morph)
Expand Down Expand Up @@ -146,19 +145,6 @@ export class TraversalContext {
return this.currentErrorCount !== 0
}

pathHasError(path: TraversalPath): boolean {
if (!this.hasError()) return false

let partialPropString: string = ""
// this.errors.byPath is null prototyped so indexing by string is safe
if (this.errors.byPath[partialPropString]) return true
for (let i = 0; i < path.length; i++) {
partialPropString = appendPropToPathString(partialPropString, path[i])
if (this.errors.byPath[partialPropString]) return true
}
return false
}

get failFast(): boolean {
return this.branches.length !== 0
}
Expand Down Expand Up @@ -210,6 +196,20 @@ export class TraversalContext {
}
}

export const traverseKey = <result>(
key: PropertyKey,
fn: () => result,
// ctx will be undefined if this node isn't context-dependent
ctx: TraversalContext | undefined
): result => {
if (!ctx) return fn()

ctx.path.push(key)
const result = fn()
ctx.path.pop()
return result
}

export type TraversalMethodsByKind<input = unknown> = {
Allows: TraverseAllows<input>
Apply: TraverseApply<input>
Expand Down
Loading

0 comments on commit 304c4e2

Please sign in to comment.