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

Allow app developers to configure custom Express middleware #1293

Closed
5 tasks done
hacksparrow opened this issue Apr 27, 2018 · 10 comments
Closed
5 tasks done

Allow app developers to configure custom Express middleware #1293

hacksparrow opened this issue Apr 27, 2018 · 10 comments
Assignees
Labels
feature parity feature REST Issues related to @loopback/rest package and REST transport in general user adoption

Comments

@hacksparrow
Copy link
Contributor

hacksparrow commented Apr 27, 2018

Allow Application and Extensions developers to mount Express middleware at application level.

WORKAROUND:

Our current recommendation is to create a top-level Express application (where you can easily mount any middleware) and then mount LB4 as a component (middleware) in that application. See https://loopback.io/doc/en/lb4/express-with-lb4-rest-tutorial.html for detailed instructions.

Acceptance Criteria

  • A set of APIs allowing Application developers to mount custom Express middleware
  • A set of APIs allowing Extension developers to contribute Express middleware to target application
  • Documentation for both App & Extension developers
  • A clear guidance on how to ensure correct middleware ordering (this may require a spike)
  • Implementation wise, the piece invoking middleware should be implemented as part of the Sequence handling, so that errors reported by Express middleware are routed to reject action. (For example, we may introduce a new sequence action to be executed before findRoute.)

Out of scope

@dhmlau
Copy link
Member

dhmlau commented May 14, 2018

@hacksparrow , is this the same as #1253?

@bajtos bajtos changed the title Refactor @loopback/rest and @loopback/http-server Allow app developers to configure custom Express middleware May 16, 2018
@bajtos bajtos added feature Core-GA REST Issues related to @loopback/rest package and REST transport in general and removed DP3 labels May 16, 2018
@bajtos
Copy link
Member

bajtos commented Jun 25, 2018

Related: #559 (comment)

I am proposing to enhance RestServer/RestApplication with an API allowing consumers to mount static files, for example app.static(path, options). This API should leverage Express middleware server-static as an implementation detail.

With this new API in place, we can either write an API Explorer component bundling swagger-ui and calling app.static(pathToSwaggerUI), or preferably write a short guide explaining LB4 users how to add swagger-ui to their project themselves.

@bajtos
Copy link
Member

bajtos commented Jul 30, 2018

IMO, this is out of 4.0 GA scope as we have defined it. Relabeling this story as post-GA.

@bajtos bajtos added post-GA and removed LB4 GA labels Jul 30, 2018
@virkt25
Copy link
Contributor

virkt25 commented Jul 31, 2018

Simplifying usage is probably out of scope of GA but I think we should at the very least have some documentation around how a user can add some express middleware to their application (static file serving for example) by overriding the default requestHandler. -- I think that's something important for GA.

@bajtos
Copy link
Member

bajtos commented Nov 22, 2018

I feel the issue description is out of sync with the issue title. I am going to rewrite the description to match the title and focus on LB4 user experience.

Below is the original description for posterity.


Since we have decided to use Express as our underlying HTTP framework, @loopback/rest and @loopback/http-server can be refactored with the use of Express APIs.

Current Behavior

Code is not optimized to use Express APIs.

Expected Behavior

Use Express APIs and remove redundant code.

Acceptance Criteria

All HTTP stuff should be handled using Express API, wherever possible.

  • Our parseParam logic should be kept
  • When an express middleware has already parsed the JSON body, our parseParam should detect this and accept the parsed JSON body from the express middleware
  • redesign our sequence so that express middleware could handle errors thrown by our controllers (reject)
  • docs, blogs, etc.

@orshlom
Copy link

orshlom commented Jan 22, 2019

Hi @bajtos @dhmlau,
After reading this issue, here is a spike roadmap I came up with:

  1. Check the need of refactoring @loopback/rest and @loopback/http-server to be used with Express API (if still relevant).
  2. Where to register Express middleware per route, controller class or controller method (#2035).
  3. Invoke middleware handling from LB Sequence.
  4. Clear guidance on how to ensure correct middleware ordering.

What do you think?

@bajtos
Copy link
Member

bajtos commented Mar 5, 2019

Relevant work: #2434

@clayrisser
Copy link
Contributor

clayrisser commented Jun 21, 2019

I build a loopback component that lets you inject express middleware into the sequence. You can filter it in controllers by using the @middleware('auth') annotation, or you can directly inject middleware into the @middleware(myExpressMiddleware) annotation.

https://github.com/codejamninja/lb4-middleware

@raymondfeng
Copy link
Contributor

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature parity feature REST Issues related to @loopback/rest package and REST transport in general user adoption
Projects
None yet
Development

No branches or pull requests

7 participants