arktype
arktype copied to clipboard
Use the result with the shortest path when finalizing errors for a cyclic value
A Boston TS attendee (please take credit if you see this) suggested this could be optimized using a breadth-first approach to traversal.
While changing the traversal pattern would likely incur a significant performance cost, for cyclic types, we already need to keep track of a map of which values have been checked by which types.
Currently, if the combination we're checking has already been checked (whether valid or not), we don't recheck it. We still don't need to recheck the value, but when this occurs, we could update the error path if the one we just encountered is shorter than the one that is stored.
In the example at https://arktype.io/docs/scopes/, , we should expect the following improvement:
❌ problems:
dependencies/0/dependencies/0/contributors/0/email must be a valid email (was 'david@sharktypeio')
contributors/0/email must be a valid email (was 'david@sharktypeio')
❌ problems:
contributors/0/email must be a valid email (was 'david@sharktypeio')
I started working on implementing this but it was more complex and had more perf cost than anticipated to remap the errors. I'm going to revert this for now, here's what I started with in case someone has a better solution and wants to pick it up:
on TraversalContext:
// Each cyclic alias adds objects it encounters to this map with a
// corresponding value of the shortest path at which the value was
// encountered. This allows us to include only the error message
// corresponding to the shortest path.
seen: { [name in string]?: Map<unknown, TraversalPath> } = {}
cyclicAliases: string[] = []
traverseAlias<traverseResolution extends TraversalMethod>(
alias: string,
traverseResolution: traverseResolution,
data: unknown
): ReturnType<traverseResolution> {
const seen: Map<unknown, TraversalPath> = (this.seen[alias] ??= new Map())
const seenPath = seen.get(data)
if (seenPath === undefined) {
// if this is the first time the cyclic type has checked a reference
// to data, initialize to the current path
seen.set(data, [...this.path])
this.cyclicAliases.push(alias)
traverseResolution(data, this) as never
this.cyclicAliases.pop()
} else if (this.path.length < seenPath.length) {
// if we've already validated the data but at a longer path,
// transform any corresponding errors to use the current path
this.errors.forEach(e => {
if (e.cyclicAliases.includes(alias))
e.path = [...this.path, ...e.path.slice(seenPath.length)]
})
seen.set(data, [...this.path])
}
// otherwise, the data has already been validated at a location at least as shallow,
// so we don't need to validate anything else.
return true as never
}
Errors also had cyclicAliases
attached.