Handle "list can contain null values" better"

[newhouse]

After much exploration and thought, I settled on a few things:

1. The only part from #30 worth tackling, IMO, is the part(s) regarding how arrays/lists that can have null items are handled.
2. I think the approach that you were naturally going with when we had our call was the correct one: use an anyOf approach, vs a type: ["null", "string"]. This is for a few reasons, but mostly because apparently you cannot do something like this in JSON Schema: type: ["null", "#/definitions/myCustomType"]...references like that don't work / aren't allowed.

There are a few more tidbits/cleanup that I did along the way. One thing to note is that I found this in the JSON Schema v6 specs:

An object schema with a "$ref" property MUST be interpreted as a "$ref" reference. The value of the "$ref" property MUST be a URI Reference. Resolved against the current URI base, it identifies the URI of a schema to use. All other properties in a "$ref" object MUST be ignored.

But then I've also found this in the draft specs for v8: https://tools.ietf.org/html/draft-handrews-json-schema-02#section-7.7.1.1 Which basically allows for adding additional stuff into a schema along with a $ref key...which is good.

Fixes #30

Some references: https://graphql.org/learn/schema/

An example JSON input that should be valid:

{
  "things": [{"foo":""}, null]
}

An example JSON Schema definition where the above input is valid (i.e. what I got this library to do now):

{
  "$schema": "https://json-schema.org/draft/2019-09/schema",
  "definitions": {
    "mytype": {
      "type": "object",
      "properties": {"foo": {"type": "string"}},
      "required":["foo"]
    }
  },       
  "properties": {
    "things": {        
      "type": "array",
      "items": {
        "anyOf": [
          {"$ref": "#/definitions/mytype"},
          {"type": "null"}
        ]
      }
    }
  }
}

[newhouse]

bump @wittydeveloper

[newhouse]

Changed the default to false. Cleaned up some build/compile/test related things as well...

[charlypoly]

@newhouse we already use ts-jest, I don't understand why we have to compile the project before running tests 👀

[newhouse]

@newhouse we already use ts-jest, I don't understand why we have to compile the project before running tests 👀

@wittydeveloper

The way I thought about it was this: Typescript needs to be compiled to run (of course). If we are going to compile things differently when we test the typescript (e.g. using ts-jest) than we are when we publish the package (i.e. using tsc), this just seems like an accident waiting to happen to me.

While it's possible that the risk of differences is zero, without being 100% confident that the behavior will be identical it just seems not worth the risk. At the very least, ts-jest seems to do whatever it's doing "in-memory" and does not write out files to the dist/ directory...I don't know, it just seems fishy and unnecessarily slick/magic that adds the potential for differences.

As you'll notice, the tests themselves are still written in typsescript and compiled on-the-fly by ts-jest, but when it comes down to testing our compiled code, we reach to the dist/ directory where it's already been compiled by the same method that our users will have it compiled.

If you disagree, I can put it back...or feel free to update this PR!

[newhouse]

bump @wittydeveloper

Let me know if this is still ok, or if you want me to change back!

[newhouse]

@wittydeveloper double-bump :)

[charlypoly]

@newhouse sorry, very busy these days.

I followed the official recommendations of jest: https://jestjs.io/docs/en/next/code-transformation#typescript-with-type-checking

I feel like that relying on ts-jest is pretty safe since it is just a wrapper that will use the same typescript version. Also, if we want to use tsc all the way, I feel like we could rely on jest.transform options in package.json instead of pre-compiling code.

[newhouse]

@newhouse sorry, very busy these days.

I followed the official recommendations of jest: https://jestjs.io/docs/en/next/code-transformation#typescript-with-type-checking

I feel like that relying on ts-jest is pretty safe since it is just a wrapper that will use the same typescript version. Also, if we want to use tsc all the way, I feel like we could rely on jest.transform options in package.json instead of pre-compiling code.

@wittydeveloper

All good! I have just reverted the changes I made related to jest/ts-jest and tests all still pass.

Ready to merge and release? I've been using this branch already in my project by pointing to my fork + branch in github...would love to point back to this package!

[charlypoly]

@newhouse published in graphql-2-json-schema@0.6.0

[newhouse]

@newhouse published in graphql-2-json-schema@0.6.0

Nice, thanks!!!