to wrap or not to wrap?

[wmathes]

Now that the module structure is finally working like a charm, i'm playing around with actually using this with the shop service module. Having the native fetch response with typing still feels very clunky and i'm thinking about if we should wrap it or not...

Here's a quick example to illustrate the idea and the problem.

Creating a service package will basically consist of a class which extends Service like so

import {IResponse, Service} from "@crazy-factory/cf-service-client";

export interface IMyEndpointData {
  value: string;
}

export class MyService extends Service {
  public getPost(id: number): Promise<IResponse<IMyEndpointData>> {
    return this.client.process({
      url: `/endpoint/${id}`
    });
  }
}

So this is nice and fine. Easy to read and generate. It will pass through the Stack, reach the Fetch Middleware and return Promise containing the typed FetchResponse. All good?

Not quite... because of the nature of the fetch response object which needs a lot of caretaking to play nicely... like so

// get the service from somewhere... container, global, singleton, factory, blah
var myService = getMyServiceInstance();

// make the request, it will return a promise
myService.getPost(15).then((response) => {
    // figure out if it worked and if we have json data
    if (response.ok && response.headers.get("Content-Type").indexOf("application/json") != -1) {
        // get a promise that will get the json data
        response.json().then((data) => {
            // do something with data
            alert(`the cow says ${data.value}`);
        });
    }
});

This looks ok on a first glance, but feels kind of clunky and i can already feel the pain of having to write this code block hundreds of times. Sadly there are some good reasons behind it.

Pros

• no double standards all requests are treated equally. the promise should never reject. Using the ok- and status-properties is clean and easy.
• low performance impact as long as you don't call json() the stream will never be parsed. Parsing of unnecessary information can be skipped.

Cons

• ContentType comparison is mandatory. Because json() will try to parse the stream as json no matter what. This will result in Errors, but could by accident actually lead to parsable values (e.g. a number or an empty string).
• Duplicate code and validation logic
• Any single function call that uses the body (this includes json()) will break all future calls to body.

So how can we solve this without loosing some of the benefits of this approach?

Initially we had the FetchResponse class to handle parsing of the FetchResponse and i think we should go for this again. There should be some changes made though, which may require a few changes to the Client class (mostly to it's typings).

• implement conversion as Middleware?
• return a simple immutable object?
• it should give access to the original fetch response?
• it should only parse/fetch json when actually got?
• json data should be accessible as a get field?
• json data should be accessible multiple times and be immutable?
• different return types for data and blob endpoints?
• what do we actually need?
  request? response?
  data, text, blob?
  hasData, hasText, hasBlob?
  ok, status, statusText?
  headers?
  any additional fields?
• how to handle non-data endpoints?

How to handle middleware adding additional values to the request?

[wmathes]

This would be a nice scenario

// get the service from somewhere... container, global, singleton, factory, blah
var myService = getMyServiceInstance();

// make the request, it will return a promise
var response = myService.getPost(15).then((response) => {
  // Handle error scenarios
  if (!response.ok || !response.hasData) {
    handleMyErrorStuff();
  }

  // Handle success scenario
  alert(`the cow say ${response.data.value}`);
});

[cyberhck]

IMO, just use the promise returned by fetch. And maybe add a middleware or something which will modify the fetch data. Maybe someone will look for something to do with those native fetch promises.

[wmathes]

yes totally doable. Middleware should be the way to go. Though we would have to loosen typing on Stack as this will introduce type changes

[wmathes]

Very basic ideas for wrapping:

interface IServiceRequest

Includes everything needed to be converted to a FetchRequest. Provides a nested FetchOptions as well.

class ServiceResponse

• Wraps the FetchResponse. Accessible as fetchResponse (or something similar). Is an abstraction layer for data access.

class ServiceMiddleware

Sits on top of the FetchStack. Is added to Stack in Service-class.

• converts IServiceRequest to FetchRequest
• converts FetchResponse to ServiceResponse

Service

Has a configured client instance. Offers a request-method which accepts a IServiceRequest (for codecompletion)

[cyberhck]

My suggestion: include a wrap middleware which simply does a json, and returns a wrapped response, with a key of "body", that way user will still be able to access status code, headers (if any) etc.

When a consumer decides to use this, their SDK will also need to do the typing accordingly. cc @paibamboo

Doesn't include any more complexity, uses just plain middleware. If only we could provide a documentation on how to use it (with generics), so user has ability to use both wrap and without wrap on same generated SDK (generics should be able to solve this)

[cyberhck]

Actually I just changed my mind on that, I think we should allow 3 ways,

• User should be able to access the actual body and do .json themself if they want,
• User should be able to access just the typed parsed body
• User should be able to access wrapped body with status code, headers etc.

BUT not all 3 in one (that'd pollute the code),

so here's my proposal: include a wrapp middleware which can do

• nothing for first option (don't even include this middleware)
• just a .json() for second option
• .json into a new object for third option

(can be 2 separate middlewares)

One downside is that it'll increase a lot of typings:

Here's what it'd look like (interfaces will need to be generated for all 3 types) (generator can decide not to do all 3 and just 1 as well)

interface IWrappedUserNode {
    findById(id: number): Promise<Wrapped<User>>
}

interface IUserNode {
    findById(id: number): Promise<User>
}

interface IRawUserNode {
    findById(id: number): Promise<IFetchResponse<User>>
}

Then each node would look like this (but this won't need to be duplicated)

class UserNode implements IUserNode, IWrappedUserNode, IRawUserNode {
    public findById(id: number): Promise<any> {
        return Promise.resolve() as any;
    }
}

finally Sdk would look like this:

class SdkWrapped extends Service {
    public get user(): IWrappedUserNode {
        return new UserNode(this.client);
    }
}

class Sdk extends Service {
    public get user(): IUserNode {
        return new UserNode(this.client);
    }
}
class SdkRaw  extends Service {
    public get user(): IRawUserNode {
        return new UserNode(this.client);
    }
}

Again, the generator can actually do either of these,

Another option is to let generator decide if it even wants this middleware (or if it wants, then maybe even let them implement as well, which means it won't have to be in this repo and issue could be closed),

I'm fine either way, please express your thoughts. cc @paibamboo @wmathes