Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

initial impressions/feature requests #20

Open
muturgan opened this issue Feb 29, 2020 · 1 comment
Open

initial impressions/feature requests #20

muturgan opened this issue Feb 29, 2020 · 1 comment
Labels
enhancement New feature or request

Comments

@muturgan
Copy link

hi dear team

I found out about your framework in the NodeWeekly email feed.

it is really pleasant to work with it. it is simple but powerful.

and my proposal is:
as for me it is very conveniently to handle endpoints like get /users and get /users/:id at the same controller. so I think it would make sense to add optional param with suburl to controller's methods. something like

class UsersController extends Controller {
  @method('GET')
  public users(ctx: Context) {
    ctx.response.body = users;
  }

  @method('GET')
  public singleUser('/:id', ctx: Context) {
    const id = ???;
    // for example:
    // const splitted = ctx.path.split('/');
    // const id = splitted[splitted.length - 1];
    // but shortcut will be very usefull
    ctx.response.body = users[id];
  }

or

  @get()
  public users(ctx: Context) {
    ctx.response.body = users;
  }

  @get('/:id')
  public singleUser(ctx: Context) {
    const id = ???;
    ctx.response.body = users[id];
  }

thank you for your work

@evert
Copy link
Member

evert commented Mar 1, 2020

I think we might want to do something like this by adding a @route('/foo') decorator, but not entirely sure how this would work yet. I can also see we just make a new routed-controller package.

If you're building a REST service, I would in the meantime recommend getting used to the 1 controller per route pattern. It might be a bit strange getting used to, but I feel it adds a level of clarity and flexibility that the rails-like (index/read/create/update/delete) lacks.

You might end up liking it.

However, I will keep this issue open as a feature request and figure out down the line if this is something that we really want. If this is something you absolutely need and need it now, I would suggest trying to write your own controller.

Especially if you intend to stick to the 5 rails methods that it uses for its MVC pattern, and you're consistent it's not that hard to write a simple alternative controller base-class.

To have your class recognized as a controller, all you need is one method:

https://github.com/curveball/controller/blob/master/src/controller.ts#L44

@evert evert transferred this issue from curveball/core Mar 1, 2020
@evert evert added the enhancement New feature or request label Mar 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants