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

Recursive type #18

Open
aomader opened this issue Nov 26, 2021 · 12 comments
Open

Recursive type #18

aomader opened this issue Nov 26, 2021 · 12 comments

Comments

@aomader
Copy link

aomader commented Nov 26, 2021

Hej,

how would you handle a recursive type, for example somethling like:

interface Foo {
    [key: string]: Foo | string | undefined;
}

Currently, I do not see a way to express this.

@ShlomiAltostra
Copy link
Contributor

ShlomiAltostra commented Nov 28, 2021

Hi!,

You're right
Currently, there's no built-in support for recursive object-types

You can still write a predicate (boolean returning function) for that case and use it alongside and with, other type-validations

@ShlomiAltostra
Copy link
Contributor

ShlomiAltostra commented Nov 28, 2021

@aomader

Simple example if you can live without rejections:

const isFoo = recordOf(maybe(
  // `isFoo` is wrapped with a function - so it finishes to initialize, but can still be referenced by `anyOf`
  anyOf(string, (value: unknown): boolean => isFoo(value))
))

If you have to get rejections, you can use the more elaborate code

const isFoo: TypeValidation<Foo> = recordOf(maybe(anyOf(
  string,
  registerRejectingValidator(
    (value: unknown, reject): value is Foo => isFoo(value, reject),
    'Foo',
    (transform, args) => isFoo[transformValidation](transform, args)
  )
)))

I'm aware this is suboptimal, and intend to create a built-in for recursive types - but for now I hope this helps

@aomader
Copy link
Author

aomader commented Nov 30, 2021

@ShlomiAltostra Thank you for the input, I didn't know that I may pass in a simple boolean function (or type guards for that matter). Thank you!

@aomader
Copy link
Author

aomader commented Nov 30, 2021

@ShlomiAltostra Turns out, that this is not working as expected:

import { anyOf, primitives, recordOf } from "@altostra/type-validations";

interface Foo {
    [key: string]: Foo | string | undefined;
}

export const isFoo = recordOf({
    key: primitives.string,
    value: anyOf(
        primitives.undefinedValidation,
        primitives.string,
        (data: unknown): data is Foo => isFoo(data)
    ),
});

test("isFoo", () => {
    expect(isFoo({ foo: "bar" })).toBe(true);
    expect(isFoo({ foo: [{ bar: "1337" }] })).toBe(false); // FAILS!!!
});

This is somewhat unexpected to me. The array should not valid.

@ShlomiAltostra
Copy link
Contributor

@aomader
Arrays are objects
And by the cyclic definition of Foo, an array of Foos is also a Foo (because all its own enumerable keys are Foos)

@aomader
Copy link
Author

aomader commented Nov 30, 2021

I totally understand that this is technically correct. Yet, I highly doubt that people would consider this a good default behavior nor an expected one.
Especially, given that the following does not compile in typescript:

const rndm: Record<string, unknown> = ["asd"];

@ShlomiAltostra
Copy link
Contributor

ShlomiAltostra commented Nov 30, 2021

@aomader

I understand that is unexpected for you - but some can have the exact opposite expectation, and might even rely on it

It is as easy to exclude arrays, just as it would be easy to include them if they weren't objects

export const isFoo = allOf(
  recordOf({
    key: primitives.string,
    value: anyOf(
        primitives.undefinedValidation,
        primitives.string,
        (data: unknown): data is Foo => isFoo(data)
    ),
  }),
  (value: unknown) => !Array.isArray(value)
);

TS also regards arrays as objects - as long as the defined properties do not conflict with Array methods and properties
image
The validation is performed only on own-enumerable properties, and I do need (and will) to document this.

Maybe I'll consider adding options to validate other properties if I'll see demand for such feature - but to validate parsed JSON (or any such format) this is never required

P.S.
If we include other-than own-enumerable properties in the validation, neither {} (an empty object) is a Record<any, string> - as it has a few methods inherited from Object

@aomader
Copy link
Author

aomader commented Nov 30, 2021

This is mostly about API ergonomics and communicating the intent.

The function is called recordOf, which validates Record<>. In TS, as I illustrated above, an array is not a Record<string, unknown>. Hence, you would not expect the recordOf validator to allow arrays...

Additionally, there is an arrayOf, which obviously should be used for array types.

To be honest, I think this is a big problem. This somewhat widened the allowed range of input data quite a bit without me noticing, since the behavior somewhat deviates from TS.

@ShlomiAltostra
Copy link
Contributor

ShlomiAltostra commented Nov 30, 2021

Is array a record?
Yes it is:
image
The problem is with the "string" index signature conflicting with Array methods and properties

Now, there's the fact that property-keys in JS are always strings, so there's no difference between
{1: 'a'} and {"1": 'a'} , they are structured exactly the same - and this is also true for arrays.

Thus, a record where the keys are "numeric", is also a record of "string" keys. there is no difference.
Indices are stored as strings in the array just like any other object.

Also, if you exclude arrays, what about { ...[{ baz: 'str' ] }?
Is this a record, or not?
If not, how can you differentiate this from a record?

What a proper validation in your opinion should exclude?

  • Specifically arrays but no other objects?
  • Every object inheriting any class (this is the technically correct definition for a record: an object created with Object.create(null))?
  • Every object inheriting any class but Object (so one can use { a: 5 } object literals)?

Why specifically?

Arrays are objects, with the very meaning of being a record.
The only problem here is expecting arrays to be something else than a record with some special behavior - while in fact, they are regular objects with special behavior specified by their prototype.

The recordOf validation checks that values are objects and that their own-enumerable properties have a certain type.
Not the object inheritance chain, nor anything else.

So we'll have to agree to disagree regarding your suggestion being good API ergonomics.


arrayOf validates that a value is indeed an array and not just mere "array-like" object
recordOf validates that a value is an object with a specified properties type.
isObject validates that value is an object and not a primitive

An array is an object and a record, but an object is not necessarily an array.

@aomader
Copy link
Author

aomader commented Nov 30, 2021

Considering the MWE from above again:

test("isFoo", () => {
    // this checks out
    expect(isFoo([])).toBe(true);

    // but this does not even compile
    const foo: Foo = [];
});

I find this discrepancy unsettling.

@ShlomiAltostra
Copy link
Contributor

ShlomiAltostra commented Dec 1, 2021

image

Yet, as you can see, the problem is not that Foo is not a record - because it is, rather that TS checks both own properties and inherited properties, and we only deal with own-enumerable properties, as I already explained.

After checking for isFoo, you can be assured that the object is a record of either strings or Foos (in a recursive manner).
It doesn't matter what you think about arrays - you can use all the APIs to work with "records" because that is what they are.

What is concerning for you?
Can you be specific about a problem that you think may arise?

@aomader
Copy link
Author

aomader commented Dec 1, 2021

Well, the code you are using is not the one I'm talking about.

Again, to show it in its full glory:

import { anyOf, primitives, recordOf } from "@altostra/type-validations";

interface Foo {
    [key: string]: Foo | string | undefined;
}

export const isFoo = recordOf({
    key: primitives.string,
    value: anyOf(
        primitives.undefinedValidation,
        primitives.string,
        (data: unknown): data is Foo => isFoo(data)
    ),
});

test("isFoo", () => {
    // this checks out
    expect(isFoo([])).toBe(true);

    // but this does not even compile
    const foo: Foo = [];
});

And obviously something that does not compile should also not validate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants