Allow references that don't start with #/definitions/

[epicfaace]

status

Ready for review. This PR introduces a few breaking changes:

• We no longer support "recursive references to deep schema definitions" as mentioned before because that is not supported by the JSON pointer spec.
• A new key, rootSchema, has been added to the registry. registry.definitions is still available, though, to ease compatibility.

original considerations

A few considerations:

• We currently support definitions of the form #/definitions/. However, JSON Pointer standard says that absolute definition URLs should start with something like /definitions/ -- the former is only the "URI Fragment Identifier" representation of the JSON Pointer. However, Swagger does it the former way (starting with #). Not sure which one is best for us to go with? In the future, we might also want to support relative JSON Pointers, which is a consideration too.
• This PR uses the jsonpointer library to resolve references. However, this changes the behavior for what I call "recursive references to deep schema definitions":

Consider the following schema:

{
      definitions: {
          testdef: {
            $ref: "#/definitions/testdefref",
          },
          testdefref: {
            type: "object",
            properties: {
              bar: { type: "string" },
            },
          },
        },
        type: "object",
        properties: {
          foo: { $ref: "#/definitions/testdef/properties/bar" },
        },
      }

In the previous behavior (added in #556), $ref: "#/definitions/testdef/properties/bar" would actually resolve to type: "string". However, using the jsonpointer library, it says that this definition does not even exist.

Perhaps I misunderstood the JSON Pointer spec when implementing #556 - maybe it's not supposed to resolve "recursive references to deep schema definitions". Is that the case, and if so, should we go by the standard or keep with our current implementation? (backwards compatibility isn't a huge issue as this will go into v2.0.0)

any thoughts @domharrington @erunion or others?

[erunion]

Perhaps I misunderstood the JSON Pointer spec when implementing #556 - maybe it's not supposed to resolve "recursive references to deep schema definitions". Is that the case, and if so, should we go by the standard or keep with our current implementation? (backwards compatibility isn't a huge issue as this will go into v2.0.0)

That's tricky. The spec spec doesn't clarify $ref as being recursive in the way that you've got it, but there's also $recursiveRef which is for that, and RJSF doesn't support $recursiveRef right now.

I don't really want to get into putting stuff behind options, but maybe this should be? Create a new allowRecursiveRefs option, and if the pointer isn't found with jsonpointer, and this new option is enabled, fallback to the existing recursive code and document the expectation that this option is not compatible with OAS/Swagger schemas.

[epicfaace]

• We currently support definitions of the form #/definitions/. However, JSON Pointer standard says that absolute definition URLs should start with something like /definitions/ -- the former is only the "URI Fragment Identifier" representation of the JSON Pointer. However, Swagger does it the former way (starting with #). Not sure which one is best for us to go with? In the future, we might also want to support relative JSON Pointers, which is a consideration too.

Looking at the spec, it seems like JSON schema does require that $ref's are URI references (i.e. the version starting with #). So I'll change this PR accordingly.

[epicfaace]

@edi9999 this PR should be ready for review -- could you please take a look?

[edi9999]

sorry, i don't have time right now, only after January 10th

[epicfaace]

@edi9999 ?

[epicfaace]

@edi9999 can you take a look at this PR?

[edi9999]

I'm not sure to be able to read this with a lot of things to say, but it looks good to me