nearst / laconia

Create well-crafted serverless applications, effortlessly.
http://laconiajs.io
Apache License 2.0
326 stars 30 forks source link

Ability to create an adapter dynamically #124

Open ceilfors opened 5 years ago

ceilfors commented 5 years ago

There are a lot of cases where users are not able to use laconia's built-in adapters. For example when creating an API endpoint, users are unable to create PUT or PATCH endpoint with the built-in adapters as we only support the inputType of body or params at the moment i.e. PUT might require body and params.

The current adapter for params is also forcing an object to be passed as an input, and sometimes destructuring just a single parameter feels quite unnatural. For example if a user is trying to get id path parameter, it will look like:

const app = ({ id }) => {}

Sometimes I'd like to have complete control over the signature of my app, and if I would like my signature like the following, I won't be able to use the current built-in adapter:

const app = (id) => {}

In Java world, this can be solved by using annotation, which may look like the following.

const app = (@PathVariable('id') id, @Body automaticallyParsedHttpBody, @PathVariable('foo') foo) => {}

As this is unsupported in JavaScript, how can we allow users to do this? Thoughts?

ceilfors commented 5 years ago

I have just discovered nestjs and really liked the decorator concept that it is using for JavaScript.

See this example code from nestjs

```js import { Controller, Get, Post, Body, Bind, Dependencies } from '@nestjs/common'; import { CatsService } from './cats.service'; @Controller('cats') @Dependencies(CatsService) export class CatsController { constructor(catsService) { this.catsService = catsService; } @Post() @Bind(Body()) async create(createCatDto) { this.catsService.create(createCatDto); } @Get() async findAll() { return this.catsService.findAll(); } } ```

If we adopt a similar concept in laconia, we would be able to encourage developers to design their application first, and think about laconia later which is perfect. It might look like this:

const config = require("@laconia/config");

@Dependencies("mySecret", CatsService) // Sequence matters here. String for name based injection, constructor for type based injection. We can start with just names first which is what we support as of now
class MyAppFunction {
  constructor(secret, catsService) {
    this.secret = secret
    this.catsService = catsService;
  }

  @Function() // Let Laconia know which method to be called upon handler call
  @Adapter(ApiGateway.HTTP) // Let Laconia know which adapter should be used
  @Bind(@Body(), @Param('id')) // Sequence matters here. Adapter will parse the event and inject the request body and id parameter to the method
  execute(input, id) {
    console.log(input, id, catsService) // do something with input and dependencies
  }
}

const handler = laconia(MyAppFunction)
  .register([config.envVarInstances(), instances])

A couple of caveats:

@laconiajs/contributors Any thoughts on the design and caveat above? Has anyone used JavaScript decorators before?

ceilfors commented 5 years ago

After a debate with @uncinimichel, I have counter-arguments for the usage decorator:

Looking at the code again, it seems like the key concept in nestjs is the @Bind, which is something that we can introduce in the adapter level:

const app = (id, foodOrder) => {}

const apigateway = adapterApi.apigateway({
  binding: [
    adapterApi.apigateway.binding.pathVariable('id'),
    adapterApi.apigateway.binding.body
  ] // Similar to react prop types
});

exports.handler = laconia(apigateway(app)).register(instances);

We can also actually just let the users decide fully on the mapping by providing a function:

const app = (id, name) => {}

const apigateway = adapterApi.apigateway({
  mapping: (parsedRequest) => [parsedRequest.params.id, parsedRequest.body]
);

exports.handler = laconia(apigateway(app)).register(instances);
ljcoomber commented 5 years ago

Is the use of destructuring assignment that unnatural? I guess it's quite new but I would expect it to become increasingly familiar with JS devs. I would have thought that adding decorators was much more unnatural for JS devs and currently adds another dependency.

I like the idea of user defined mappings for those that don’t want to use destructuring. It’s explicit and intuitive for anyone who knows vanilla-JS.

Native support for decorators is coming soon so think it would be worth re-examining it then. Given the proposal appears to be syntactic sugar for calling an unary-function, I wonder if there is a way of adding this capability so that the same approach can be used for explicit decoration and use of the annotation?

ceilfors commented 5 years ago

On the point of being natural or not, I guess I wonder how much of nestjs users are actually using the JS version of its capability (versus TypeScript). I have to agree with you on this one that it's quite unnatural, I'm quite biased with my Java background.

On the native support and dependency, nestjs is using babel to transpile the decorator, so the dependency is not actually needed in runtime. To make it super easy, they also have a start kit i.e. one click bootstrap for JS projects.

That's an interesting idea to support it with explicit-decorator now and support annotated-decorator later. Just to clarify my term, it may look like this:

Explicit-decorator binding:

const bind = bind([
    param('id'),
    body()
  ])
const apigateway = bind(adapterApi.apigateway({
  binding: 
}));

Annotated-decorator binding:

@Bind(@Body(), @Param('id'))

I wonder if the explicit-decorator brings much clarity as compared to the annotated ones that it's worth doing.

One of the benefits of the binding mechanism is it's declarative. Being declarative may bring other benefits when it's a typed language, how much value will it bring in JS though?

What are the disadvantages of the imperative mapping approach?

const apigateway = adapterApi.apigateway({
  mapping: (parsedRequest) => [parsedRequest.params.id, parsedRequest.body]
);
ljcoomber commented 5 years ago

Fair enough on the dependency point. But I don't class requiring Babel to use Laconia being much of an improvement ;-) (though I guess it could be optional so only if you want the decorator annotations?)

ceilfors commented 5 years ago

Agreed, maybe this is too much of an overkill. Seems like the imperative approach would work. Are there disadvantages that we can think of?

hugosenari commented 4 years ago

add .on("mapping", parsedRequest => [parsedRequest.params.id, parsedRequest.body]) to #116 ;-)

ceilfors commented 4 years ago

@hugosenari Thanks for contributing here. I feel that .on() is more suitable towards events, and the mapping doesn't feel like one of it. It feels a little unnatural to include a logic that we will apply by method chain, unless the entire flow is managed by a method chain e.g. ['an', 'array'].filter().map().reduce().

hugosenari commented 4 years ago
const apigateway = adapterApi.apigateway({
  mapping: (parsedRequest) => [parsedRequest.params.id, parsedRequest.body]
);

Looks good to me, only problem is with 'mapping', term is to broad.

Roots:

Suffixes:

IE.:

ceilfors commented 4 years ago

@all-contributors please add @hugosenari for design

allcontributors[bot] commented 4 years ago

@ceilfors

I've put up a pull request to add @hugosenari! :tada: