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.

[abalmos]

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.

Have you played with this much? I have been wondering how it will work from TS side. Here is what I mean (and maybe I am just misunderstanding).

Let's say that /bookmarks/services/serviceA has content-type application/vnd.oada.serviceA and that @oada/types knows about that format. Then:

const data = oada.get('/bookmarks/services/serviceA');

should result in data typed as ServiceA: { name: string; } ... but then if I have

console.log(data.name);

How does TS know this is okay at compile time?

Or do you have to give a hint like:

const data = oada.get('/bookmarks/services/serviceA', 'application/vnd.oada.serviceA');

[awlayton]

It would not work in that case no.

A case where it does work if where you are expecting a specific set of content types so you can then use them is conditions

const {data, headers} = axios.get('/boomarks/foo')

switch (headers['content-type']) {
  case: 'application/vnd.foo':
    assert('application/vnd.foo', data))
    const foo = data // Has type Foo
    // Do stuff with a Foo
  break

  case 'application/vnd.bar':
    assert('application/vnd.foo', data))
    const bar = data // Has type Bar
    // Do stuff with a Bar
  break
}

As to how that manifests in the API of this library I am not sure.

[awlayton]

I am not more in love with this option than inferring the type from a validation function, I just wanted to list it. I'm not sure which is better. You could perhaps support either method, but that might be overkill.

[abalmos]

Some gut reaction thoughts:

1. I wonder if we can somehow use the tree to determine the expected content-type at compile time?

2. I think what @awlayton posted is also a good solution, but that is likely how you do it outside of @oada/client.

@awlayton will @oada/types let us fetch a type guard based on content-type? If so, we could do two things:

1. Pass a type string into get and we just assume that is a content-type and we give to @oada/types. This would be the same code path as determining content-type from the tree. TS seems to follow string enum types at compile time reasonable well...

2. We ask the client to pass a type guard in (from @oada/types or not). This is by far the easiest to type within @oada/client.

How about this: I'll work on a PR for this API surface, typed with option 2 above and then open an issue to track @oada/types and reconsider?

[awlayton]

You cannot fetch a specific type guard function from a content type currently, I suppose that could be added.

But you can already use the validate function with a content type rather than a schema id and if you have imported the corresponding types then there is an override that will tell TS that validate with that id asserts the corresponding type.

[abalmos]

Sorry if I am being dense, but are you saying this would be compatible with @oada/types:

function<T> get(path: string; assertType: (resource: any) => asserts resource is T) {
   const data = axios.get(path);
   assertType(data);

   return data;
}

and used like:

import { types } from '@oada/types'

const r = oada.get('/bookmarks/services/serviceA', types.assertType('application/vnd.oada.serviceA'));

Or maybe even

import { types } from '@oada/types'

function<T> get(path: string; assertType: (resource: any) => asserts resource is T | string) {
  const data = axios.get(path);
   
  if (typeof contentType === 'string') {
    types.assertType('application/vnd.oada.serviceA')(data);
  } else {
    assertType(data);
  }

  return data;
}

Of course I am guessing at the @oada/types API. Is that available in some pre-form that I could develop this PR off of?

[awlayton]

There is no method like types.assertType('application/vnd.oada.serviceA') to retrieve an assertion function from a content type.

You can do this:

import types from '@oada/types'

types.validate('application/vnd.oada.bookmarks.1+json', data) // Returns true or false

Also, currently if you import the type definition corresponding to the content type, TS will understand the type being checked.

import types from '@oada/types'

// Importing types adds type overrides for the validate function with their id / content type
import Bookmarks from '@oada/types/oada/bookmarks'

if (types.validate('application/vnd.oada.bookmarks.1+json', data)) {
  // TS will know data is a Bookmarks inside this if
}

I need to rework the structure to make TS understand all the types without having to import them all.

There is a WIP version of @oada/types on github.

[awlayton]

Also, if you know the type you want to assert, you do not need to bother with the content type:

import Bookmarks, { assert } from '@oada/types/oada/bookmarks'

assert(data) // Asserts data is a Bookmarks

[abalmos]

Interesting, so maybe something like this would work then

function<T = any> get(path: string; assert?: (resource: any) => asserts resource is T) {
  const data = axios.get(path);

  assert && assert(data);

  return data;
}

import Bookmarks, { assert } from '@oada/types/oada/bookmarks'

const r = oada.get<Bookmarks>('/bookmarks/services/serviceA', assert)

It is a little clunky but has benefit of allowing a local assert function if @oada/types doesn't own the type of what you are trying to get. I guess it is no worse then what I originally suggested.

If assert is being import from a path like @oada/types/oada/bookmarks, can't it be hard coded by @oada/types to assert the Bookmarks type directly? In that case the usage could be a bit simplier:

import { assert } from '@oada/types/oada/bookmarks'

const r = oada.get('/bookmarks/services/serviceA', assert)

I suppose you will normally need to rename the assert function so you can have more than one.

[awlayton]

That works as well already, you do not have to import the type for the assert function to work. That was just in my example from copy pasta.

[awlayton]

Also, there is a type in @oada/types for the assertion functions:

import type { TypeAssert } from '@oada/types'

async function get<T = any> (path: string, assert?: TypeAssert<T>): Promise<T> {
  const { data } = await axios.get(path);

  assert ?? assert(data);

  return data;
}