Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
iamdanfox opened this issue Aug 7, 2018 · 1 comment

Comments

@iamdanfox
Copy link
Contributor

iamdanfox commented Aug 7, 2018

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
Copy link

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants