[typescript-angular] Allow generating discriminator based type visitor

[auke-]

When types which extend a common type and are distinguished based on a
discriminator are consumed they are often cast to their specific type
which results in error prone boilerplate code.

By generating a visitor for those kind of types the various subtypes can
be consumed in a type safe manner.

PR checklist

• Read the contribution guidelines.
• Pull Request title clearly describes the work in the pull request and Pull Request description provides details about how to validate the work. Missing information here may result in delayed response from the community.
• Run the following to build the project and update samples:

  ./mvnw clean package 
  ./bin/generate-samples.sh
  ./bin/utils/export_docs_generators.sh
  

  Commit all changed files.
  This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master.
  These must match the expectations made by your contribution.
  You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example ./bin/generate-samples.sh bin/configs/java*.
  For Windows users, please run the script in Git BASH.
• File the PR against the correct branch: master, 5.1.x, 6.0.x
• If your PR is targeting a particular programming language, @mention the technical committee members, so they are more likely to review the pull request.
  @TiFu (2017/07) @taxpon (2017/07) @sebastianhaas (2017/07) @kenisteward (2017/07) @Vrolijkx (2017/09) @macjohnny (2018/01) @topce (2018/10) @akehir (2019/07) @petejohansonxo (2019/11) @amakhrov (2020/02)

[TiFu]

Thank you for the PR!

Could we potentially add a generated sample for this?
It would be great to at least see whether the TS code compiles as we don't have any explicit tests.

[TiFu]

As you commented above, this change is incompatible with taggedUnions - could we add a check for this here, and output an error & terminate if both options are set?

I am unsure how to implement this though - maybe @macjohnny or @wing328 know more and could shed some light on how this is usually done in other generators.

[auke-]

The current implementation ignores the subtypeVisitor property when taggedUnions is set, it doesn't (or at least shouldn't) generate non-compilable code.

This could of course be changed in order to throw an exception if that is preferred.

[macjohnny]

I think an exception or at least a warning would be useful for users

[auke-]

I have added support for generating a visitor for tagged unions, so there is no need for this warning anymore.

[auke-]

Thank you for the PR!

I have implemented a visitor for the Micronaut OpenAPI generator (see https://github.com/kokuwaio/micronaut-openapi-codegen/pull/66/files) and was actually a bit surprised that the default Java generators didn't support this.

Since TypeScript doesn't allow default or overlay methods on interfaces a function is generated to mimic the visitor pattern, next to the visitor interface.

Could we potentially add a generated sample for this?
It would be great to at least see whether the TS code compiles as we don't have any explicit tests.

I picked a random example with a discriminator from the current tests, allOf_composition_discriminator:

// This interface was already generated and Cat, Dog, Lizard, Reptile and Snake all extend Pet
export interface Pet { 
    petType: string;
}

// This is the generated visitor interface
export interface PetVisitor<R> {
    visitCat(value: Cat): R;
    visitDog(value: Dog): R;
    visitLizard(value: Lizard): R;
    visitReptile(value: Reptile): R;
    visitSnake(value: Snake): R;
}

// This is the generated visit function which contains the boilerplate 
export function visitPet<R>(value: Pet, visitor: PetVisitor<R>): R {
    switch (value.petType) {
        case 'Cat':
            return visitor.visitCat(<Cat>value);
        case 'Dog':
            return visitor.visitDog(<Dog>value);
        case 'Lizard':
            return visitor.visitLizard(<Lizard>value);
        case 'Reptile':
            return visitor.visitReptile(<Reptile>value);
        case 'Snake':
            return visitor.visitSnake(<Snake>value);
        default:
            throw new Error("Unknown model discriminator '" + value.petType + "'");
    }
}

An implementation of the visitor could look like this:

class PetSoundVisitor implements PetVisitor<string> {
    visitCat(value: Cat): string {
        return value.name?value.name:"Meow";
    }
    visitDog(value: Dog): string {
        return value.bark?value.bark:"Woof";
    }
    visitLizard(value: Lizard): string {
        ...
    }
    visitReptile(value: Reptile): string {
        ...
    }
    visitSnake(value: Snake): string {
        ...
    }
}

Which can be used using the visit function:

  var myPet:Pet = ...
  var sound:string = visitPet(myPet, new PetSoundVisitor());

[topce]

@auke- Thanks for PR!
But instead of visitor pattern in my opinion it is better to use discriminated-unions
https://www.typescriptlang.org/docs/handbook/2/narrowing.html#discriminated-unions
@TiFu is this same as taggedUnions ?

[auke-]

I must say I'm pretty new to TypeScript and just applying the design patterns used in the Java world.

It looks like discriminated unions could be used, but when the discriminator is just a string (like in the example) it won't help since the TypeScript compiler doesn't know the possible values.

[topce]

@auke- I think TS compiler is very powerful so it knows possible values.
You will be surprised how good TS is for example in Java we still have NullPointerException
but not in TS.
I mean if you want to contribute to typescript generator you should learn little bit TS
You can play with TS in playground :
https://www.typescriptlang.org/play
there is also: handbook https://www.typescriptlang.org/assets/typescript-handbook.pdf
also in five minutes tutorial for OOP (Java programmers ) :
https://www.typescriptlang.org/docs/handbook/typescript-in-5-minutes-oop.html
good luck!

[auke-]

@auke- I think TS compiler is very powerful so it knows possible values.

But in the given example it doesn't since the information isn't in the generated typescript files, the petType property is of type string and nothing in the generated files defines the possible values.

[topce]

@auke-

Maybe you are right but instead of string it should be union type for example
petType : "Cat" | "Dog" | "Lizard" | "Reptile"| "Snake" ;
Did you tired with taggedUnions option?
Maybe this should be fixed not just for Angular but all TS clients
to check with other reviewers :
@TiFu @macjohnny @wing328

[auke-]

The thing is that we have to work with 3rd party OpenAPI specs which we can't change. It would be nice if the OpenAPI generator can handle all kinds of use cases.

[amakhrov]

Instead of generating

export interface Pet { 
    petType: string;
}

we should be generating

export type Pet  = Cat | Dog | Lizard | ...

Otherwise Typescript wouldn't know how to distinguish concrete pet subtypes based on the discriminator.
Although I'm not sure how well it matches recommended models structure, based on examples in OAS3.

Alternatively - if we're not able to produce correct types - codegen can generate type guards.

export interface Pet { 
    petType: string;
}

export function petIsCat(pet: Pet): pet is Cat {
  return pet.petType === 'Cat';
}

Visitor pattern looks very alien to this use case in Typescript. Java is different - afaik it doesn't have built-in ways to narrow types down.

[auke-]

From my perspective this would be a nice addition which reduces a lot of error prone boilerplate. The visitor pattern is not only useful for casting types, but provides the means to navigate complex data structures without the loss of type safety and accidentally omitting types.

With this change users can opt-in for this behavior and even mix it with other patterns like the ones you suggested.