Skip to content

Remove optional type from the validation context parameter #1747

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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

adrianolek
Copy link

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

8.6.2, happens in the latest version

Your typescript code

import { DataValidateFunction } from "ajv/dist/types";

ajv.addKeyword({
  keyword: "test",
  schema: false,
  validate: function (_data: any, dataCxt) {
    dataCxt.rootData;
    return true;
  } as DataValidateFunction
});

Typescript compiler error messages

(7,22): TS2532: Object is possibly `undefined`.

It means that the dataCxt object is possibly 'undefined'.

Describe the change that should be made to address the issue?

Remove the optional ? operator from the dataCxt parameter in ValidateFunction and SchemaValidateFunction interface types.

I've checked the generated code, and the validation context object is always passed to the validation function. Also the codegen validateFunction seems to always include the validation context (although I'm not familiar with codegen itself, so I may be wrong here).

Are you going to resolve the issue?

Yes. The PR removes the optional operator from dataCxt and renames DataValidationCxt to ValidationCxt & dataCxt to valCxt to match the codegen function.

@epoberezkin
Copy link
Member

I think it is most likely a breaking change, and I do think it has to be optional... But in any case, please try to make the tests pass - I've fixed the typescript issue that was breaking the compilation.

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

Successfully merging this pull request may close these issues.

2 participants