Skip to content

Conversation

@vadyvas
Copy link

@vadyvas vadyvas commented Oct 22, 2025

What issue does this pull request resolve?

fix for: redocly cli #1737

What changes did you make?

Changes in unevaluatedProperties.ts to skip the forced static check in anyOf/oneOf. This removes false unevaluatedProperties errors and allows schema to pass validly with the current example:

const schema = {
  type: "object",
  properties: {
    address: {type: "string"},
  },
  anyOf: [
    {
      type: "object",
      required: ["firstName", "lastName"],
      properties: {
        firstName: {type: "string"},
        lastName: {type: "string"},
      },
    },
    {
      type: "object",
      required: ["name"],
      properties: {
        name: {type: "string"},
      },
    },
  ],
};

const data = {
  address: "someWhere",
  firstName: "John",
  lastName: "Doe",
};

ajv.validate(schema, data); // should be true

Is there anything that requires more attention while reviewing?

gen.if(_`${N.isAllOfVariant} === 0`, staticCheck)
} else {
staticCheck()
if (!it.compositeRule || cxt.schema !== undefined) {
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In composite rules run static check only when schema explicitly provides unevaluatedProperties

)

if (isForced && it.errorPath.emptyStr()) {
if (isForced && it.errorPath.emptyStr() && !it.compositeRule) {
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Skip forced static check under composite rules (anyOf/oneOf)

@vadyvas
Copy link
Author

vadyvas commented Oct 22, 2025

tested in redocly-cli and monorepo. all tests pass

@vadyvas vadyvas marked this pull request as ready for review October 22, 2025 11:08
RomanHotsiy
RomanHotsiy previously approved these changes Oct 22, 2025
Copy link
Member

@RomanHotsiy RomanHotsiy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 LGTM

@RomanHotsiy
Copy link
Member

Does original AJV has the same problem if the unevaludatedProperties is added to the root schema? Consider contributing to their repo if it does.

Did you also run ajv tests?

@vadyvas
Copy link
Author

vadyvas commented Oct 23, 2025

@RomanHotsiy

Does original AJV has the same problem if the unevaludatedProperties is added to the root schema? Consider contributing to their repo if it does.

No, the issue doesn’t reproduce in the original AJV (see screenshots below).
The error is caused by our “forced” check of unevaluatedProperties inside a branch, which doesn’t exist in the original repo. Therefore, a contribution to the AJV repo isn’t needed

image image image

Did you also run ajv tests?

yes, all pass

image

tatomyr
tatomyr previously approved these changes Oct 28, 2025
@vadyvas vadyvas dismissed stale reviews from tatomyr and RomanHotsiy via cb9778c October 29, 2025 10:37
@vadyvas vadyvas force-pushed the fix/unevaluated-properties-anyof-oneof branch from cb9778c to 00c32b6 Compare October 29, 2025 11:00
@vadyvas vadyvas merged commit 33ac332 into master Oct 29, 2025
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants