feat(core): introducing EffectHttpResponse type

[JozefFlakus]

PR Type

What kind of change does this PR introduce?

[ ] Bugfix
[x] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[x] Build related changes
[ ] CI related changes
[ ] Documentation content changes
[ ] Other... Please describe:

What is the current behavior?

• EffectResponse interface is restricted only to HTTP protocol. It should be only represented as a base type which could be used outside, eg. in websockets protocol.

What is the new behavior?

• The basic Effect generic interface extends EffectResponse defaulting to EffectHttpResponse type - because we are targeting HTTP protocol by default. From now it allows us to build Effects, not restricted only to HTTP protocol.

export interface EffectResponse<T = any> {
  body?: T;
}

export interface EffectHttpResponse<T = any> extends EffectResponse<T> {
  status?: HttpStatus;
  headers?: HttpHeaders;
}

• removed bumped-up lint-staged to version 8.1.0 👉 removed rxjs-compat
• added support for TypeScript v3.2.2

Does this PR introduce a breaking change?

[x] Yes
[ ] No

[codecov]

Codecov Report

Merging #87 into next will not change coverage.
The diff coverage is 100%.

@@         Coverage Diff         @@
##           next    #87   +/-   ##
===================================
  Coverage   100%   100%           
===================================
  Files        45     45           
  Lines       499    499           
  Branches     62     62           
===================================
  Hits        499    499

Impacted Files	Coverage Δ
packages/core/src/http.interface.ts	100% <ø> (ø)	⬆️
packages/core/src/http.listener.ts	100% <100%> (ø)	⬆️
packages/core/src/response/response.handler.ts	100% <100%> (ø)	⬆️
packages/core/src/router/router.resolver.ts	100% <100%> (ø)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5a9f2ae...938944f. Read the comment docs.

[krzysztof-miemiec]

Since it extends Effect<HttpRequest, EffectHttpResponse>, shouldn't it be HttpErrorEffect?

[JozefFlakus]

If I will name it as HttpErrorEffect then it will be a huge breaking change because constructs like Effect, ErrorEffect, Middleware are more commonly used compared to raw Error<..., ...> definitions. 🤔

[JozefFlakus]

Maybe ErrorEffect should be more generic? Eg. like Middleware 🤔

[JozefFlakus]

Ok, if the Effect and ErrorEffect should be HTTP independent then both ErrorEffect, Middleware and base Effect interfaces should have an ability to construct new types upon it:

export interface Middleware<
  I extends HttpRequest = HttpRequest,
  O extends HttpRequest = HttpRequest,
  T = HttpResponse
> extends Effect<I, O, any, T> {}

export interface ErrorEffect<
  T extends Error = Error,
  U = HttpRequest,
  V = EffectHttpResponse,
> extends Effect<U, V, T> {}

export interface Effect<
  T = HttpRequest,
  U = EffectHttpResponse,
  V = any,
  W = HttpResponse,
> {
  (req$: Observable<T>, res: W, meta: V): Observable<U>;
}

@krzysztof-miemiec WDYT?

[JozefFlakus]

Then we can construct new Websockets-related interfaces for WsEffect, WsErrorEffect and WsMiddleware

[JozefFlakus]

Or to only define the fully compassable Effect interface and leave ErrorEffect and Middleware as it is.

export interface Effect<
  T = HttpRequest,
  U = EffectHttpResponse,
  V = any,
  W = HttpResponse,
> {
  (req$: Observable<T>, res: W, meta: V): Observable<U>;
}

Then we can create WsEffect, WsErrorEffect and WsMiddleware types basing only on ☝️ it.

[JozefFlakus]

IMO the better is the last solution. 🤔

[krzysztof-miemiec]

Hm, I think that the latter solution would be best. Default way of working with Marble would be HTTP, and WS is added as additional feature. 👌 BTW. Should we add support for server push (with http2) in Marble.js 2.0?

[JozefFlakus]

Yes, but these are our priorities for v2.0:
Websockets >> http2