fix(@rjsf/core): Nested allOf blocks with multiple if/then/else statements fail to render correctly

[stuartwilkes]

Reasons for making this change

As described here, if an allOf block is present at multiple levels within a schema, and the nested allOf block implements multiple if/then/else statements, the nested dependency is not correctly rendered.

This PR takes the patch provided in the linked thread and implements it within the library to fix the issue as described. It also provides a test for the issue.

The current behaviour can be seen as failing in this playground

Checklist

• I'm updating documentation
  • I've checked the rendering of the Markdown text I've added
• I'm adding or updating code
  • I've added and/or updated tests. I've run npm run test:update to update snapshots, if needed.
  • I've updated docs if needed
  • I've updated the changelog with a description of the PR
• I'm adding a new feature
  • I've updated the playground with an example use of the feature

[AlimovSV]

I think properties !== null check is redundand, isn't?

[stuartwilkes]

Thanks, you're right - this was a holdover from when I was addressing an issue that caused the patch to fail this test

[nickgros]

Could you add a comment clarifying why we have to do this new step, and what exactly we're doing? It's hard to follow exactly what's going on.

[stuartwilkes]

Something like "For each level of the dependency, we need to recursively determine the appropriate resolved schema given the current state of formData, otherwise nested allOf subschemas will not be correctly displayed"?

@AlimovSV wrote the original code here, so may want to word this more appropriately?

[AlimovSV]

finding the right words is not my best side :) I like your description

[stuartwilkes]

@nickgros Hopefully this comment resolves your query...

[nickgros]

It looks like properties will always be a different object than resolvedSchema.properties, since you declared a new properties object on line 771. So I think this would always be true.

[AlimovSV]

No, this is possible ONLY when the recursive call of retrieveSchema returns a modified schema for the specific property. Otherwise resolvedSchema will be returned unchanged.

[nickgros]

I follow that retrieveSchema could return the value referenced by propSchema under certain conditions. I think we need a deep equality check for the second condition.

[AlimovSV]

The idea of this check is to reduce creation of shallow copy for the retrieved schemas. So if a schema isn't computed schema (doesn't have any if/then/else) it will be returned unchanged (and reference equality check will be enough for this condition)

[nickgros]

Sorry if I'm being unclear, but essentially I don't think you will ever have referential equality between resolvedSchema.properties and properties because properties is always a new object.

[AlimovSV]

No, let me explain. Before loop we initialize properties variable as a new object, you right. And during the iteration (first time when we meet propSchema !== resolvedPropSchema) the condition resolvedSchema.properties !== properties will be truthy, you right too. But after we make a shallow copy of the schema with new properties container resolvedSchema = { ...resolvedSchema, properties }; during any next iteration resolvedSchema.properties !== properties will be falsy and we will not make shallow copy. The bonuse is: when resolvedSchema doesn't contain properties with computed schema we do not make shallow copy at all and return the unmodified object.

[nickgros]

🤦‍♂️ I finally see it now! Thank you for this explanation (and for bearing with me)!

[nickgros]

This looks good to me if tests are passing. I'd like another set of eyes since our schema resolution utils are so complicated