palantir / conjure-typescript

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

Serialization/deserialization layer (translate the string "NaN" to the js Number.NaN value) #48

Open iamdanfox opened 6 years ago

iamdanfox commented 6 years ago

Currently, we just do JSON.parse to turn received JSON into javascript objects but this is not very ergonomic.

Problems

  1. Conjure double types are inconvenient to use: number | "NaN" because the value NaN is represented as the JSON string "NaN" on the wire... users have to ugly translation code
  2. Conjure optional<T> types are inconvenient to use: 'optionalString'?: string | null;... we should pick one of 'undefined' or 'null' and commit to this.
  3. Conjure unions are inconvenient to use: a more idiomatic typescript approach would be to express these as Variant1 | Variant2 | Variant3 | UnknownVariant, where the type field is no longer nested.

Proposed solution

conjure-typescript should generate a serialization / deserialization function for each type (e.g. toJSON and fromJSON).

Generated FooService classes would invoke the toConjureJSON function on all body arguments before passing them to the conjure-typescript-client:

+import { IFooRequest, toFooRequestJSON } from "./fooRequest"
+import { IFooResponse, fromFooResponseJSON } from "./fooResponse"

 export class FooService {
     constructor(private bridge: IHttpApiBridge) {
     }

     public foo(body: IFooRequest, header: string, path: string, query: string): Promise<IFooResponse> {
         return this.bridge.callEndpoint<void>({
-            data: body,
+            data: toFooRequestJSON(body),
             endpointName: "foo",
             endpointPath: "/foo/{path}",
             headers: {
                 "Header": header,
             },
             method: "GET",
             pathArguments: [
                 path,
             ],
             queryArguments: {
                 "Query": query,
             },
             requestMediaType: MediaType.APPLICATION_JSON,
             responseMediaType: MediaType.APPLICATION_JSON,
-         });
+         }).then(fromFooResponseJSON);
     }
 }

The toJSON/fromJSON functions don't need to tackle all three problems initially, but I think this approach is general enough that they could be added.

dphilipson commented 6 years ago

Huh, cool. If you're willing to add a serialize/deserialize layer to the TypeScript client, then you might also consider representing Conjure datetime as a JavaScript Date.