layrjs / layr

Dramatically simplify full‑stack development
https://layrjs.com
MIT License
1.21k stars 38 forks source link

Liaison & Wildcard #2

Closed brillout closed 4 years ago

brillout commented 4 years ago

Hi!

I'm doing something similar: https://github.com/reframejs/wildcard-api

I'd be curious about your thoughts!

I'm excited that RPC is gaining traction.

mvila commented 4 years ago

Hi @brillout!

I had a look at your project. I admire the work you have done, and especially the documentation. But, in my opinion, you should go a little further in abstracting the communication protocol.

Indeed, in Wildcard, HTTP is still there. The users have to worry about this.headers to get the authorization tokens. As a result, the communication protocol cannot be replaced easily.

For the user perspective, a function, whether executed locally or remotely, should behave exactly the same. So the RPC library should take care of:

My view of what an RPC library should provide might a bit extreme though. So, I'd be happy to know what you think.

Floriferous commented 4 years ago

Get some inspiration from Meteor Methods if you want! :)

https://guide.meteor.com/methods.html

brillout commented 4 years ago

I didn't forget about this and I will reply later :-)

brillout commented 4 years ago

I admire the work you have done

Same here — what you are doing is courageous.

HTTP is still there

You're right, authentication shouldn't be handled in RPC functions. I made changes in that regard.

No hidden parameters (no this.headers).

Wildcard uses this not as parameter but as a context object.

endpoints.whoAmI = function() {
  return 'You are: '+this.user.username;
};

The this.user context should be available to all functions and shouldn't be a parameter.

Full serialization of parameters and return values including class instances

You seem to be in fond of OOP and from that perspective it makes sense. On the other hand, I'm trying to keep everything as simple and as minimalistic as possible. I'd say that only supporting JavaScript objects is good enough for now but I'm open to support more language features in the future.

when an error happens in the backend, a generic server error should be thrown in the frontend.

This is actually already the case. I improved the Error Handling documentation to make this clearer.

 

Get some inspiration from Meteor Methods if you want! :)

Thanks for the link :). It's a pity that Meteor's RPC implementation is not usable outside of Meteor. Unfortunately Meteor is a big monolith and is not very flexible. In a high-paced env, such as JavaScript, not being flexible is fatal — you need to be able to embrace new tech and Meteor has shown difficulties in that regard. It took Meteor too long to adopt React, for example.

 

Thanks for the feedback! I've couple of thoughts regarding Liaison that I want to further digest. I'll let you know once my thoughts clarified.

Floriferous commented 4 years ago

You can use Meteor's RPC outside of Meteor now. Obviously you need a meteor server that sets up all the endpoints, but then you can just use a library like this: https://github.com/Gregivy/simpleddp

It is completely agnostic from Meteor, and let's you talk to meteor servers from any JS client :)

And you're right, Meteor has taken some time to adopt the latest things, it just released official TypeScript support a few weeks back, along with polished react-hooks support :)

mvila commented 4 years ago

Thanks, @brillout for your reply.

I see you've been active lately. The "Context API" is a nice improvement. It makes the remote functions more decoupled from the transport protocol. But to fully unify frontend and backend, I think the "Context API" should be available on the frontend side as well.

For example, let's say that when we call endpoints.getSomeData(), the authentication token could be read from the backend through this.token. Then, it would make sense to be able to set the token from the frontend with something like endpoints.token = 'abc123' (or through some sort of frontend middleware). Then, from the developer perspective, calling getSomeData() remotely would be no different than calling it locally.

About serializing instances and supporting OOP paradigms, I understand that you don't want to enter into that. It would completely distort your project, and it might be better to keep it as simple as possible.

About error handling, I read the documentation again, and I better understand now. Thanks.

But you still invite the developer to return objects instead of throwing errors. I think it is a mistake. In my opinion, it would be better to throw errors with custom attributes so the frontend has a chance to handle them in a meaningful way, and when it doesn't, they are just regular errors.

For example, in case signing in fails because the password is wrong, the backend could throw something like this:

throw Object.assign(new Error('Wrong password'), {
  code: 'WRONG_PASSWORD',
  displayMessage: 'The password you entered is incorrect.'
});

Then, Wildcard would throw an internal error while preserving the custom attributes so the frontend would be able to identify the error and properly handle it.

I did something similar in my implementation of the RealWorld example, and I think it works pretty well.

brillout commented 3 years ago

It's an interesting idea to be able to modify the context on the client side. I'll implement it if I stumble upon a use case for it. So far the main use case for the context has been to use to authenticate users. So basically:

server.login = async function(username, password) {
   if (await invalidCredentials(username, password)) {
     return;
   }

   // `this.user` is persisted
   this.user = {
     username,
   };

   // The user is now logged-in!
};

server.getUserPosts = async function() {
  // `this.user` was set by a previous `server.login` call
  const {user} = this;

  if (!user) {
    // Not logged-in
    return;
  }

  const posts = await db.getUserPosts(user.id);

  // ...
};

/* How does it work?

Telefunc persists the context by using two cookies:
 - One cookie that holds the `this.user` data.
   (Without `HttpOnly` so that the client-side code can read the data.)
 - A second cookie that holds a server-side generated signature with `MY_SCRET_KEY`.
   (With `HttpOnly` so that no browser-side JS attack can steal the credentials.)

*/

(I'm going to rename Wildcard to Telefunc)

from the developer perspective, calling getSomeData() remotely would be no different than calling it locally.

I share the sentiment although this is possible only to a certain degree. Frontend and backend are different in nature, e.g. the server is allowed to set authentication tokens whereas code ran on the client is untrusted.

The way I see it is that what is to abstract away are the transport details.

Which leads to something that has always been aching me with Layr. I really like what you are trying to do but I'm concerned about the separation of concerns.

For example, there are cases were I don't need OOP. Yet, with Layr I'm forced to use OOP. That doesn't fit for certain type of apps.

Let's imagine a React frontend. On a high level:

  1. The frontend is a mere representation of JSON data received from the backend
  2. With a couple of stateful components that may fetch new data and re-render

For 1., there is no need for ES6 classes: I iterate over the JSON data and use my React funtional components to render the data.

For 2., the states are managed by React and I again don't need any ES6 classes. (My React functional components act as classes and it is React that manages the class instances.)

In such a situation, I cannot use Layr, which is bummer!

Which leads me to my overall observation: I'd argue that Layr is too much of a monolith. Wouldn't it be great to break it down into do-one-thing-do-it-well libraries that are independent of each other?

For example, it's been a while I've been thinking of a new kind of query language NQL (https://github.com/reframejs/wildcard-api/issues/60 in case you are curious) and I deem it crucial for NQL to be independent of Telefunc.

Telefunc + NQL is a combination that aims to be better than GraphQL in all use cases. Whereas Layr would be limited: you don't like OOP? You cannot use Layr.

Don't take me wrong, I really like what you are doing and I'd love for you to have huge success.