-
-
Notifications
You must be signed in to change notification settings - Fork 172
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
Align JSON Schema with TSON.is()
validation checks
#290
Conversation
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.
Look, above image is my README content and is()
function allows superfluous properties. Such superfluous properties are even allowed in the TypeScript compiler. About the addtionalProperties: false
option, haven't I implemented and benchmarked through equals()
function with TypeBoxObjectUnionImplicitEquals
DTO? Please stop trying endless request about that addtionalProperties: false
option.
interface IMember {
id: string;
name: string;
}
const elem = {
id: "some-id",
name: "some-name",
account: "some-account", // superfluous property
};
const member: IMember = elem; // possible
Also, about the #258 (comment), is there any reason that do not separating object type as independent component? In my case, I'm utilizing the JSON schema type to automating frontend deveolopment like below (would be published soon). Therefore, stop separating object type would be a cautious subject for me. |
@samchon Appreciated, but in order for this test to be run, you NEED Allow ALL Superfluous Propertiesconst Directory = (bucket: TSchema) => Type.Object({
id: Type.Number(),
name: Type.String(),
path: Type.String(),
children: Type.Array(bucket),
});
const SharedDirectory = (bucket: TSchema) => Type.Object({
id: Type.Number(),
name: Type.String(),
path: Type.String(),
children: Type.Array(bucket),
access: Type.Union([ // 'read' or 'write' ONLY
Type.Literal("read"),
Type.Literal("write")
]),
});
const Union = Type.Union([Directory, SharedDirectory])
const Result = Union.Check({ // const Result = true ????? wrong...
id: 1,
name: 'folder',
path: '/test/folder',
children: [],
access: 'foobar' // <---- wait, this shouldn't be allowed !!!!
}) ExplanationThe reason this test fails is because To further elaborate, consider the following tests run in isolation. Directory.Check({
id: 1,
name: 'folder',
path: '/test/folder',
children: [],
access: 'foobar' // <---- Ok (additional property allowed)
})
SharedDirectory.Check([
id: 1,
name: 'folder',
path: '/test/folder',
children: [],
access: 'foobar' // <----Fail (not allowed)
])
Union.Check([
id: 1,
name: 'folder',
path: '/test/folder',
children: [],
access: 'foobar' // <---- Ok (because Directory is Ok) (so this test fails)
]) I think this is what your union specialization algorithm is trying to accommodate for, but JSON Schema would outright treat this as ambiguous (and require an Will submit an update to remove |
Refer to bd119aa for closest JSON schema approximation while allowing for superfluous properties on |
Yes, that's fair. if it's not something on the TSON roadmap, no problem. |
…his needs review)
@samchon Have fixed up
So, I've looked at the failing tests, and honestly, I'm having a difficult time making heads or tails out of what data is being run for what schema, or what the validation constraints should be (and I really don't have much time to look any deeper I'm afraid). However, I have swapped the failing schemas for
With this said, all the TB tests are passing and the Ajv tests could be made to pass with an equivalent set Will leave this PR here for review, If the updates are not acceptable, I'm happy to have this issue closed off (but would advice keeping the Cheers |
Going to close this PR and consider these schemas non-reconcilable. Good luck with your project. |
Enhance assert benchmark program for #290
This PR updates 4 schemas for Ajv (1) and TypeBox (3) to attain better alignment for the internal validation rules implemented for
TSON.is()
. This PR also updates TypeBox from0.24.52
to0.25.2
which has since included additional support foradditionalProperties: TSchema
(which enables extended support for{ __cache: undefined }
additional properties).Note:
additionalProperties: { ...schema }
is recognized by the JSON Schema specification, howeveradditionalProperties: { type: 'undefined' }
is not. The TypeCompiler provides support through extension types, but would be discouraged in any typical implementation. This may have implication forTSON.application<T, 'ajv'>
as there would be no way to produce an equivalent schema that would be understood by Ajv. For consideration.This PR is a follow on from this query #289 (comment)
This should get TypeBox and Ajv as close to inline with the current test suites and spoiler data.
Submitting for Review