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

Covariance (?) bug: pipe checks type assignability in a wrong "direction" #979

Open
0x009922 opened this issue Dec 13, 2024 · 2 comments
Open
Assignees
Labels
question Further information is requested

Comments

@0x009922
Copy link

0x009922 commented Dec 13, 2024

Minimal reproduction

const SchemaA = v.union([v.string(), v.number()])

const SchemaB = v.pipe(v.string(), SchemaA)

Expected

Should work - SchemaA accepts both string and number as input.

Actual

SchemaA isn't allowed in pipe after v.string():

No overload matches this call.
  Overload 1 of 21, '(schema: StringSchema<undefined>, item1: BaseSchema<string, unknown, BaseIssue<unknown>> | BaseValidation<string, unknown, BaseIssue<unknown>> | BaseTransformation<...> | BaseMetadata<...>): SchemaWithPipe<...>', gave the following error.
    Argument of type 'UnionSchema<[StringSchema<undefined>, NumberSchema<undefined>], undefined>' is not assignable to parameter of type 'BaseSchema<string, unknown, BaseIssue<unknown>> | BaseValidation<string, unknown, BaseIssue<unknown>> | BaseTransformation<string, unknown, BaseIssue<unknown>> | BaseMetadata<...>'.
      Type 'UnionSchema<[StringSchema<undefined>, NumberSchema<undefined>], undefined>' is not assignable to type 'BaseSchema<string, unknown, BaseIssue<unknown>>'.
        Types of property ''~standard'' are incompatible.
          Type 'StandardSchemaProps<string | number, string | number>' is not assignable to type 'StandardSchemaProps<string, unknown>'.
            Type 'string | number' is not assignable to type 'string'.
              Type 'number' is not assignable to type 'string'.
  Overload 2 of 21, '(schema: StringSchema<undefined>, ...items: PipeItem<string, string, BaseIssue<unknown>>[]): SchemaWithPipe<[StringSchema<undefined>, ...PipeItem<string, string, BaseIssue<...>>[]]>', gave the following error.
    Argument of type 'UnionSchema<[StringSchema<undefined>, NumberSchema<undefined>], undefined>' is not assignable to parameter of type 'PipeItem<string, string, BaseIssue<unknown>>'.
      Type 'UnionSchema<[StringSchema<undefined>, NumberSchema<undefined>], undefined>' is not assignable to type 'BaseSchema<string, string, BaseIssue<unknown>>'.
        Types of property ''~standard'' are incompatible.
          Type 'StandardSchemaProps<string | number, string | number>' is not assignable to type 'StandardSchemaProps<string, string>'.
            Type 'string | number' is not assignable to type 'string'.
              Type 'number' is not assignable to type 'string'.ts(2769)

So, although you would expect that the input type of SchemaA should extend the output type of v.string(), it seems it works vice versa and requires the output type of v.string() to extend the input type of SchemaA.

Workaround

I have to cast the schema with "wrong requirements" explicitly:

const SchemaA = v.union([v.string(), v.number()])

const SchemaB = v.pipe(v.string(), SchemaA as v.GenericSchema<string, string | number>)
@fabian-hiller
Copy link
Owner

fabian-hiller commented Dec 13, 2024

Thank you for creating this issue. Currently, pipe requires that the input type of a subsequent item extends the output type of the previous item. Since string | number does not extend string, TypeScript displays an error. I am not sure if it is possible to implement it differently without loosing type safety. What is your use case? Why is the current implementation a problem in the real world? In most cases, I recommend using only a single schema function at the beginning of the pipeline, as there should be no need to validate the data type again.

@fabian-hiller fabian-hiller self-assigned this Dec 13, 2024
@fabian-hiller fabian-hiller added the question Further information is requested label Dec 13, 2024
@0x009922
Copy link
Author

Currently, pipe requires that the input type of a subsequent item extends the output type of the previous item. Since string | number does not extend string, TypeScript displays an error.

AFAIK T extends U means that T is a subtype of U. So, you are saying that pipe requires that in the chain A -> B the input type of B must be a subtype of the output type of A. This, I think, is not correct. If B accepts string | number, it means it is completely safe to pass a string to it (string is a subtype of string | number). Therefore, A output must be a subtype of B input. Or am I missing something?

Here is my case:

const accountIdSchemaObj = v.object({
  signatory: v.pipe(
    v.string(),
    v.hexadecimal(),
    v.transform((hex) => PublicKey.fromMultihash(hex)),
  ),
  domain: DomainIdSchema,
})

const AccountIdSchema = v.pipe(
  v.union([
    accountIdSchemaObj,
    v.pipe(
      v.string(),
      v.rawTransform(({ dataset, addIssue, NEVER }) => {
        const input = dataset.value
        const parts = input.split('@')
        if (parts.length !== 2) {
          addIssue({ message: `AccountId should have format '⟨signatory⟩@⟨domain⟩', got: '${input}'` })
          return NEVER
        }
        const [signatory, domain] = parts
        return { signatory, domain }
      }),
      accountIdSchemaObj,
    ),
  ]),
  v.transform((x) => new AccountId(x.signatory, x.domain)),
)

const assetDefIdSchemaObj = v.object({ name: NameSchema, domain: DomainIdSchema })

const AssetDefinitionIdSchema = v.pipe(
  v.union([
    v.pipe(
      v.string(),
      v.rawTransform(({ dataset: { value: input }, addIssue, NEVER }) => {
        const parts = input.split('#')
        if (parts.length !== 2) {
          addIssue({ message: `AssetDefinitionId should have format '⟨name⟩@⟨domain⟩', got '${input}'` })
          return NEVER
        }
        const [name, domain] = parts
        return { name, domain }
      }),
      assetDefIdSchemaObj,
    ),
    assetDefIdSchemaObj,
  ]),
  v.transform((x) => new AssetDefinitionId(x.name, x.domain)),
)

const AssetIdSchema = v.pipe(
  v.string(),
  v.rawTransform(({ dataset: { value: input }, addIssue, NEVER }) => {
    const match = input.match(/^(.+)#(.+)?#(.+)@(.+)$/)
    if (!match) {
      addIssue({
        message:
          `AssetId should have format '⟨name⟩#⟨asset domain⟩#⟨account signatory⟩@⟨account domain⟩' ` +
          `or '⟨name⟩##⟨account signatory⟩@⟨account domain⟩' (when asset and account domains are the same), got '${input}'`,
      })
      return NEVER
    }
    const [, asset, domain1, account, domain2] = match
    return {
      account: { signatory: account, domain: domain2 },
      definition: { name: asset, domain: domain1 ?? domain2 },
    }
  }),
  v.object({
    account: AccountIdSchema,
    definition: AssetDefinitionIdSchema,
  }) as v.GenericSchema<
    { account: { signatory: string; domain: string }; definition: { name: string; domain: string } },
    { account: AccountId; definition: AssetDefinitionId }
  >,
//^^^^^^^^^^^^^^^^^^^^^^^^^^^^ issue is here
  v.transform((x) => new AssetId(x.account, x.definition)),
)

where DomainIdSchema is a branded string.

So, I have a "waterfall" of parsing here: AssetId accepts only a string, which it parses and delegates further to AccountId and AssetDefinitionId to parse as objects. Those can parse objects alongside with their own string format.

Is there an idiomatic way to build such a pipeline in Valibot?

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