Skip to content
This repository has been archived by the owner on May 7, 2021. It is now read-only.

Extract loading support into library and reimplement reactively #145

Open
pmuir opened this issue Mar 19, 2017 · 5 comments
Open

Extract loading support into library and reimplement reactively #145

pmuir opened this issue Mar 19, 2017 · 5 comments
Assignees
Milestone

Comments

@pmuir
Copy link
Contributor

pmuir commented Mar 19, 2017

The loading feature is interesting, and something we should adopt for the whole platform, but I think we need to reimplement it to be suitable for reactive streams rather than stateful webapps.

I would propose that all services should return an observable of a wrapper object

export class ResultWrapper<T> {
   val: T;
   loading: boolean
}

Each request to a service would result in two emissions; the first, which happens immediately on method return, would be

{
   result: null;
   loading: true;
}

Once the service actually receives data, it then emits:

{
   result: val;
   loading: false;
}

If we decide to add additional metadata to results (for example, validation information, authz info etc.) we can do this without breaking the API.

@pmuir
Copy link
Contributor Author

pmuir commented Mar 19, 2017

/cc @jstrachan

@pmuir pmuir modified the milestones: Summer, fabric8-ui integration Mar 20, 2017
@jstrachan
Copy link
Contributor

Agreed with the general idea - not yet totally sure the right implementation approach.

I still think it might be easier on the user to have 2 streams though; the loading and the data; as then its easier to expose the loading + entities properties into your controllers; rather than having loads of reactive ResultWrapper objects you need to navigate inside to get at the data. e.g. the loading indicator is often used from a generic UI component only (to show/hide the spinner thingy) then the rest of the UI just wants to iterate over the Observable of the entities & doesn't really care about the loading indicator. e.g. most of the UI stuff just wants to work on the Observable of the entities really. e.g.
https://github.com/fabric8-ui/fabric8-runtime-console/blob/master/src/app/kubernetes/ui/service/list-page/list-page.service.component.ts#L13 then https://github.com/fabric8-ui/fabric8-runtime-console/blob/master/src/app/kubernetes/ui/service/list/list.service.component.html#L3 and https://github.com/fabric8-ui/fabric8-runtime-console/blob/master/src/app/kubernetes/ui/service/list/list.service.component.html#L4

I wonder if having a little helper object that wraps up the 2 streams might be easier to reuse & make it easier to combine things together? e.g. the same composition function can be used on the loading streams & then developers just need to figure out how to compose the entities from 2 streams?

A little on the fence really ;) - but for sure I think we need to make it much easier to compose stuff. e.g. I found myself writing a 'store' (though its not really a Store is it ;) - to compose Service and Routes lately:
https://github.com/fabric8-ui/fabric8-runtime-console/blob/master/src/app/kubernetes/store/route.service.store.ts#L8

it literally is just a class which has 2 streams; but internally does the composition from 4 of them in a couple of simple lines of code to grok.

Though I guess having a single Observable does reduce the subscriptions somewhat so maybe its not a biggie after all?

@pmuir
Copy link
Contributor Author

pmuir commented Mar 21, 2017

I think I find transformations and filters easy on streams, but combinations hard.

For example, with a wrapper you can do:

pipelineStore.loadAll().filter(wrapper => !wrapper.loading).map(wrapper => wrapper.result); to get extract the just the loaded results

@pmuir
Copy link
Contributor Author

pmuir commented Mar 21, 2017

I'm going to try to implement a POC of this today.

@jstrachan
Copy link
Contributor

cool!

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

No branches or pull requests

3 participants