Add test for conditional to show/hide dependent values

[RuslanZavacky]

Thank you for the library! I am experimenting with it, and noticed that example at https://json-schema-form.vercel.app/?path=/story/demos-combinations--conditional-fields doesn't work when this library is used independently.

I'd expect when calling handleValidation, for the fields object to get isVisible updated correctly.

I understand that formValuesToJsonValues is used to remove fields that are in conflict with conditionals, but the result of that function is not the one you would use to render form. I might be missing something there.

I've added test that shows that field isVisible property is in unexpected state.

---

To render form elements, I rely on fields and isVisible. And I assume it is not the same logic that is used to build out demo, as it is doing something else to derive, if field is visible or not.

[sandrina-p]

Hi @RuslanZavacky , thank you a lot for this bug report and tests! Someone in our team will look into it later in a couple of days max 👀

[sandrina-p]

Hi @RuslanZavacky! I had the time now to dig into this. Explaining it is tricky, so I pushed a new commit with the data flow explanation. Look for the 📌 emoji in the code!

4871d63. Update: I amended the commit, here's the new version: d7f80b9

Spoiler: I did not fix the bug but provided alternatives. Let me know if it makes sense.

[RuslanZavacky]

Thank you @sandrina-p - yes, makes sense :) I did notice on my tests, when I switch radio option twice - it would correctly re-calculate, now I see way :) We don't use React, but Google Lit. Are you looking to figure out how to do re-calculation internally? If not, I might try to see if I can pin point where it happens and how to fix it.

[sandrina-p]

No, we don't plan to look into it in the next few days (or weeks). If you could give it a try, we'd highly appreciate. :)

But be aware that this problem doesn't get fixed by just doing a re-calculation. In this case we only have 1 condition that depends on another condition. (a chain of 2 conditions), so a re-calculation works. But given a JSON Schema with a chain of 3, 4, 5+ conditions, it means we'd need to do the re-calculation 3,4,5+ times.

I think the best solution is fixing the JSON Schema condition to account for the 2 dependent fields (has_pet and age) as shown in the unit test I created. Does that make sense to you?

[RuslanZavacky]

Yes, that makes sense :) Thank you!

[RuslanZavacky]

@sandrina-p do you think it is worth to merge this test case, or to cancel this PR? I feel like trying to solve the underlying recomputation for validation will need a separate ticket/pr, as it looks like quite a lot of effort might be needed :)

[sandrina-p]

@RuslanZavacky, answering your question here (to keep it in a thread)

@sandrina-p do you think it is worth to merge this test case, or to cancel this PR? I feel like trying to solve the underlying recomputation for validation will need a separate ticket/pr, as it looks like quite a lot of effort might be needed :)

Good question.

Internally we are fixing the demo to avoid "chained conditionals". So the new schema is like this. Should be merged in a few hours, max tomorrow.

  if: {
    properties: {
+      has_pet: {
+        const: 'yes',
+      },
      pet_age: {
        minimum: 5,
      },
    },
    required: ['pet_age'],
  },

In this PR, I'm not sure, the tests are failing so it cannot be merged. Would you mind cleaning the tests to have 1 test only, alongside code comments explaining the "redundant validation"? :)