Suggested API change

[abalmos]

I was thinking this would be a good time make API changes if we want. We can maintain an external layer that presents an 'oada-cache' style API ... with the hope that a new API is cleaner (more typeable) and eventually wins.

Here is an example of the API that comes to mind. This is driven by my use case in 'oada-jobs'. Would love to have a discussion on this. If you are agreeable, I would be happy to help make it so.

import { connect, /* LRU */ } from '@oada/client'

// For now just WS, but could have others on day
const conn = new OadaWS("example.oada.com");

/* Maybe "wrap" in a cache layer?
 * conn = LRU(conn);
 */
// Note: type annotation is just for example clarity
// Note: oada1, oada2, ... live at the same time and they
//       need a way store the token separately 
const oada1: OadaClient = conn.connect(token1);  
const oada2: OadaClient = conn.connect(token2);

/*
 * all of these use the same WS socket from above but are
 * authed with the correct token
 */
oada1.get(path) // Return type? Should leverage @awlayton's oada-types as possible
oada2.put(path, body, options) // Return type? Should leverage @awlayton's oada-types as possible
oada1.watch(path).on('all|new|edit|delete', (change: OadaChane, oada: OadaClient) => {
	// where `oada` is the OadaClient that recieved the change
})

EDIT: Removing clone and setToken as they are not needed. We will make conn.connect() "cheap" instead.

[abalmos]

Here is high-level oada-jobs implementation with this API (my usecase)

let conns = {
	'domain1': new OadaWs('domain1'),
	'domain2': new OadaWs('domain2')
};

let workers = [
	{
		serviceName: 'myCoOlSeRvIcE',
		domain: 'domain1',
		token: 'verySecure',
		work: (job: OadaChane, oada: OadaClient) => { /* ... */ }
	},
	{
		serviceName: 'mylesscoolservice',
		domain: 'domain2',
		token: '1337',
		work: (job: OadaChane, oada: OadaClient) => { /* ... */ }
	},
}

workers.forEach((worker) => {
	let conn = conns[worker.domain];
	conn.connect(worker.token)
	    .watch(`/bookmarks/services/${worker.serviceName}`)
	    .on('add', (job: OadaChane, oada: OadaClient) => {
			worker.work(job, oada.clone());
		});
});

[abalmos]

Actually, I just realized I don't need clone(). I can do something like:

workers.forEach((worker) => {
	let conn = conns[worker.domain];
	conn.connect(worker.token)
	    .watch(`/bookmarks/services/${worker.serviceName}`)
            .on('add', (job: OadaChane, oada: OadaClient) => {
			worker.work(job, conn.connect(worker.token));
		});

as long as 'connect' stays very light. I'm thinking 'connect' just creates an 'OadaClient' adds a reference of 'conn' internally and copies the token. Basically the same work 'clone' would need to do at this point.

[awlayton]

Please stop calling the tokens barriers. They are bearer tokens

[awlayton]

As for the return types of get, etc. we could make them generic like most of the libraries I see (e.g., axios) where you can tell it the type that the response could be. Were you thinking of something fancier?

[abalmos]

@awlayton What? The only use of "barriers" that I find is correct. The intention behind clone() is to insulate the original client from things done to the new client. For example, a .watch() called on the cloned thing should cause new events to arrive at the parents handler.

[tomohiroarakawa]

@abalmos I am fine with your proposed API. If we decide to change it, I would prefer not to keep the 'oada-cache' style API because maintaining multiple APIs is sometimes a pain...

[tomohiroarakawa]

@abalmos Currently, connect function is not very light, i.e., it creates a new websocket connection every time it is called.

[tomohiroarakawa]

@awlayton Yes, we could do that. Would you like to contribute...?

[abalmos]

As for the return types of get, etc. we could make them generic like most of the libraries I see (e.g., axios) where you can tell it the type that the response could be. Were you thinking of something fancier?

I'm not sure. The problem with that model is you get good feelings at compile time and forgot to add checks for runtime (also makes it hard to not allow any elsewhere). In this case, it is very likely the result doesn't match the type given how easy it is change things in OADA. I say that because those seems to be the primary source of bugs I had while writing services. It would be nice to know where and how it breaks ... I believe you were working on this type of system?

Could something like this work (haven't had a chance to play with it, which is why I didn't originally suggest it). Seems like it should.

function<T> get(path: string; isType: (data: any) => is a T) {
   const data = get(path);

   if (isType && !isType(data)) {
     throw "Not the right type";
  } else {
    return data;
  }

That, of course, can be called without the typeguard check but at least if you /can/ give one. I suppose it would be called like this:

const blah = wait oada.get("/bookmarks/blah", isBlah);

Where I think TS will call blah type T.

[abalmos]

@tomohiroarakawa

1. Agree on just dropping oada-cache style API ... we might need to convince others to start using this though. I guess we'll have to see.

2. In the new API OadaWS would do the websocket connection and all the OadaClients would use it. I suppose OadaWS should have some parent interface that would allow OadaClient to use an OadaHTTP, OadaMQTT, OadaSmokeSignals, etc. without change in the future.

If we can come to a reasonable answer to typing get then I can work on a PR. I was hoping @awlayton would fill us in on the types things he was working on. From the last we chatted it sounds like it was heading in a very nice direction, but then @aultac tried to kill it -- not sure where that conversation went.

[tomohiroarakawa]

@abalmos For #2: I like the idea. We have oada-client/lib/websocket.ts, which is a self-contained low-level API for websocket connection. Maybe we can extract it and make it a new API OadaWS, with a proper parent interface.

[awlayton]

you said "provide some barriers" and then gave it a bearer token.

[awlayton]

If you are wanting runtime checking of the responses then we could use the content type of the response to find the appropriate schema and check again that.

[awlayton]

Or yes we can just pass in a runtime check function and infer the type from that.

[awlayton]

It doesn't quite work as I had hoped in @oada/types just yet, but theoretically if I can get the function overrides right, then checking based on content-type would tell TS what the return type is... but that does not yet work.