-
Notifications
You must be signed in to change notification settings - Fork 934
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: Implement standard schema interface #2258
base: master
Are you sure you want to change the base?
Changes from all commits
1846e7d
012626d
42554e4
c92aec9
7fd36c1
9ca5949
fb5e080
b7ac309
1898736
7e841ef
f640f6b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,144 @@ | ||
/** | ||
* Copied from @standard-schema/spec to avoid having a dependency on it. | ||
* https://github.com/standard-schema/standard-schema/blob/main/packages/spec/src/index.ts | ||
*/ | ||
|
||
import ValidationError from './ValidationError'; | ||
|
||
export interface StandardSchema<Input = unknown, Output = Input> { | ||
readonly '~standard': StandardSchemaProps<Input, Output>; | ||
} | ||
|
||
export interface StandardSchemaProps<Input = unknown, Output = Input> { | ||
readonly version: 1; | ||
readonly vendor: string; | ||
readonly validate: ( | ||
value: unknown, | ||
) => StandardResult<Output> | Promise<StandardResult<Output>>; | ||
readonly types?: StandardTypes<Input, Output> | undefined; | ||
} | ||
|
||
export type StandardResult<Output> = | ||
| StandardSuccessResult<Output> | ||
| StandardFailureResult; | ||
|
||
export interface StandardSuccessResult<Output> { | ||
readonly value: Output; | ||
readonly issues?: undefined; | ||
} | ||
|
||
export interface StandardFailureResult { | ||
readonly issues: ReadonlyArray<StandardIssue>; | ||
} | ||
|
||
export interface StandardIssue { | ||
readonly message: string; | ||
readonly path?: ReadonlyArray<PropertyKey | StandardPathSegment> | undefined; | ||
} | ||
|
||
export interface StandardPathSegment { | ||
readonly key: PropertyKey; | ||
} | ||
|
||
export interface StandardTypes<Input, Output> { | ||
readonly input: Input; | ||
readonly output: Output; | ||
} | ||
|
||
export function createStandardPath( | ||
path: string | undefined, | ||
): StandardIssue['path'] { | ||
if (!path?.length) { | ||
return undefined; | ||
} | ||
|
||
// Array to store the final path segments | ||
const segments: string[] = []; | ||
// Buffer for building the current segment | ||
let currentSegment = ''; | ||
// Track if we're inside square brackets (array/property access) | ||
let inBrackets = false; | ||
// Track if we're inside quotes (for property names with special chars) | ||
let inQuotes = false; | ||
|
||
for (let i = 0; i < path.length; i++) { | ||
const char = path[i]; | ||
|
||
if (char === '[' && !inQuotes) { | ||
// When entering brackets, push any accumulated segment after splitting on dots | ||
if (currentSegment) { | ||
segments.push(...currentSegment.split('.').filter(Boolean)); | ||
currentSegment = ''; | ||
} | ||
inBrackets = true; | ||
continue; | ||
} | ||
|
||
if (char === ']' && !inQuotes) { | ||
if (currentSegment) { | ||
// Handle numeric indices (e.g. arr[0]) | ||
if (/^\d+$/.test(currentSegment)) { | ||
segments.push(currentSegment); | ||
} else { | ||
// Handle quoted property names (e.g. obj["foo.bar"]) | ||
segments.push(currentSegment.replace(/^"|"$/g, '')); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i don't think yup even handles these cases, i'm not sure it makes sense to do it here, it's misleading, b/c validate will still likely treat these as actual path segments There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Right, I didn't really encounter this in all my time using |
||
} | ||
currentSegment = ''; | ||
} | ||
inBrackets = false; | ||
continue; | ||
} | ||
|
||
if (char === '"') { | ||
// Toggle quote state for handling quoted property names | ||
inQuotes = !inQuotes; | ||
continue; | ||
} | ||
|
||
if (char === '.' && !inBrackets && !inQuotes) { | ||
// On dots outside brackets/quotes, push current segment | ||
if (currentSegment) { | ||
segments.push(currentSegment); | ||
currentSegment = ''; | ||
} | ||
continue; | ||
} | ||
|
||
currentSegment += char; | ||
} | ||
|
||
// Push any remaining segment after splitting on dots | ||
if (currentSegment) { | ||
segments.push(...currentSegment.split('.').filter(Boolean)); | ||
} | ||
|
||
return segments; | ||
} | ||
|
||
export function createStandardIssues( | ||
error: ValidationError, | ||
parentPath?: string, | ||
): StandardIssue[] { | ||
const path = parentPath ? `${parentPath}.${error.path}` : error.path; | ||
|
||
return error.errors.map( | ||
(err) => | ||
({ | ||
message: err, | ||
path: createStandardPath(path), | ||
} satisfies StandardIssue), | ||
); | ||
} | ||
|
||
export function issuesFromValidationError( | ||
error: ValidationError, | ||
parentPath?: string, | ||
): StandardIssue[] { | ||
if (!error.inner?.length && error.errors.length) { | ||
return createStandardIssues(error, parentPath); | ||
} | ||
|
||
const path = parentPath ? `${parentPath}.${error.path}` : error.path; | ||
|
||
return error.inner.flatMap((err) => issuesFromValidationError(err, path)); | ||
} |
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can we add lot more diverse and exhaustive tests around this interface? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Absolutely, I'm relying on the other parts working as expected because this only adapts the interface to yup's current implementation so I was only testing that layer. Happy to add more tests, what kind of tests would you like to see? General ones for each schema? |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,105 @@ | ||
import { | ||
string, | ||
number, | ||
array, | ||
bool, | ||
object, | ||
date, | ||
mixed, | ||
tuple, | ||
lazy, | ||
} from '../src'; | ||
import type { StandardSchemaV1 } from '@standard-schema/spec'; | ||
|
||
function verifyStandardSchema<Input, Output>( | ||
schema: StandardSchemaV1<Input, Output>, | ||
) { | ||
return ( | ||
schema['~standard'].version === 1 && | ||
schema['~standard'].vendor === 'yup' && | ||
typeof schema['~standard'].validate === 'function' | ||
); | ||
} | ||
|
||
test('is compatible with standard schema', () => { | ||
expect(verifyStandardSchema(string())).toBe(true); | ||
expect(verifyStandardSchema(number())).toBe(true); | ||
expect(verifyStandardSchema(array())).toBe(true); | ||
expect(verifyStandardSchema(bool())).toBe(true); | ||
expect(verifyStandardSchema(object())).toBe(true); | ||
expect(verifyStandardSchema(date())).toBe(true); | ||
expect(verifyStandardSchema(mixed())).toBe(true); | ||
expect(verifyStandardSchema(tuple([mixed()]))).toBe(true); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is missing There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added it, along with a special test for it. |
||
expect(verifyStandardSchema(lazy(() => string()))).toBe(true); | ||
}); | ||
|
||
test('issues path is an array of property paths', async () => { | ||
const schema = object({ | ||
obj: object({ | ||
foo: string().required(), | ||
'not.obj.nested': string().required(), | ||
}).required(), | ||
arr: array( | ||
object({ | ||
foo: string().required(), | ||
'not.array.nested': string().required(), | ||
}), | ||
).required(), | ||
'not.a.field': string().required(), | ||
}); | ||
|
||
const result = await schema['~standard'].validate({ | ||
obj: { foo: '', 'not.obj.nested': '' }, | ||
arr: [{ foo: '', 'not.array.nested': '' }], | ||
}); | ||
|
||
expect(result.issues).toEqual([ | ||
{ path: ['obj', 'foo'], message: 'obj.foo is a required field' }, | ||
{ | ||
path: ['obj', 'not.obj.nested'], | ||
message: 'obj["not.obj.nested"] is a required field', | ||
}, | ||
{ path: ['arr', '0', 'foo'], message: 'arr[0].foo is a required field' }, | ||
{ | ||
path: ['arr', '0', 'not.array.nested'], | ||
message: 'arr[0]["not.array.nested"] is a required field', | ||
}, | ||
{ path: ['not.a.field'], message: '["not.a.field"] is a required field' }, | ||
]); | ||
}); | ||
|
||
test('should clone correctly when using modifiers', async () => { | ||
const schema = string().required(); | ||
|
||
const result = await schema['~standard'].validate(''); | ||
|
||
expect(result.issues).toEqual([ | ||
{ path: undefined, message: 'this is a required field' }, | ||
]); | ||
}); | ||
|
||
test('should work correctly with lazy schemas', async () => { | ||
let isNumber = false; | ||
const schema = lazy(() => { | ||
if (isNumber) { | ||
return number().min(10); | ||
} | ||
|
||
return string().required().min(12); | ||
}); | ||
|
||
const result = await schema['~standard'].validate(''); | ||
|
||
expect(result.issues).toEqual([ | ||
{ path: undefined, message: 'this is a required field' }, | ||
{ path: undefined, message: 'this must be at least 12 characters' }, | ||
]); | ||
|
||
isNumber = true; | ||
|
||
const result2 = await schema['~standard'].validate(5); | ||
|
||
expect(result2.issues).toEqual([ | ||
{ path: undefined, message: 'this must be greater than or equal to 10' }, | ||
]); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what is going on with path logic? There are already utilities for splitting and traversing yup's path's in utils, see the
reach
utilityThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here I'm not traversing the schema path which is what I think
reach
andgetIn
are for?What I'm doing here is I'm converting error path from as single string value to an array of strings/symbols which is what the standard schema interface requires.
For example these are the output paths from ValidationErrors and next to them is how they should look like in standard schema:
I scanned the codebase quickly, do we have utilities for this kind of thing?