rjsf-team / react-jsonschema-form

A React component for building Web forms from JSON Schema.
https://rjsf-team.github.io/react-jsonschema-form/
Apache License 2.0
14.1k stars 2.18k forks source link

Validation error when multiple $ref references point to a single definition #4152

Open ivogonzalvez opened 5 months ago

ivogonzalvez commented 5 months ago

Prerequisites

What theme are you using?

core

Version

5.x

Current Behavior

When using a schema that has multiple $ref references pointing to a single definition, the rjsf library returns a validation error stating that "reference "#" resolves to more than one schema" and does not validate the rest of the form.

Expected Behavior

The rjsf library should be able to handle schemas where multiple $ref references point to a single definition without throwing a validation error. This is in line with how AJV v8 handles $ref references.

Steps To Reproduce

  1. Define a schema with multiple $ref references pointing to a single definition.
  2. Use this schema with the rjsf library.
  3. Observe the validation error.

Please see this shared playground link

Anything else?

The issue seems to arise from the replacement of the $refs properties with the schema in the definition. This behavior is different from how AJV v8 handles $ref references, which resolves references during the validation process and uses the $id in the reference to find the corresponding schema definition.

If I remove the $id property it will work, but I need the $id prop for caching issues, because the form is huge. AJV v8 uses the $id property as a unique identifier for each schema. It uses this identifier to cache the compiled validation functions for each schema. This means that if I have multiple schemas with the same $id, AJV will only compile the validation function for the first schema it encounters with that $id, and it will use the cached function for any subsequent schemas with the same $id. This improves performance greatly when validating multiple instances against the same schema

heath-freenome commented 5 months ago

@ivogonzalvez Interesting issue. It seems like something we may be introducing into the mix via our validator-ajv8 implementation. Using the playground and running the raw validation seems to be ok. Is this something you'd be willing to provide a fix for?

ivogonzalvez commented 5 months ago

I'm willing to try and provide a fix for this issue. However, I would appreciate some guidance on where to start. Could you please point me to the relevant parts of the codebase or any specific areas that you suspect might be causing this issue? Additionally, any information on how to set up the development environment and run tests would be very helpful. Looking forward to your guidance

heath-freenome commented 5 months ago

I'm willing to try and provide a fix for this issue. However, I would appreciate some guidance on where to start. Could you please point me to the relevant parts of the codebase or any specific areas that you suspect might be causing this issue? Additionally, any information on how to set up the development environment and run tests would be very helpful. Looking forward to your guidance

@ivogonzalvez Awesome! The contributing guide hopefully has enough information for you to proceed with setting up the dev environment. I would start looking at the validator-ajv8 package, specifically the validator.ts file. When using the playground, clicking on the Raw Validation button shows the expected errors so the additional error must be coming from the code that executes before the AJV8Validator.rawValidation() function

bryanjswift commented 4 months ago

No idea of this is a valid solution or not but adding

delete resolvedSchema['$id'];

After line 267 in https://github.com/rjsf-team/react-jsonschema-form/blob/e7b82772d4011670dd9f40049b3ba9000431de67/packages/utils/src/schema/retrieveSchema.ts#L248-L267 looks like it resolves this error. I'm going to do some more testing but patch this into the version of @rjsf/utils in my project. My assumption is that the duplicated schema doesn't need the id in the subschema.

heath-freenome commented 2 months ago

@ivogonzalvez How is that fix coming?