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

draft: Gracefully handle when multiple conditional branches are true #88

Closed
wants to merge 3 commits into from

Conversation

brennj
Copy link
Collaborator

@brennj brennj commented Aug 8, 2024

Consider the following schema:

{
  "additionalProperties": false,
  "properties": {
    "word": {
      "type": "string"
    }
  },
  "allOf": [
    {
      "if": {
        "properties": { "word": { "const": "hello" } },
        "required": ["word"]
      },
      "then": { "properties": { "word": { "maxLength": 5 } } }
    },
    {
      "if": {
        "properties": { "word": { "const": "hello" } },
        "required": ["word"]
      },
      "then": {
        "properties": { "word": { "description": "I am a description" } }
      }
    }
  ]
}

When the field word has a value of hello, you'd expect the attributes to be the following:

{
  name: 'word',
  description: 'I am a description',
  maxLength: 5
}

This in the main branch does not work at the moment correctly, because on every evaluation of successfully valid conditional branch that we apply, we also run removeConditionalStateAttributes. This means that the expected result only happens here.

But, we have no way of saying "keep the results from ALL branches before removing whats stale". It considers the previously evaluated branch as stale.

What I've done here makes a unit test for this pass... but I haven't fully battle tested. And I very much hate the solution and I'm convinced theres a better way to do this that isn't as truly abhorrent as this.

@remotecom remotecom closed this Nov 27, 2024
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.

2 participants