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.

Sign up for GitHub

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

Jump to bottom

Type of sibling property of a ref property is unknown when validated via JTDSchemaType #2167

Open
ento opened this issue Nov 16, 2022 · 4 comments · May be fixed by #2316
Open

Type of sibling property of a ref property is unknown when validated via JTDSchemaType #2167

ento opened this issue Nov 16, 2022 · 4 comments · May be fixed by #2316
Labels
bug report typescript

Comments

@ento
Copy link

ento commented Nov 16, 2022

What version of Ajv are you using? Does the issue happen if you use the latest version?

8.11.2 (latest as of this writing)

Ajv options object

import Ajv, { JTDSchemaType } from "ajv/dist/jtd";
const ajv = new Ajv();

JTD Schema

type MyType = {
  referenced: number;
  stringProp: string;
};

const schema: JTDSchemaType<MyType, { num: number }> = {
  definitions: {
    num: { type: "int32" }
  },
  properties: {
    referenced: { ref: "num" },
    stringProp: { type: "string" }
  }
} as const;

Sample data

const data = {} as any;

Your code

const validate = ajv.compile(schema);

if (validate(data)) {
  const revealType: never = data;
  const stringProp: string = data.stringProp;
  console.log(stringProp);
}

https://replit.com/@ento/IntrepidIndolentOutput#index.ts

^strictNullChecks is enabled in tsconfig.json

		"strictNullChecks": true,

Validation result, data AFTER validation, error messages

TypeScript's typecheck errors:

// const revealType
Type '{ referenced: number; stringProp: unknown; } & {}' is not assignable to type 'never'.

// const stringProp
Type 'unknown' is not assignable to type 'string'.

What results did you expect?

The type of stringProp to be string after being narrowed by validate(data)

Are you going to resolve the issue?

My current workaround is to hold the definition in a const and use it to compose the schema object. (The actual code is a bit more complex than this.)

const numSchema = { type: "int32" } as const;

const schema: JTDSchemaType<MyType> = {
  properties: {
    referenced: numSchema,
    stringProp: { type: "string" }
  }
} as const;
@epoberezkin
Copy link
Member

you need to pass interface for definitions as a second parameter: https://github.com/ajv-validator/ajv/blob/master/spec/types/jtd-schema.spec.ts#L356

@epoberezkin epoberezkin reopened this Jul 29, 2023
@epoberezkin
Copy link
Member

ah actually it is passed... cc @erikbrinkman

@erikbrinkman
Copy link
Collaborator

@ento Thanks for catching this, I'm surprised that that you're the first person to report it!

The current code path for compiling existing schemas looks like this:

ajv/lib/core.ts

Line 374 in 490eb8c

compile<T = unknown>(schema: JTDSchemaType<T>, _meta?: boolean): ValidateFunction<T>

Due to the default types, it seems that typescript is reinterpreting MyType but as if refs was Record<string, unknown> instead of the refs you passed in when creating the schema.

Changing it to infer the refs like:

compile<T = unknown, R extends Record<string, unknown> = Record<string, unknown>>(schema: JTDSchemaType<T, R>, _meta?: boolean): ValidateFunction<T>

seems to get typescript to propagate the refs you specified appropriately. I'll create a diff to fix this, hoping that it doesn't break anything else. Unfortunately, the typing of compile is pretty complex, and so small simple changes like this can sometimes screw with the compiler 🤞

erikbrinkman added a commit to erikbrinkman/ajv that referenced this issue Jul 29, 2023
@erikbrinkman
add refs varaible for jtd validation
80db1da 
There seems to be some weird behavior with typescript when refs was left
to default. By specifying it manually the behavior was fixed.

fixes ajv-validator#2167
@erikbrinkman erikbrinkman linked a pull request Jul 29, 2023 that will close this issue
Open
@erikbrinkman
Copy link
Collaborator

After writing the fix, I realized that I was a little wrong. It' picking up the ref type appropriately, but not the non-ref type. This fix seems to work and makes sense, so I'm not going to dig into this any more, but it is odd. I'll also note than when I tried to stub out compile on its own, typescript didn't let me without refs, so there's so more complications due to the overloading and member function nature.

erikbrinkman added a commit to erikbrinkman/ajv that referenced this issue Jul 29, 2023
@erikbrinkman
add refs varaible for jtd validation
4d001a5 
There seems to be some weird behavior with typescript when refs was left
to default. By specifying it manually the behavior was fixed.

fixes ajv-validator#2167
erikbrinkman added a commit to erikbrinkman/ajv that referenced this issue Jul 30, 2023
@erikbrinkman
add refs varaible for jtd validation
29168a4 
There seems to be some weird behavior with typescript when refs was left
to default. By specifying it manually the behavior was fixed.

fixes ajv-validator#2167
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug report typescript
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

add refs variable for jtd validation erikbrinkman/ajv
3 participants
@ento @erikbrinkman @epoberezkin