Skip to content
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

Developing #257 - spoiler functions are reusable #258

Merged
merged 4 commits into from
Oct 23, 2022
Merged

Conversation

samchon
Copy link
Owner

@samchon samchon commented Oct 22, 2022

@sinclairzx81 Reviewing your special logic descriminating union type through Object.keys().length, no matther much I think about it, it dosn't make sense. The logic can be easily broken by object construction with rest parameters, commonly used by many TypeScript developers.

Therefore, I added a logic breaking your TypeBoxObjectUnionImplicit type validator. Until you do not implement special algorithm for specializing union type, I can prepare inifite real use-cases which can break the additionalProperties: false option. I hope you to consider such environments and TypeBox to be enhanced much powerfully.

Also, the benchmark is comparing is() function. The additionalProperties: false option is suitable for equals() function. In such reasons, I broke TypeBoxObjectUnionImplicit and TypeBoxUltimateUnion type and the additionalProperties: false would be removed soon.

Instead, I recommend you to run equals() benchmark and it would be much faster than mine.

const obj = {
    ...xo,
    ...yo,
};

@samchon samchon added the enhancement New feature or request label Oct 22, 2022
@samchon samchon self-assigned this Oct 22, 2022
@sinclairzx81
Copy link
Contributor

Reviewing your special logic descriminating union type through Object.keys().length, no matther much I think about it, it dosn't make sense.

There is no Object.keys() used in union validation (and JSON Schema has no concept of discriminated unions). Which Object.keys().length are you referring to?

The logic can be easily broken by object construction with rest parameters

Can you please provide a repro for the breaking case using rest parameters?

@samchon
Copy link
Owner Author

samchon commented Oct 22, 2022

function check(value) {
    return (
        (typeof value === "object" &&
            value !== null &&
            !Array.isArray(value) &&
            Object.keys(value).length === 2 &&
            typeof value.x === "number" &&
            typeof value.y === "number") ||
        (typeof value === "object" &&
            value !== null &&
            !Array.isArray(value) &&
            Object.keys(value).length === 3 &&
            typeof value.x === "number" &&
            typeof value.y === "number" &&
            typeof value.z === "number")
    );
}

About damaging data, I've started from assigning undefined value, but such damaging is also possible with non-undefined value. As you know, until TypeBox is determining sub-type through Object.keys().length, such breaking very easy and even when optional property exists, it's also easy to damage, too.

https://github.com/samchon/typescript-json/blob/b38aa154a7e7d8610ff0545df3b555957cda1d56/test/structures/ObjectUnionImplicit.ts#L62-L64

@sinclairzx81
Copy link
Contributor

sinclairzx81 commented Oct 23, 2022

As you know, until TypeBox is determining sub-type through Object.keys().length

This is the compiled validation routine. Object.keys().length is an optimization heuristic for additionalProperties: false objects only. It works because the subsequent checks (x and y) must pass for the expression to evaluate true. I don't believe this to be a bug.

{ 
     __cache: undefined, 
}

If you compare both TypeCompiler and Ajv, you will see they will both produce the same result. Specifying additionalProperties: false means no extra properties, and the spoiler data is passing an additional property __cache: undefined causing the test to fail (which is expected). Again, this isn't really a bug and TypeScript has the exact same behavior.

TypeScript Link Here

function test(value: { x: number, y: number }) {}

test({
  x: 1,
  y: 2,
  __cache: undefined // error!
})

// Argument of type '{ x: number; y: number; __cache: undefined; }' is not assignable to 
// parameter of type '{ x: number; y: number; }'. Object literal may only specify known 
// properties, and '__cache' does not exist in type '{ x: number; y: number; }'

Are you sure that TSON should be permissive of the __cache: undefined property?

@samchon
Copy link
Owner Author

samchon commented Oct 23, 2022

const suprlus = {
    x: 3,
    y: 4,
    note: "A 2d point"
};
const p: Point2D = surplus; // possible
TSON.is<Point2D>(surplus); // true
TSON.is<Point2D>(p); // true

TSON.is<Point2D>({
    x: 1,
    y: 2,
    ...{
        surplus: "proeprty"
    }
}); // true and can be Point2D type

TypeScript also can possible to assign surplus property like above.

Also, using additionalProperties: false on ObjectUnionImplicit and UltimateUnion types is not me, but you. It was your choice. I'm saying that such additionalProperties: false assignment for is() function can be very easily broken. If you're targetting to the equals() function, such Object.keys().length statement with additionalProperties: false option can be suitable, whether how to handle undefined assigned property.

