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

intersect of object and record does not work like Typescript #491

Open
bug-brain opened this issue Mar 19, 2024 · 9 comments
Open

intersect of object and record does not work like Typescript #491

bug-brain opened this issue Mar 19, 2024 · 9 comments
Assignees
Labels
question Further information is requested

Comments

@bug-brain
Copy link

I want to model a type that has both normal properties and an index signature, for example { a: number; [b: number]: string; }. My best attempt so far is this one.

import * as v from 'valibot';

const object = v.object({ a: v.number() })
const record = v.record(v.union([v.number()]), v.string());
const intersect = v.intersect([object, record])

const t: v.Output<typeof intersect> = { a: 0, 1: "x", 2: "y" }
const result = v.safeParse(intersect, t);
console.log(result);

Notice that Typescript does not show any errors but valibot gives this result which shows that the normal properties are checked against the index signature even though the the property ("a") does not match the signature (number).

{
  typed: false,
  success: false,
  output: undefined,
  issues: [
    // ...
    {
      reason: "type",
      context: "number",
      expected: "number",
      received: "\"a\"",
      message: "Invalid type: Expected number but received \"a\"", // here
      input: "a",
      path: [
        {
          type: "record",
          origin: "key",
          input: {
            1: "x",
            2: "y",
            a: 0
          },
          key: "a",
          value: 0
        }
      ],
      issues: undefined,
      lang: undefined,
      abortEarly: undefined,
      abortPipeEarly: undefined,
      skipPipe: undefined
    },
    {
      reason: "type",
      context: "string",
      expected: "string",
      received: "0",
      message: "Invalid type: Expected string but received 0", // here
      input: 0,
      path: [
        {
          type: "record",
          origin: "key",
          input: {
            1: "x",
            2: "y",
            a: 0
          },
          key: "a",
          value: 0
        }
      ],
      issues: undefined,
      lang: undefined,
      abortEarly: undefined,
      abortPipeEarly: undefined,
      skipPipe: undefined
    }
  ]
}
@fabian-hiller
Copy link
Owner

Please try object with rest argument:

import * as v from 'valibot';

const Schema = v.object({ a: v.number() }, v.string());

@fabian-hiller fabian-hiller self-assigned this Mar 19, 2024
@fabian-hiller fabian-hiller added the question Further information is requested label Mar 19, 2024
@bug-brain
Copy link
Author

While this improves validation the inferred output type is { a: number } & Record<string, string> which is basically never because "a" matches string and has to have a string & number value. Further it does not validate that the rest keys are of type number (or more accurately, strings that represent numbers because of the implicit conversion).

@fabian-hiller
Copy link
Owner

fabian-hiller commented Mar 20, 2024

In my environment there is no problem with the current typing. a is a number and everything else is a string. But I see your point and am happy to improve the API and implementation of the library. Do you have any concrete ideas? Do you know how this bahaves in other schema libraries?

@bug-brain
Copy link
Author

Just to be clear I was trying out this

import * as v from 'valibot';
const Schema = v.object({ a: v.number() }, v.string());
const obj: v.Output<typeof Schema> = { a: 1, b: "" }

and then got

Type '{ a: number; b: string; }' is not assignable to type '{ a: number; } & Record<string, string>'.
  Type '{ a: number; b: string; }' is not assignable to type 'Record<string, string>'.
    Property 'a' is incompatible with index signature.
      Type 'number' is not assignable to type 'string'. (2322)

As for ideas my best guess is to replace the rest parameter with two new ones (maybe in an option object):

  • an array of index signatures that are tuples of key and value types
  • a flag that controls whether unknown properties produce and error or not

Then for example the schema

v.object(
  { a: v.number() },
  {
    indexSignatures: [
      [
        v.special(
          (input) => typeof input === "string" && input.startsWith("prefix")
        ) as v.SpecialSchema<unknown, `prefix${string}`>,
        v.string(),
      ],
    ],
    allowUnknownProperties: true,
  }
)

could parse { a: 0 }, { a: 1, prefix: "" }, { a: 2, b: null } but not { a: 3, prefixB: null } or { a: "4" }. For each key/value-pair check if key matches some properties and/or index signatures. If yes check value against all those value types otherwise raise an error if flag is set. For v.number() as key type there could be a special case to check something along the lines of parseFloat(key).toString() === key.

But I have to admit that sounds like a lot of work and I get the feeling I have over done my types over here.

@fabian-hiller
Copy link
Owner

I am currently rewriting the entire library. Most things will stay the same, but I expect the bundle size to be smaller, the performance to be better, and the types to be more accurate. I plan to rewrite this part of the library as well. My idea is to remove the rest parameter from object and instead provide a strictObject and objectWithRest schema. I will investigate if I can fix this type problem.

@fabian-hiller fabian-hiller added enhancement New feature or request priority This has priority and removed question Further information is requested labels Mar 21, 2024
@fabian-hiller
Copy link
Owner

I have investigated this issue further. There is a problem. Something like { a: number; [b: number]: string } does not "really" exist in JavaScript. If you use an object like { a: 123, 0: 'foo' }, the key 0 is automatically converted or interpreted as a string. For example, if you type { 0: 'foo', '0': 'bar' } JavaScript will return { 0: 'bar' }. It seems that there is no way for us as a schema library to know if a key was entered as a number. If we were to validate against the number schema for the key of an object, it would always fail.

@fabian-hiller
Copy link
Owner

Furthermore, TypeScript does not allow us to define an object as { foo: number; [key: string]: string }. So I am not sure if there is an alternative to { foo: number } & Record<string, string>. Do you have any idea how we could type this better?

@fabian-hiller fabian-hiller added question Further information is requested and removed enhancement New feature or request priority This has priority labels Apr 1, 2024
@bug-brain
Copy link
Author

I don't think it is feasible to check the key types for overlaps during the construction of the schema because the main purpose of this library is to check concrete values against schemas and not schemas against schemas (at that point you would basically implement TypeScript yourself). The user should be responsible for ensuring that the index signature do not overlap (or if they overlap the value type should match).

As for the implicit conversion of numbers to strings the same logic as TypeScript's should be applied, so for example for the type { [n: number]: string } these are valid properties: "0", "0.1", "NaN", "Infinity", "-Infinity"; but these are not: "1.0", "1e6", "001", "1_000". So basically leading and trailing zeroes and exponent and separated notation are forbidden. This is why I suggested to use parseFloat(key).toString() === key to see if the string represents a number as JavaScript would have encoded it. How a user entered it should not matter, only the resulting object.

@fabian-hiller
Copy link
Owner

I really don't know how we could implement that in a clean way. Feel free to figure it out after we are done with the rewrite in #502.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants