Fix invalid TypeScript syntax for array of union

[michaelcamper]

The current implementation of npx @gel/generate interfaces based on the following schema:

module default {

    type Actor {
        name: str;
    }

    type Producer {
        name: str;
    }

    type Movie {
        title: str;
        year: int32;
        multi contributors: Actor | Producer;
    }
}

generates the following interface

...

  export interface Movie extends std.$Object {
    "title"?: string | null;
    "year"?: number | null;
    "contributors": Actor | Producer[]; // an Actor or an array of Producers
  }

...

However, I would expect this interface

...

  export interface Movie extends std.$Object {
    "title"?: string | null;
    "year"?: number | null;
    "contributors": (Actor | Producer)[]; 
  }

...

NB: This is my first contribution to an open-source project, please go easy on me. I ran the tests inside the generate package, but could not find any similar tests so I didn't add one

[gel-data-cla]

All commit authors signed the Contributor License Agreement.

[scotttrinh]

This is my first contribution to an open-source project, please go easy on me.

🎉 Amazing, and welcome! We're super friendly here and happy to have the contribution. 🙏

generates the following interface

Nice catch, and I think your solution here is the most scalable. Another approach would be to detect unions in array types and add the parens then, but there might be other constructs where we want an explicitly parenthetical union expression.

The reason why we're just catching this now, and something to consider for your own data modeling, is that union types like that are not idiomatic in the Gel schema language. Typically you'd want to use a shared parent super type like:

module default {
    abstract type IndustryPerson {
      required name: str;
    }

    type Agent extending IndustryPerson {}

    type Actor extending IndustryPerson {
      agent: Agent;
    }

    type Studio {
      required name: str;
    }

    type Producer extending IndustryPerson {}

    type Movie {
        required title: str;
        year: int32;
        multi contributors: IndustryPerson;
    }
}

That lets you do polymorphic queries like:

select Movie {
  title,
  contributors: {
    name,
    [is Actor].agent: { * },
  }
};

[scotttrinh]

RE: tests:

Maybe we can update the dbschema here

gel-js/integration-tests/lts/dbschema/default.esdl

Line 152 in 1ac54d6

link xy -> W | X | Y;

to be a multi link, which we can add an assertion for here:

gel-js/integration-tests/lts/interfaces.test.ts

Line 46 in 1ac54d6

xy?: W | X | Y | null;

---

Not a blocker, I can also add this test later, so lemme know if you want to give that a shot (running the tests locally will require going into the integration-tests/lts directory and running gel project init to get a local project instance linked to that project).

[michaelcamper]

Hey this is fun, I should contribute more often!

Thank you for your prompt reaction and great guidance @scotttrinh, I've updated the test.

I actually stumbled across this issue because the polymorphic approach did not work for my use case:

Defining these schemata

module default {
   type Survey {
     required title: str;
      multi fields: TextField | NumberField | BooleanField;
   }
     
   type TextField {
     maxLength: int32;
   }
     
   type NumberField {
     min: int64;
     max: int64;
   }
     
   type BooleanField {}
}

so that TypeScript is able to narrow down the type

survey.fields.map((field) => {
  switch (field.type) { // see not below
    case 'text':
      // field is of type "TextField"
      break;
    case 'number':
      // field is of type "NumberField"
      break;
    case 'boolean':
      // field is of type "BooleanField"
      break;
  }
})

Note: I could not find a way to add the type property, yet. I tried to leverage a computed property for that (as it would be useless to store the type in the database), but it seems computed properties cannot be used with unions.

NB: My general goal is to use the gel schemata as single source of truth for my data model and compose all other data structures based the generated interfaces

[scotttrinh]

Hey this is fun, I should contribute more often!

Please do! We have a very small handful of outside contributors who do really valuable work because you're actually using this day to day, so it's very much appreciated to get contributions and discussions from the community.

I actually stumbled across this issue because the polymorphic approach did not work for my use case:

Polymorphic queries ought to work for this case, let me know what didn't work specifically.

module default {
   type Survey {
     required title: str;
      multi fields: Field;
   }

  abstract type Field {}
     
   type TextField extending Field {
     maxLength: int32;
   }
     
   type NumberField extending Field {
     min: int64;
     max: int64;
   }
     
   type BooleanField extending Field {}
}

And then in a query:

select Survey {
  title,
  fields: {
    *,
    [is TextField].*,
    [is NumberField].*,
    [is BooleanField].*
  }
}

Note: I could not find a way to add the type property, yet. I tried to leverage a computed property for that (as it would be useless to store the type in the database), but it seems computed properties cannot be used with unions.

You can make a computed on the actual query with a __typename property which you could use as a discriminant:

select Survey {
  title,
  fields: {
    __typename := .__type__.name,
    *,
    [is TextField].*,
    [is NumberField].*,
    [is BooleanField].*
  }
}

You won't see that as a useful thing in your generated interface, but you can type this query yourself and get a nice discriminated union out of it. Feel free to hop over to Discord or open a discussion if you want to riff on this some more.