glideapps / quicktype

Generate types and converters from JSON, Schema, and GraphQL
https://app.quicktype.io
Apache License 2.0
12.43k stars 1.08k forks source link

[Typescript, JSON Schema] No union type with anyOf #1338

Open proalphaDev opened 4 years ago

proalphaDev commented 4 years ago

Using quicktype in combination with JSON Schema and typescript leads to the following issue:

anyOf does not convert to a union type in some cases.

E.g.

{
  "$ref": "#/definitions/TestObject",
  "definitions": {
    "TestObject": {
      "type": "object",
      "properties": {
        "firstProperty": {
          "anyOf": [{ "$ref": "#/definitions/MyObjectA" }, { "$ref": "#/definitions/MyObjectB" }]
        },
        "secondProperty": {
          "anyOf": [
            { "type": "array", "items": { "$ref": "#/definitions/MyObjectA" } },
            {
              "$ref": "#/definitions/MyObjectB"
            }
          ]
        }
      }
    },
    "MyObjectA": {
      "type": "object",
      "properties": {
        "a": { "type": "string" },
        "b": { "type": "number" },
        "c": { "type": "boolean" }
      }
    },
    "MyObjectB": {
      "type": "object",
      "properties": {
        "d": { "type": "string" },
        "e": { "type": "number" },
        "f": { "type": "boolean" }
      }
    }
  }
}

Expected:

export interface TestSchema {
    firstProperty?:  MyObjectA | MyObjectB;
    secondProperty?: MyObjectA[] | MyObjectB;
}

export interface MyObjectA {
    a?: string;
    b?: number;
    c?: boolean;
}

export interface MyObjectB {
    d?: string;
    e?: number;
    f?: boolean;
}

Actual result:

export interface TestSchema {
    firstProperty?:  MyObject;
    secondProperty?: MyObjectA[] | MyObjectB;
}

export interface MyObject {
    a?: string;
    b?: number;
    c?: boolean;
    d?: string;
    e?: number;
    f?: boolean;
}

export interface MyObjectA {
    a?: string;
    b?: number;
    c?: boolean;
}

export interface MyObjectB {
    d?: string;
    e?: number;
    f?: boolean;
}

The union in the secondProperty (array with the object type) works as expected. However the union with the two object types generates a combined interface with a weaker type than the expected union type.

DrMattFaulkner commented 4 years ago

@proalphaDev did you ever solve this problem?

proalphaDev commented 4 years ago

@proalphaDev did you ever solve this problem?

Not yet. We had to fix this manually by adding our own union types to our project. However, if we change the JSON Schema, we also have to remember these types and change them as well :( Currently, we evaluate alternatives for quicktype.

mike-marcacci commented 4 years ago

Indeed, I have encountered this as well, which makes it impossible to add type discriminators; see this example.

abbottdev commented 4 years ago

Also hitting this - but with an items array: https://app.quicktype.io?share=nVtT36Kx7AvPXAObKYVM

cmawhorter commented 9 months ago

i found this PR that didn't make it in and i didn't author. it includes some work to avoid flattening the unions which seems to work as a SUPER hacky patch. in my very narrow use, this seems to work:

import {
  StringTypeMapping,
  Type,
  emptyTypeAttributes,
  UnionType
} from 'quicktype-core';
import { TypeGraph, TypeRef, derefTypeRef } from 'quicktype-core/dist/TypeGraph';
import { UnifyUnionBuilder, unifyTypes } from 'quicktype-core/dist/UnifyClasses';
import { GraphRewriteBuilder } from 'quicktype-core/dist/GraphRewriting';
import { ok } from 'assert';
import { setFilter, iterableSome } from "collection-utils";
import { makeGroupsToFlatten } from 'quicktype-core/dist/TypeUtils';
import { IntersectionType } from 'quicktype-core/dist/Type';
// eslint-disable-next-line @typescript-eslint/no-var-requires
const FlattenUnionsModule = require('quicktype-core/dist/rewrites/FlattenUnions');

const shouldFlattenUnions = false;

FlattenUnionsModule.flattenUnions = function flattenUnions(
  graph: TypeGraph,
  stringTypeMapping: StringTypeMapping,
  conflateNumbers: boolean,
  makeObjectTypes: boolean,
  debugPrintReconstitution: boolean
): [TypeGraph, boolean] {
  let needsRepeat = false;

  function replace(types: ReadonlySet<Type>, builder: GraphRewriteBuilder<Type>, forwardingRef: TypeRef): TypeRef {
      const unionBuilder = new UnifyUnionBuilder(builder, makeObjectTypes, true, trefs => {
          ok(trefs.length > 0, "Must have at least one type to build union");
          trefs = trefs.map(tref => builder.reconstituteType(derefTypeRef(tref, graph)));
          if (trefs.length === 1) {
              return trefs[0];
          }
          needsRepeat = true;
          return builder.getUnionType(emptyTypeAttributes, new Set(trefs));
      });
      return unifyTypes(types, emptyTypeAttributes, builder, unionBuilder, conflateNumbers, forwardingRef);
  }

  const allUnions = setFilter(graph.allTypesUnordered(), t => t instanceof UnionType) as Set<UnionType>;
  const nonCanonicalUnions = setFilter(allUnions, u => !u.isCanonical);
  let foundIntersection = false;
  const groups = makeGroupsToFlatten(nonCanonicalUnions, members => {
      ok(members.size > 0, "IRNoEmptyUnions");
      if (iterableSome(members, m => m instanceof IntersectionType)) {
        // FIXME: This is stupid.  `flattenUnions` returns true when no more union
        // flattening is necessary, but `resolveIntersections` can introduce new
        // unions that might require flattening, so now `flattenUnions` needs to take
        // that into account.  Either change `resolveIntersections` such that it
        // doesn't introduce non-canonical unions (by using `unifyTypes`), or have
        // some other way to tell whether more work is needed that doesn't require
        // the two passes to know about each other.
        foundIntersection = true;
        return false;
    }

    if (
        !shouldFlattenUnions &&
        members.size > 1
    ) {
        // If the target language supports unions of classes we can avoid
        // union flattening.
        //
        // FIXME: This is suboptimal because it also completely skips
        // merging of number types based on `conflateNumbers`. The proper
        // place for this logic would be in UnionBuilder/UnionAccumulator,
        // but that module would have to be refactored to allow for
        // multiple variants with the same typekind.
        return false;
    }

    return true;
  });
  graph = graph.rewrite("flatten unions", stringTypeMapping, false, groups, debugPrintReconstitution, replace);

  // console.log(`flattened ${nonCanonicalUnions.size} of ${unions.size} unions`);
  return [graph, !needsRepeat && !foundIntersection];
}

by doing import './that_patch' before quicktype, i get this output, which seems about right:

export interface Example {
  firstProperty?:  MyObjectA | MyObjectB;
  secondProperty?: MyObjectA[] | MyObjectB;
  [property: string]: any;
}

export interface MyObjectA {
  a?: string;
  b?: number;
  c?: boolean;
  [property: string]: any;
}

export interface MyObjectB {
  d?: string;
  e?: number;
  f?: boolean;
  [property: string]: any;
}