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

Dereference fails to dereference if root object has $ref #172

Closed
hixus opened this issue May 3, 2020 · 5 comments
Closed

Dereference fails to dereference if root object has $ref #172

hixus opened this issue May 3, 2020 · 5 comments

Comments

@hixus
Copy link

hixus commented May 3, 2020

Tried to deref json-schema produced by quicktype. Quicktype adds all objects to definitions and puts main object $ref to root. dereference seems to pull main object from root correctly but does not dereference any of the nested refrences. Below is and example of quicktype json-schema.

const $RefParser = require("@apidevtools/json-schema-ref-parser");
const assert = require("assert");

const schemaByQuickType = {
  $schema: "http:https://json-schema.org/draft-06/schema#",
  $ref: "#/definitions/Row",
  definitions: {
    Row: {
      type: "object",
      additionalProperties: false,
      properties: {
        a: {
          type: "integer"
        },
        b: {
          type: "integer"
        },
        c: {
          $ref: "#/definitions/C"
        }
      },
      required: ["a", "b", "c"],
      title: "Row"
    },
    C: {
      type: "object",
      additionalProperties: false,
      properties: {
        ca: {
          type: "integer"
        },
        cb: {
          type: "integer"
        }
      },
      required: ["ca", "cb"],
      title: "C"
    }
  }
};

(async () => {
  const dereferencedC = {
    type: "object",
    additionalProperties: false,
    properties: { ca: { type: "integer" }, cb: { type: "integer" } },
    required: ["ca", "cb"],
    title: "C"
  };
  const parser = new $RefParser();
  const unexpected = await parser.dereference(schemaByQuickType);
  assert.deepStrictEqual(unexpected.properties.c, dereferencedC); // is actually {'$ref': '#/definitions/C'}

  const { Row, ...restOfTheDefinitions } = schemaByQuickType.definitions;
  const expected = await parser.dereference({
    ...Row,
    definitions: restOfTheDefinitions
  });
  assert.deepStrictEqual(expected.properties.c, dereferencedC); // works as expected
})();

Example also in codesandbox https://codesandbox.io/s/unruffled-keller-jotwi?file=/src/index.js

@philsturgeon
Copy link
Member

Thanks for mentioning this. Could you provide an example output, and explain how that output differs from your expectation?

I've seen a lot of bugs which are "I see this and I think it's wrong" and that's not the most complete bug report around.

Input, expected output, actual output, then variables like node version number, etc.

If you can provide some of that, we can take a swing at either answering your question or identifying a bug,.

@hixus
Copy link
Author

hixus commented May 4, 2020

Ok, improved the example code and added link to nodejs codesandbox. Nodejs codesandbox seems to need cmd+s for the assert to be run. The problem is that for provided schema deference does not "dereference" Row's property c.

@philsturgeon
Copy link
Member

@hixus This is rather confusing because it's entirely invalid to have anything next to $ref in JSON Schema Draft 6, this was not supported until JSON Schema 2019-09.

I expect quicktype did this for you, but I dunno why. It's pretty pointless to have the root be a $ref when it's pointing to a definition inside itself anyway, why not just make the root be the root object.

const schemaByQuickType = {
  $schema: "http:https://json-schema.org/draft-06/schema#",
  type: "object",
  additionalProperties: false,
  properties: {
    a: {
      type: "integer"
    },
    b: {
      type: "integer"
    },
    c: {
      $ref: "#/definitions/C"
    }
  },
  required: ["a", "b", "c"],
  title: "Row",
  definitions: {
    C: {
      type: "object",
      additionalProperties: false,
      properties: {
        ca: {
          type: "integer"
        },
        cb: {
          type: "integer"
        }
      },
      required: ["ca", "cb"],
      title: "C"
    }
  }
};

The dereference will probably work just fine now, and this will make readability and portability a little easier.

@hixus
Copy link
Author

hixus commented May 4, 2020

Not too familiar with quicktype but I'm guessing they get root node name inferred that way. Yeah, the deref works fine if I pull the root node manually beforehand.

But if it's not valid Schema Draft 6 I will make issue for quicktype. Thanks for fast response.

@philsturgeon
Copy link
Member

Yay! Ok maybe we can put this down to quicktype and let $ref sibling behavior get more clarity in #145 as part of the JSON Schema 2019-09 efforts.

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

No branches or pull requests

2 participants