However, this is is() function allowing superfluous properties.

@sinclairzx81
Copy link
Contributor

sinclairzx81 commented Oct 23, 2022

Also, using additionalProperties: false on ObjectUnionImplicit and UltimateUnion types is not me, but you. It was your choice. I'm saying that such additionalProperties: false assignment for is() function can be very easily broken. If you're targetting to the equals() function, such Object.keys().length statement with additionalProperties: false option can be suitable, whether how to handle undefined assigned property.

Sure, If equals and is uses different policies, then you would need to encode each policy in separate schemas. But which policy should TSON.application<T, 'ajv'> generate?

@samchon
Copy link
Owner Author

samchon commented Oct 23, 2022

My json schema generator does not support the additionalProperties: false type yet.

@samchon
Copy link
Owner Author

samchon commented Oct 23, 2022

In the previous PR of yours (#256), I merged it without deep consideration. It is my mistake, but I repeatedly asked you that really is it okay and you told me that it is okay. At that scene, I told only undefined value assigned case. However, learning and reviewing your project, I undetstood that what I have missed.

I am sorry for late understanding. I may got it too late because I was impressed by hyper performance shown in the TypeboxObjectUnionImplicit type and tried to learn from your repo before validating the previous PR. I had to validate it first.

@samchon samchon merged commit d5616ea into master Oct 23, 2022
@sinclairzx81
Copy link
Contributor

sinclairzx81 commented Oct 23, 2022

It is my mistake, but I repeatedly asked you that really is it okay and you told me that it is okay.

Well, the schemas were fine until you updated the tests afterwards to invalidate them. While I think it's good to be able to identify where disparities in validation logic exist (and TSON being permissive of additional properties with undefined values is a big disparity), if the goal is to benchmark, why not benchmark with working schematics operating over consistent data so results are possible?

I mean, Ajv can produce results for those failing tests, but has been disqualified on a technicality unrelated to the actual test being run. it's a bit of a shame, but oh well.

I am sorry for late understanding. I may got it too late because I was impressed by hyper performance shown in the TypeboxObjectUnionImplicit type and tried to learn from your repo before validating the previous PR. I had to validate it first.

Yes, That's fine. And you're welcome to use any of TypeBox's optimizations in TSON if you find them useful.

All the best!
S

@sinclairzx81
Copy link
Contributor

Heya, I've had a look at fixing up these benchmarks. Unfortunately, only Ajv's AjvArrayRecursiveUnionImplicit appears resolvable due to the __cache: undefined requirement. The TB updates do require the latest version to support non-boolean values on additionalProperties, but should be working fine under the current spec.

Ajv

AjvArrayRecursiveUnionImplicit.ts
import { TSchema, Type } from "@sinclair/typebox";
import Ajv from "ajv";

const ImageFile = Type.Object(
    {
        id: Type.Number(),
        name: Type.String(),
        path: Type.String(),
        width: Type.Number(),
        height: Type.Number(),
        url: Type.String(),
        size: Type.Number(),
    },
    {
        additionalProperties: false,
    },
);

const TextFile = Type.Object(
    {
        id: Type.Number(),
        name: Type.String(),
        path: Type.String(),
        size: Type.Number(),
        content: Type.String(),
    },
    {
        additionalProperties: false,
    },
);

const ZipFile = Type.Object(
    {
        id: Type.Number(),
        name: Type.String(),
        path: Type.String(),
        size: Type.Number(),
        count: Type.Number(),
    },
    {
        additionalProperties: false,
    },
);

const Shortcut = <T extends TSchema>(bucket: T) =>
    Type.Object(
        {
            id: Type.Number(),
            name: Type.String(),
            path: Type.String(),
            target: bucket,
        },
        {
            additionalProperties: false,
        },
    );

const Directory = (bucket: TSchema) =>
    Type.Object(
        {
            id: Type.Number(),
            name: Type.String(),
            path: Type.String(),
            children: Type.Array(bucket),
        },
        {
            additionalProperties: false,
        },
    );

const SharedDirectory = (bucket: TSchema) =>
    Type.Object(
        {
            id: Type.Number(),
            name: Type.String(),
            path: Type.String(),
            children: Type.Array(bucket),
            access: Type.Union([Type.Literal("read"), Type.Literal("write")]),
        },
        {
            additionalProperties: false,
        },
    );

const Bucket = Type.Recursive((bucket) =>
    Type.Union([
        ImageFile,
        TextFile,
        ZipFile,
        Shortcut(bucket),
        Directory(bucket),
        SharedDirectory(bucket),
    ]),
);

export const __AjvArrayRecursiveUnionImplicit = Type.Array(Bucket);
const ajv = new Ajv();
const validate = ajv.compile(__AjvArrayRecursiveUnionImplicit);
export const AjvArrayRecursiveUnionImplicit = {
    Check: (input: unknown) => validate(input) as boolean,
};

TypeBox

npm install @sinclair/[email protected]

TypeBoxArrayRecursiveUnionImplicit.ts
import { TSchema, Type } from "@sinclair/typebox";
import { TypeCompiler } from "@sinclair/typebox/compiler";

const ImageFile = Type.Object(
    {
        id: Type.Number(),
        name: Type.String(),
        path: Type.String(),
        width: Type.Number(),
        height: Type.Number(),
        url: Type.String(),
        size: Type.Number(),
    },
    {
        additionalProperties: false,
    },
);

const TextFile = Type.Object(
    {
        id: Type.Number(),
        name: Type.String(),
        path: Type.String(),
        size: Type.Number(),
        content: Type.String(),
    },
    {
        additionalProperties: false,
    },
);

const ZipFile = Type.Object(
    {
        id: Type.Number(),
        name: Type.String(),
        path: Type.String(),
        size: Type.Number(),
        count: Type.Number(),
    },
    {
        additionalProperties: false,
    },
);

const Shortcut = <T extends TSchema>(bucket: T) =>
    Type.Object(
        {
            id: Type.Number(),
            name: Type.String(),
            path: Type.String(),
            target: bucket,
        },
        {
            additionalProperties: false,
        },
    );

const Directory = (bucket: TSchema) =>
    Type.Object(
        {
            id: Type.Number(),
            name: Type.String(),
            path: Type.String(),
            children: Type.Array(bucket),
        },
        {
            additionalProperties: false,
        },
    );

const SharedDirectory = (bucket: TSchema) =>
    Type.Object(
        {
            id: Type.Number(),
            name: Type.String(),
            path: Type.String(),
            children: Type.Array(bucket),
            access: Type.Union([Type.Literal("read"), Type.Literal("write")]),
        },
        {
            additionalProperties: false,
        },
    );

const Bucket = Type.Recursive((bucket) =>
    Type.Union([
        ImageFile,
        TextFile,
        ZipFile,
        Shortcut(bucket),
        Directory(bucket),
        SharedDirectory(bucket),
    ]),
);

export const __TypeBoxArrayRecursiveUnionImplicit = Type.Array(Bucket);
export const TypeBoxArrayRecursiveUnionImplicit = TypeCompiler.Compile(
    __TypeBoxArrayRecursiveUnionImplicit,
);
TypeBoxObjectUnionImplicit.ts
import { Type } from "@sinclair/typebox";
import { TypeCompiler } from "@sinclair/typebox/compiler";

const Point = Type.Object(
    {
        x: Type.Number(),
        y: Type.Number(),
        slope: Type.Optional(Type.Union([Type.Null(), Type.Number()])),
    },
    { additionalProperties: Type.Undefined() },
);
const Line = Type.Object(
    {
        p1: Point,
        p2: Point,
        distance: Type.Optional(Type.Union([Type.Null(), Type.Number()])),
    },
    { additionalProperties: Type.Undefined() },
);
const Triangle = Type.Object(
    {
        p1: Point,
        p2: Point,
        p3: Point,
        width: Type.Optional(Type.Union([Type.Null(), Type.Number()])),
        height: Type.Optional(Type.Union([Type.Null(), Type.Number()])),
        area: Type.Optional(Type.Union([Type.Null(), Type.Number()])),
    },
    { additionalProperties: Type.Undefined() },
);
const Rectangle = Type.Object(
    {
        p1: Point,
        p2: Point,
        p3: Point,
        p4: Point,
        width: Type.Optional(Type.Union([Type.Null(), Type.Number()])),
        height: Type.Optional(Type.Union([Type.Null(), Type.Number()])),
        area: Type.Optional(Type.Union([Type.Null(), Type.Number()])),
    },
    { additionalProperties: Type.Undefined() },
);
const Polyline = Type.Object(
    {
        points: Type.Array(Point),
        length: Type.Optional(Type.Union([Type.Null(), Type.Number()])),
    },
    { additionalProperties: Type.Undefined() },
);
const Polygon = Type.Object(
    {
        outer: Polyline,
        inner: Type.Optional(Type.Array(Polyline)),
        area: Type.Optional(Type.Union([Type.Null(), Type.Number()])),
    },
    { additionalProperties: Type.Undefined() },
);
const Circle = Type.Object(
    {
        centroid: Type.Optional(Point),
        radius: Type.Number(),
        area: Type.Optional(Type.Union([Type.Null(), Type.Number()])),
    },
    { additionalProperties: Type.Undefined() },
);

const Union = Type.Union([
    Point,
    Line,
    Triangle,
    Rectangle,
    Polyline,
    Polygon,
    Circle,
]);

export const __TypeBoxObjectUnionImplicit = Type.Array(Union);
export const TypeBoxObjectUnionImplicit = TypeCompiler.Compile(
    __TypeBoxObjectUnionImplicit,
);
TypeBoxUltimateUnion.ts
import { TSchema, Type } from "@sinclair/typebox";
import { TypeCompiler } from "@sinclair/typebox/compiler";

const Attribute = {
    description: Type.Optional(Type.String()),
    "x-tson-metaTags": Type.Optional(
        Type.Array(
            Type.Object({
                // @todo - must be specified, but too hard
                kind: Type.String(),
            }),
        ),
    ),
    "x-tson-jsDocTags": Type.Optional(
        Type.Array(
            Type.Object({
                name: Type.String(),
                text: Type.Optional(
                    Type.Array(
                        Type.Object({
                            text: Type.String(),
                            kind: Type.String(),
                        }),
                    ),
                ),
            }),
        ),
    ),
};

const Unknown = Type.Object(
    {
        ...Attribute,
    },
    { additionalProperties: Type.Undefined() },
);
const NullOnly = Type.Object(
    {
        type: Type.Literal("null"),
        ...Attribute,
    },
    { additionalProperties: Type.Undefined() },
);

const Atomic = (literal: string, type: () => any) => {
    return Type.Object(
        {
            type: Type.Literal(literal),
            nullable: Type.Boolean(),
            default: Type.Optional(type()),
            ...Attribute,
        },
        { additionalProperties: Type.Undefined() },
    );
};

const Constant = (literal: string, type: () => any) =>
    Type.Intersect(
        [
            Atomic(literal, type),
            Type.Object({
                enum: Type.Array(type()),
            }),
        ],
        { additionalProperties: Type.Undefined() },
    );

const Array = <T extends TSchema>(schema: T) =>
    Type.Object(
        {
            type: Type.Literal("array"),
            items: schema,
            nullable: Type.Boolean(),
            ...Attribute,
        },
        { additionalProperties: Type.Undefined() },
    );

const Tuple = <T extends TSchema>(schema: T) =>
    Type.Object(
        {
            type: Type.Literal("array"),
            items: Type.Array(schema),
            nullable: Type.Boolean(),
            ...Attribute,
        },
        { additionalProperties: Type.Undefined() },
    );

const Reference = Type.Object(
    {
        $ref: Type.String(),
        ...Attribute,
    },
    { additionalProperties: Type.Undefined() },
);

const RecursiveReference = Type.Object(
    {
        $recursiveRef: Type.String(),
        ...Attribute,
    },
    { additionalProperties: Type.Undefined() },
);

const OneOf = <T extends TSchema>(schema: T) =>
    Type.Object(
        {
            oneOf: Type.Array(schema),
            ...Attribute,
        },
        { additionalProperties: Type.Undefined() },
    );

const ObjectDef = <T extends TSchema>(schema: T) =>
    Type.Object(
        {
            $id: Type.Optional(Type.String()),
            type: Type.Literal("object"),
            nullable: Type.Boolean(),
            properties: Type.Record(Type.String(), schema),
            patternProperties: Type.Optional(
                Type.Record(Type.String(), schema),
            ),
            additionalProperties: Type.Optional(schema),
            required: Type.Optional(Type.Array(Type.String())),
            description: Type.Optional(Type.String()),
            "x-tson_jsDocTags": Type.Optional(Type.Array(Type.Any())),
            $recursiveAnchor: Type.Optional(Type.Boolean()),
        },
        { additionalProperties: Type.Undefined() },
    );

const Components = <T extends TSchema>(schema: T) =>
    Type.Object(
        {
            schemas: Type.Record(Type.String(), ObjectDef(schema)),
        },
        { additionalProperties: Type.Undefined() },
    );

const Application = <T extends TSchema>(schema: T) =>
    Type.Object(
        {
            schemas: Type.Array(schema),
            components: Components(schema),
            purpose: Type.Union([Type.Literal("swagger"), Type.Literal("ajv")]),
            prefix: Type.String(),
        },
        { additionalProperties: Type.Undefined() },
    );

const Schema = Type.Recursive((schema) =>
    Type.Union([
        Atomic("boolean", () => Type.Boolean()),
        Atomic("integer", () => Type.Number()),
        Atomic("bigint", () => Type.Number()),
        Atomic("number", () => Type.Number()),
        Atomic("string", () => Type.String()),
        Constant("boolean", () => Type.Boolean()),
        Constant("integer", () => Type.Number()),
        Constant("bigint", () => Type.Number()),
        Constant("number", () => Type.Number()),
        Constant("string", () => Type.String()),
        Array(schema),
        Tuple(schema),
        Reference,
        RecursiveReference,
        OneOf(schema),
        Unknown,
        NullOnly,
    ]),
);

export const __TypeBoxUltimateUnion = Type.Array(Application(Schema));
export const TypeBoxUltimateUnion = TypeCompiler.Compile(
    __TypeBoxUltimateUnion,
);

Cheers

@samchon
Copy link
Owner Author

samchon commented Oct 23, 2022

I'll try the new benchmark about equals() functions with your additionalProperties: Type.Undefined() option.

It may be next weekend. Thanks for the amazing patch.

@samchon
Copy link
Owner Author

samchon commented Oct 23, 2022

This question is just by my curious about the TypeBox.

Does TypeBox have any plan to support TypeScript type definition like io-ts or zod through type meta programming?

@sinclairzx81
Copy link
Contributor

Does TypeBox have any plan to support TypeScript type definition like io-ts or zod through type meta programming?

Yes, it has first class support TypeScript Link Here

import { Type, Static } from '@sinclair/typebox'

const Point = Type.Object({
  x: Type.Number(),
  y: Type.Number(),
  slope: Type.Optional(Type.Union([Type.Null(), Type.Number()])),
});

type Point = Static<typeof Point> // <-- hover Point 

// type Point = {
//     slope?: number | null | undefined;
//     x: number;
//     y: number;
// }

@sinclairzx81
Copy link
Contributor

Hi @samchon.

Hey, Just looking at the benchmarks you've included in your readme, I do see TB is performing quite well (which is nice, and certainly appreciate you including TB in the performance graphs), but how you would feel about removing TB from the performance graphs, and just updating the following chart?

image

To be honest, it was this chart in particular that prompted the initial PR's. I submitted TypeBox and Ajv to demonstrate it was possible for JSON Schema to express these data structures, and to offer a reference model for future updates to TSON.application<T, 'ajv'> (noting the original benchmarks were measuring TSON.application<T, 'ajv'> schemas, not the ajv-spec variants)

Again, I do appreciate you including TB so prominently in your benchmarks, but feel free to remove if you wish. I'd much rather see TSON / Typia performance shine against those other ecosystem libraries (which are way more popular than TB); and for you to use TB as a tool to help produce reference schemas for some of these more complex data structures (like UltimateUnion).

Btw, Have you considered maybe moving the performance benchmarks into a separate project? The current benchmark site everyone uses is https://github.com/moltar/typescript-runtime-type-benchmarks, but note there isn't much variation between the schemas (the schemas are very simple), and they don't really highlight the functional differences between each library. If you wanted to set something up, I'd be happy to help contribute TB and other libraries for testing.

Cheers dude!

@samchon
Copy link
Owner Author

samchon commented Oct 25, 2022

https://github.com/samchon/typescript-json/blob/a97181c3ecc473a1ab307ccda57744eded82862a/benchmark/internal/IsBenchmarker.ts#L33-L39

The reason why zod is not failed on the benchmark program is, because I did not check spoiler cases for zod. If erase above special condition, zod really fails like the comparison table. As you know, if I kill all validator libaries in the is() function benchmark, succeded libraries would be expressed as 100%, therefore emphasizing succeeded libraries are not possible.

I also did not check spoiler cases for class-validator either, but it was broken by itself.

Also, you showed me a nice idea avoiding failure case of implicit union type by using additionalProperties: false option. However, such additionalProperties: false can be a good option only when comparing with special validation function equals(). As you know, additionalProperties: false option always being broken whenever superfluous property comes. In such reason, I think it can't be said that TypeBox can validate implicit union type.

If TypeBox can validate those union types without additionalProperties: false option, I'll change the table immediately.

Therefore, only wrong information from that table is Object (hierarchical) type about ajv.

@sinclairzx81
Copy link
Contributor

Well, these would the configurations for the varying tests with respect to additional properties. I think they could all be implemented (but would need separate schemas per test, so would be a pain to implement). The schemas up there should match the rules of the is test (the equals test would likely need a new set of schemas to pass).

allow superfluous properties

const T = Type.Object({
  x: Type.Number(),
  y: Type.Number(),
})

{ x: 1, y: 2 }                     // ok
{ x: 1, y: 2, z: 3 }               // ok
{ x: 1, y: 2, __cache: undefined } // ok

disallow superfluous properties (implicit for union)

const T = Type.Object({
  x: Type.Number(),
  y: Type.Number(),
}, { additionalProperties: false })

{ x: 1, y: 2 }                     // ok
{ x: 1, y: 2, z: 3 }               // fail
{ x: 1, y: 2, __cache: undefined } // fail

allow superfluous properties of undefined only (implicit for union with permissive __cache)

const T = Type.Object({
  x: Type.Number(),
  y: Type.Number(),
}, { additionalProperties: Type.Undefined() })

{ x: 1, y: 2 }                     // ok
{ x: 1, y: 2, z: 3 }               // fail
{ x: 1, y: 2, __cache: undefined } // ok

@samchon
Copy link
Owner Author

samchon commented Oct 25, 2022

I think they could all be implemented (but would need separate schemas per test, so would be a pain to implement)

Really is it possible to support all the data case? Whether possible or not, it just sounds like a crazy behavior for me. At one time, I had considered to mark symbol on implicit union type, but such painful and unpredictable circumstance is the reason why I've marked it as .

You might have complain about what I'd kept changing the union implicit DTO types for creating failure cases. But on the other hand, it also means that TypeBox is vulnerable to implicit union types so easily that I can find failure cases so easily. How can I check such TypeBox to about implicit union type?

Implementing union type specialization algorithm is not difficult. I just hope you to implement it and come back again.

@sinclairzx81
Copy link
Contributor

sinclairzx81 commented Oct 25, 2022

also means that TypeBox is vulnerable to implicit union types so easily that I can find failure cases so easily

How? The schemas were written to match the cases you were testing for. You added additional tests after the fact, so I updated the schemas to conform to the specifications of the new tests. JSON Schema doesn't assume any default validation behavior, this is the programmers job to produce schemas with the desirable constraints. The goal of these PR's were to find these constraints, and I think that was largely achieved?

You might have complain about what I'd kept changing the union implicit DTO types for creating failure cases

I'm not complaining, I'm trying to help. The updated tests with spoiler data revealed disparities between the validation behaviors of Ajv, TSON.is and TSON.application (which I think is a good thing to highlight), As such I have tried to align these schemas to match the internal validation rules implemented in TSON for the benefit of TSON. Because these schemas now (hopefully) match the internal validation rules for TSON.is(). This means you can export TSON validation logic as JSON Schema and have it work exactly the same on remote systems (for example those written in C#)...isn't this a good thing?

So, assuming these schemas are aligned, the only thing left to do would be implement the same JSON schema for TSON.application<T, 'ajv'> and everything should line up perfectly. That would be an amazing feature for TSON (at least I think so)

Implementing union type specialization algorithm is not difficult. I just hope you to implement it and come back again.

Yes, I have reviewed the union specialization algorithm, unfortunately it violates many compositional components for both JSON Schema and JSON Type Definition (JTD), so can't be implemented in TypeBox. This doesn't mean you shouldn't implement in TSON, it's a good algorithm.

@samchon
Copy link
Owner Author

samchon commented Oct 26, 2022

How? The schemas were written to match the cases you were testing for. You added additional tests after the fact, so I updated the schemas to conform to the specifications of the new tests. JSON Schema doesn't assume any default validation behavior, this is the programmers job to produce schemas with the desirable constraints. The goal of these PR's were to find these constraints, and I think that was largely achieved?

I repeat that your special logic additionalProperties: false for implicit union type always broken whenever superfluous property comes. I know you're saying that it is okay because you've succeeded to react whenever I'd changed the superfluous property has been changed. But I have an extreme solution you never can react with the additionalProperties: false option.

const raw = {
    ...props of ObjectUnionImplicit.IPolyline
    ...{
        [RandomGenerator.string()]: "some random value",
    }
};
const polyline: ObjectUnionImplicit.IPolyline= raw; // possible

You can yield the above code is really crazy. However, this typescript-json is targeting to the TypeScript spec and TypeScript allows such superfluous property. I understand that how JSON schema and TypeScript type spec is different, but this typescript-json is targeting to the TypeScript type spec and I have to follow the TypeScript rule. Please understand me.

In such reason, for me, normal validation means supporting superfluous properties. Also, when a validation function being broken by those superfluous properties, I consider it as vulnerable. I've made typescript-json under such idea.

On the other hand, I appreciate your contributions and special guidances for the JSON schema spec. I've learned my things from them.

@sinclairzx81
Copy link
Contributor

sinclairzx81 commented Oct 26, 2022

On the other hand, I appreciate your contributions and special guidances for the JSON schema spec. I've learned my things from them.

Cool, I'm glad :D Based on these schemas that have been setup thus far though. Would it be possible to have TSON.application<[T], 'ajv'> generate something like the following?

Inline

import TSON from 'typescript-json'

interface Bar {
    a: string,
    b: string
}

interface Foo {
    x:   number,
    y:   number,
    bar: Bar
}

const Foo = TSON.application<[Foo], 'ajv'>()

// const Foo = {
//    $id: 'Foo',
//    type: 'object',
//    required: ['x', 'y'],
//    properties: {
//      x: { type: 'number' }
//      y: { type: 'number' }
//      bar: {
//        $id: 'Bar',
//        type: 'object',
//        required: ['a', 'b'],
//        properties: {
//          a: { type: 'string' }
//          b: { type: 'string' }
//        }
//      }
//    }
// }

Normalized

import TSON from 'typescript-json'

type Ref<T> = T // marker

interface Bar {
    a: string,
    b: string
}

interface Foo {
    x:   number,
    y:   number,
    bar: Ref<Bar>
}

const Foo = TSON.application<[Foo], 'ajv'>()

// const Foo = {
//    $id: 'Foo',
//    type: 'object',
//    required: ['x', 'y'],
//    properties: {
//      x: { type: 'number' }
//      y: { type: 'number' }
//      bar: { $ref: 'Bar' }
//    }
// }

const Bar = TSON.application<[Bar], 'ajv'>()

// const Bar = {
//    $id: 'Bar',
//    type: 'object',
//    required: ['a', 'b'],
//    properties: {
//      a: { type: 'string' }
//      b: { type: 'string' }
//    }
// }

Generic Normalized

Using a convention that generic arguments be delimited with _ such that T_T0_T1_T2 would correlate to type T<T0, T1, T2> = ... ?

import TSON from 'typescript-json'

type Ref<T> = T // marker

interface Bar<T0, T1> {
    a: T0,
    b: T1,
}

interface Foo {
    x:   number,
    y:   number,
    bar: Ref<Bar<string, number>>
}

const Foo = TSON.application<[Foo], 'ajv'>()

// const Foo = {
//    $id: 'Foo',
//    type: 'object',
//    required: ['x', 'y'],
//    properties: {
//      x: { type: 'number' }
//      y: { type: 'number' }
//      bar: { $ref: 'Bar_string_number' }
//    }
// }

const Bar = TSON.application<[Bar<string, number>], 'ajv'>()

// const Bar = {
//    $id: 'Bar_string_number',
//    type: 'object',
//    required: ['a', 'b'],
//    properties: {
//      a: { type: 'string' }
//      b: { type: 'number' }
//    }
// }

Recursive

import TSON from 'typescript-json'

interface Node {
  id: string
  nodes: Node[]
}

const Node = TSON.application<[Node], 'ajv'>()

// const Node = {
//    $id: 'Node',
//    type: 'object',
//    required: ['id', 'nodes'],
//    properties: {
//      id: { type: 'string' }
//      nodes: { 
//        type: 'array', 
//        items: { $ref: 'Node' } 
//      }
//    }
// }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants