v7 : unevaluatedProperties , modify values to match schema

[peerreviewsystem]

What version of Ajv you are you using? v7

What problem do you want to solve? To remove unevaluated properties similar to the removeAdditional feature with a removeUnevaluated option.

Is this feature planned or should I contribute with a PR to implement it ? ?

[epoberezkin]

Good idea - wasn't planned :)

PR would be great! Happy to give some pointers as needed, let me know.

It would be simpler than removeAdditional - just a boolean flag that would add code to remove properties that would otherwise be reported as unevaluated.

I am thinking - it may be helpful to add "code organisation doc" for contributors - long due - to help navigate the code and understand what parts do what - I would probably do it, or if you figured it out already feel free to contribute it before/after this feature.

[epoberezkin]

#1347

[epoberezkin]

@peerreviewsystem see components.md

[peerreviewsystem]

Thank you ! I will start taking a look at this in a week.

[EtaiG]

@peerreviewsystem how's it going?
I am happy to contribute, but do not want to take over if you are working on it and plan on finishing soon.
Do you want/need help?

[peerreviewsystem]

Hello @EtaiG , I’m working on it but I don’t have a precise deadline, hopefully I’ll have something in a few weeks. This is my first contribution to a major open source project so I would be very happy to be able to do it but if it is a feature that you urgently need then I can’t tell you to not do it :)

But if you let me do it, of course your help would be very welcome if I get stuck :)

[epoberezkin]

@peerreviewsystem @EtaiG I am very happy to help splitting the work - it's exciting to have you both interested in the feature.

Also, happy to provide early feedback on the implementation plan to make review/merging easier.

There are some smaller isolated tasks here that can be merged independently to move it faster :)

I could make a feature branch e.g. remove-unevaluated, and then smaller PRs would go into it, e.g.:

1. write docs
2. write [failing] tests (we can skip them all to begin with)
3. add option to types and have Ajv constructor recognise it.
4. implement actual functionality - it probably should be contained within the unevaluatedProperties.ts itself, but given cross-cutting nature of the keyword, it might spill out to other places that track evaluated properties - hopefully not...

Making it 4 PRs would make it move and merge faster - and also allow you to collaborate if you are open to it.

My main focuses for Q1 is to add support for JTD (JSON Type Definition - a new RFC, an alternative to JSON Schema) and the new website - something modern and nice like svelte.dev, but happy to support this feature - removeAdditional has been a source of much confusion historically, hopefully this one will produce more expected behaviours.

@peerreviewsystem @EtaiG Let me know what you think

[peerreviewsystem]

Thank you for your answer. On a personal level, it is a great exercise for me to try to make an implementation, get familiar with the codebase and make a meaningful contribution, but I initially did not expect that this would be an urgently wanted feature for other people so I did not spend a great amount of time yet. So depending on @EtaiG needs at his company we can split the work accordingly.

[EtaiG]

@epoberezkin I'm all for smaller PR's.
@peerreviewsystem if you're interested in collaborating, shoot me an email to etai / AT / wix.com . We can setup a zoom / slack session and talk about it.

[EtaiG]

@epoberezkin
@peerreviewsystem and I will start working on it this week.

some questions.

1. We see that 'removeAdditional' was implemented inside of applicator/additionalProperties and referenced through applicator/properties, within the code(ctx) function. Is the intention that the implementation should follow this pattern, and implement the 'removeUnevaluated' inside unevaluated/unevaluatedProperties and unevaluated/unevaluatedItems ?
2. Is there a specific reason unevaluatedProperties is inside it's own folder (unevaluated) and not inside applicator? I do see this is part of the applicator keywords defined by the applicator meta-schema: https://json-schema.org/draft/2019-09/meta/applicator
3. Do you have any tips or things to keep in mind that you recommend?

[epoberezkin]

Great - I will set up the branch for you to make PRs against (or you can make it in the fork of course, but having several smaller PRs is probably better? up to you)

1. Is the intention that the implementation should follow this pattern, and implement the 'removeUnevaluated' inside unevaluated/unevaluatedProperties and unevaluated/unevaluatedItems ?

I am open to other ideas, but it seems like the right approach. At the moment "keyword" is a processing unit and decision which properties are "unevaluated" happens in this place. But let me know if you come up with other approaches.

2. Is there a specific reason unevaluatedProperties is inside it's own folder (unevaluated) and not inside applicator?

