Feature: enable parse of model-config.js for client models extension

[pandaiolo]

This PR enables model-config.json definitions for the angular client ie enables models code sharing within current angular-sdk-client

I implemented this because full-blown browserification (like in loopback-example-full-stack) is a too heavy payload for me, and because loopback-sdk-angular works well for me.

3 main commits with comments on each of them for the details

List of main features added by each commit

1. 0cdf152 Enables client-side methods from sources defined in model-config.json
2. f9587d4 Enables non persisted models to be shared with the client as well (not angular resources, just classic objects)
3. 3ed6668 Enables declaring Dependencies used in each resources custom methods

Highlights

• No compatibility break
• Specs updated
• Tries to respect loopback 2.x workspace structure / model config
• Not really isomorphic, but for simple needs it's easier and lightweight
• Just drop client code in client/models, common code in common/models, and voila

Notes

• I added a fix for the warnings about deprecated usage of res.send express method, can remove if out of purpose
• I have updatet grunt-loopback-sdk-angular as well, incoming PR

Related: the issue #24 and a previous attempt on the implementation in #25.

[bajtos]

Hi @pandaiolo, thank you for the pull request. It is a large change, I'll need more time to review it.

Few things that stand out on the first read:

generateServices partially duplicates the logic implemented in loopback-boot and is not 100% compatible. A better approach is to use loopback-boot's function compile that prepares instructions for the bootstrap process. This may require you to move the configuration of client models to client/models-config.json, which seems like a good idea to me, since that's the approach used by the isomorphic version too.

I am rather unhappy with the API of the shared methods:

MyModel.findTwo = function (successCb, errorCb) {
   return MyModel.find({filter: {limit: 2}}, function(items, headers) {
     if (successCb)
       successCb(items, headers);
   }, errorCb);
};

It's not an idiomatic Node.js code, it's not an idiomatic promise-style code. I suppose it is close to the ngResource API when used on the client side, but that does not work at all on the server - MyModel.findTwo() does not return an object that will be populated with the result properties.

We should spend some more time here and come up with a nicer API.

Related:

• Promises in core? loopbackio/loopback-datasource-juggler#177
• Isomorphic AngularJS SDK for LoopBack [proposal] #83
• Support promises loopback#418

[bajtos]

As for the commit "Fix deprecated express res.send(status,...) calls", can you submit it as a standalone PR please? I am happy to merge that one.

[bajtos]

This may require you to move the configuration of client models to client/models-config.json, which seems like a good idea to me, since that's the approach used by the isomorphic version too.

Seems this is already impemented in this PR, at least according to the comment Apply client model config, usually from client/model-config.json

[pandaiolo]

Thanks for reviewing my PR !

About the example in the spec, that I called MyModel.findTwo actually It is not opinionated and can be changed : the method will be passed to the client whatever the spec. It's up to the developer to develop it's preferred style of methods (returning promise, or calling node-style callback).

About generateServices implementation, should I reuse compile functions to load the data-model.json, even as I introduced a slight difference - with angular dependencies?

[bajtos]

About the example in the spec, that I called MyModel.findTwo actually It is not opinionated and can be changed : the method will be passed to the client whatever the spec. It's up to the developer to develop it's preferred style of methods (returning promise, or calling node-style callback).

Here is the problem: at the moment, the usual way is to use ngResource style in the client and Node callbacks on the server.

// client
var users = User.find();
users.$promise.then(...)

// server
User.find(function(err, users) { ... })

How can I achieve the same result using the solution proposed in this patch?

[bajtos]

About generateServices implementation, should I reuse compile functions to load the data-model.json, even as I introduced a slight difference - with angular dependencies?

IIRC, the compile function does not interpret contents of per-model configuration object in model-config.json. Where the server bootstrapper expects properties like dataSource and public, you can use dependencies instead.

Let's figure out how to improve the API of the shared methods first and defer the implementation details for the next step.

[pandaiolo]

Ok I understand, yes obviously Angular $resource and LB models behaviours have little in common, and it would be nice if this SDK could be more isomorphic.

But isn't this PR on the roadmap? It's only a small step ahead, but it extends the features of current angular sdk in a way developers can take advantage of [server,common,client]/models directories introduced by LB 2.x, which is really handy

(Example : I have a persistToDb() method in client & server so the common code works well either way. Ok, a common save() method would be nicer, but it's not possible ATM anyway...)

Assuming that point of view, couldn't a future hypothetic PR, that would improve isomorphism, do it on top of these features ? It would be some kind of wrapper right ?

[bajtos]

@ritch could you please add your opinion too?

[pandaiolo]

Related to this PR, some questions I have about the boot process : https://groups.google.com/forum/#!topic/loopbackjs/Ndz_F1NxvY0

[ritch]

Wow this is a huge change. Thank you for taking the time to spell it out so clearly. I share the same concerns outlined by @bajtos.

My biggest concern is that this directly conflicts with the isomorphic approach which I would like to rebuild the angular SDK on. Can you provide a standalone example (very small) that we can go through? The examples in the test seem a bit too trivial to really see the overlap that I am concerned with. We only now know the size for full stack example after building it, I think the same thing is required here so we don't miss similar issues.

Overall I really like this idea and want to find some way to include it, document, and make it easy for people to use.

[pandaiolo]

Thanks !

I'll give a shot at a demonstrative example this WE or next week.

Also, after @bajtos explanation of the boot process, I see that I
misinterpreted it, which means this PR may not be aligned on the "Loopback
way" of booting the app : I assumed that common dir contains stuff for
both common AND client/server, for example always extending same class
Model, while instead, models config are read from either common OR
client/server, and a BaseModel is expected in common, with models
ClientModel and ServerModel extending from it.

Not sure if it's clear... however it was when @bajtos explained here :
https://groups.google.com/forum/#!searchin/loopbackjs/boot/loopbackjs/Ndz_F1NxvY0/EPzSO0kxnTMJ

Anyway, the expected structure does not seem technically in conflict with
the patch, but the patch kind of advertise a less clean structure (always
extending same Model class)

I'll try to put an example together, I let you know

2014-10-22 20:28 GMT+02:00 Ritchie Martori notifications@github.com:

Wow this is a huge change. Thank you for taking the time to spell it out
so clearly. I share the same concerns outlined by @bajtos
https://github.com/bajtos.

My biggest concern is that this directly conflicts with the isomorphic
approach which I would like to rebuild the angular SDK on. Can you provide
a standalone example (very small) that we can go through? The examples in
the test seem a bit too trivial to really see the overlap that I am
concerned with. We only now know the size for full stack example after
building it, I think the same thing is required here so we don't miss
similar issues.

Overall I really like this idea and want to find some way to include it,
document, and make it easy for people to use.

—
Reply to this email directly or view it on GitHub
#84 (comment)
.

[bajtos]

@pandaiolo sorry for taking so much time to comment here. I'd like to come up with a way how to break this PR into smaller incremental changes and start landing those that makes most sense.

The first step is to implement support for basic client/model-config.json that will allow the developer to pick which models should be included in the generated file.

// client/model-config.json
{
  "User": { /* no config yet */ },
  "Product": { }
}

The API for the the code generator and the implementation should be ideally similar to loopback-boot. Instead of passing server app to the generator, it should accept appRootDir pointing to the client folder and follow the file conventions used by loopback-boot to build up a list of models to include. Of course, we must preserve backwards compatibility and support the old API accepting a server app object too.

What's your opinion on making this part the first change to get landed?

[pandaiolo]

Yes this sounds like good first step to me too ! I'll have a look on how loopback-boot implements this.

I have another loosely related question about browser SDK as a whole. I discovered angular-data, which looks awesome. It is not as lightweight as $resource, but it's much more featured. Most interesting are models relations, in-memory key-value store for models, with caching mechanism for both individual models and collections, not to mention the firebase-like 3-way data binding.

$resource is 4K minified, angular-data 64K, a lot more, but still way more lightweight than full-stack LB payload.

My questions :

• Do you know angular-data ? Any opinion on it ?
• Any thoughts about making a loopback-angular-data like loopback-sdk-angular ? (which I am considering at the moment)

Interesting notes (after some emails with Jason the developer) :

• The project already has a framework agnostic 1.0.beta called js-data (with a planned js-data-angular). When it's ready, any loopback wrapper would enable non-angular developers to power their web app with it.
• Apparently, the development choices made on this future release are aligned with Angular 2.0 future release (which look very disruptive)

I feel like this seem completely redundant to the full-stack loopback initiative... However, as an end user, I cannot really know when it will be lightweight enough for production, if it does anytime soon - you earlier said that web clients SDK is not a big priority in SL present efforts (did that include full stack LB?).

Context: I am in the middle of a major refactoring of my app which uses Loopback, so I really have to make a call here on which solution I will use in the next weeks that can quickly land into production.

[bajtos]

I don't know anything about angular-data and thus don't have any opinion.

@ritch IIRC, you were collaborating on the design of angular data, could you please read the comment above and let us know your thoughts?

Any thoughts about making a loopback-angular-data like loopback-sdk-angular ? (which I am considering at the moment)

IMO, if you have the time to put into developing an alternate SDK based on angular-data, then go for it. If your approach turns out to be good, then we can borrow some of your ideas for the official SDK, or even make your project the official SDK for AngularJS. Even if you discover along the way that angular-data is not the right approach, then at least we will all know we have to look for different solutions.

[bajtos]

@pandaiolo I am not sure what is the status of this PR. I can see new commits, however they still implement more that what I described for the first step.

Please leave a comment when the PR is ready for another round of review.

[bajtos]

@pandaiolo could you please provide us with an update? Are you still keen to get this finished?

[bajtos]

I am closing this pull request as it seems to be abandoned. @pandaiolo let us know if you ever find time to finish this work, I am happy to reopen the pull request in such case.

[pandaiolo]

Yep, ok, I will rebase this branch for me because my app use it, but anyway I need to find some time to propose the first step as a new PR (just parse client/model-config.json). I cannot do it before probably a few weeks, maybe after seeing how the current development of the loopback boot process evolves. I'm also not giving up on developing an angular-data solution. Any under-staffed programmer plz contact me :-)