palantir / conjure-typescript

Conjure generator for TypeScript clients
Apache License 2.0
17 stars 16 forks source link

Conjure "set" generates a Typescript "Array" #98

Open Ricky-N opened 5 years ago

Ricky-N commented 5 years ago

What happened?

Generated typescript for conjure set specification is an Array, which can mislead developers to assume explicit ordering that is not guaranteed by the api contract.

This response object

ResponseObject:
    fields:
        myField: set<Thing>

Generates this typescript

export interface IResponseObject {
    'myField': Array<IThing>;
}

For developers who aren't referring back to the original conjure apis, but relying on the typescript definitions for code-safety, this creates opportunities for bugs that may not pop up until down-the-line when the backend service owners change internal ordering mechanics.

What did you want to happen?

The generated typescript should generate a Set that has the same semantic expectations as the original api contract specification.

ferozco commented 5 years ago

Agreed that it would be nice if we did generate Sets in bindings but there are a couple reasons we can not.

Ricky-N commented 5 years ago

Ack on the challenges -- I wanted to make sure I filed this as part of P0 remediation. The problem popped up because a FE we maintain was relying on the implicit set ordering, which worked reasonably well until the backend added a filtering step that broke that order.

In looking to the root cause, I had a hard time fully blaming "bad dev practices" as the FE api explicitly specifies an Array type, and I could imagine someone who isn't going back to reference the conjure api might make the poor choice to assume some order, and trust that ordering.

This seems like a reasonable "wontfix", or at least a "delayed until a big break" kind of thing, but I think its worth tracking.