Firstly, it has to be supported in Ajv2019 subclass and not in Ajv subclass - it was just easier to have it separate, so the unit of keyword initialisation is a "vocabulary", rather than individual keyword. These keywords in particular are cross-cutting, their support requires changes in generated code in many other keywords (it shouldn't affect this feature though, I expect it to be contained within this keyword - but we will see)

Secondly, coincidentally, the latest JSON Schema draft that was published (2020-12 - it's on IETF) separated these keywords into a separate vocabulary.

3. Do you have any tips or things to keep in mind that you recommend?

Nothing major other than what's above at the moment - let's start from feature definition/docs to define the scope. Do ask any questions please - codegen is not super-well documented, so it'll be an opportunity to improve it. At the moment it would help looking at the source code of codegen, KeywordCxt class and existing keywords - quite advanced things can be expressed concisely.

One extra note on codegen: for tree optimisation to work it's important to generate code using methods in codegen, and not just template literals, e.g. gen.let("a", 5) can be completely removed if a is not used, while with const a = gen.name("a"); gen.code(_`let ${a} = 5`) the generated code snippet will be the same but it cannot be optimised away whether a is used or not (even more so about control flow statements). For standalone code generation it can be done with other tools, so not so important, but for runtime closures there is a performance difference obviously - v8 has to parse it and to execute some of it, even if it is not needed - it cannot make the same assumptions that Ajv optimizer makes (e.g. no side effects from assignments etc. - see notes in codegen.md). But I'll pick on at such things at review, so not a problem.

[epoberezkin]

here it is: https://github.com/ajv-validator/ajv/tree/remove-unevaluated

[bricejar]

@epoberezkin I am not too familiar with mocha + ts. I would like to know, how may I run tests in watch mode, and reload when I modify the original ts source. If you could help with this that would save me a lot of time :)

[epoberezkin]

You can replace the file pattern in the command with the specific file name - not sure what watch mode mean? Rerunning the tests when files change? If so - I don’t use it - there should be multiple utilities for it.

[hengnee]

Hi, sorry for commenting under a closed thread.

I would like to ask if there is an option to included errors Array after generation of code as I see the errors are stored in vErrors variable and only accessible if I replaced the text with another code.

const validations = moduleCode({CreateCatDTO: 'CreateCatDTO'});
const generatedValidator = eval(validations.replace(/return errors === 0;}/, 'return {success:!errors, errors: vErrors}}'));
console.warn('standaloneCodeValidations', generatedValidator(newCat));

[epoberezkin]

This issue is open btw.

The errors are available via the errors property of the validation function - see the examples in Getting started.

[hengnee]

This issue is open btw.

The errors are available via the errors property of the validation function - see the examples in Getting started.

Oh what I meant was that after using the import standaloneCode from 'ajv/dist/standalone'; to generate the string of validation code, I wasn't able to specifically retrieve the Array of errors unless I replace the string where it returns return errors === 0; to return {success:!errors, errors: vErrors}

And from the documentation I wasn't able to find a flag that allowed CodeGen to return the errors along with the success/failed validation results.

I apologise in advance if I missed out on any part of the explanation or misunderstood anything.

[epoberezkin]

The array of errors is available in a property of the validation function.

See this example: https://ajv.js.org/guide/getting-started.html#basic-data-validation

it’s the same for function imported from standalone code.

[hengnee]

The array of errors is available in a property of the validation function.

See this example: https://ajv.js.org/guide/getting-started.html#basic-data-validation

it’s the same for function imported from standalone code.

Ahhh had been looking at the problem differently.
I understand what you mean now.

[o-alexandrov]

Could you please share your plans on this feature?

• it would be great to pass an option to remove a block that removes unevaluated properties at the end of the standalone function

As a temporary workaround, when generating a standalone function, I'm currently: getting a list of different props between oneOf, adding missing props to every schema and then remove the code block from the generated standalone function, so it doesn't check again whether object is an object and removes all properties.

Here is how auto addition of the missing props to all schemas could be done:

if (schema.oneOf || schema.anyOf) {
  let key!: "oneOf" | "anyOf"
  if (schema.oneOf) key = `oneOf`
  if (schema.anyOf) key = `anyOf`

  const differentProps = new Set<string>()
  for (const option of schema[key]) {
    for (const prop of Object.keys(option.properties)) {
      if (differentProps.has(prop)) differentProps.delete(prop)
      else differentProps.add(prop)
    }
  }

  for (const option of schema[key]) {
    differentProps.forEach((prop) => {
      if (!option.properties[prop]) {
        option.properties[prop] = {}
      }
    })
  }
}

And then the removal of the useless block (which removes unevaluated properties) of the generated standalone function:

const regex = /if\(errors === 0\)(?:.|\n)*false;\n\}\n\}/
const isMatch = regex.exec(codeRaw)
if (!isMatch) throw new Error(`missing statement`)
codeRaw = codeRaw.replace(regex, ``)

[Westermann]

Hm, I don't see any activity on that PR. Any updates on this stuff?

[felixfbecker]

This would be so great to have. It would solve the long-standing problem of removeAdditional not working with allOf intersections 🙏🏻

[acSpock]

Just ran into this issue with allOf today / intersection. Plus 1 to this.