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

fix(core): strict function types for http server types #44

Merged
merged 4 commits into from Jun 3, 2018

Conversation

JozefFlakus
Copy link
Member

PR Type

What kind of change does this PR introduce?

[x] Bugfix
[ ] Feature
[ ] Code style update (formatting, local variables)
[x] 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?

  • Typescript typing error, HttpRequest do not match with http.IncomingMessage in strict mode when used withhttp.createServer().
  • url parameter is marked as optional but should be an required parameter because we are using http.server

Issue Number: #43

What is the new behavior?

  • resolved an issue with strict function types by removing HttpResponse and HttpRequest declarations in httpListener handler function.
  • bumped up TypeScript version to 2.9.1
  • addedurl to HttpRequest interface as not optional parameter
  • simplified example server bootstrapping

Does this PR introduce a breaking change?

[ ] Yes
[x] No

@JozefFlakus JozefFlakus added bug Something isn't working enhancement New feature or request example Example related changed / improvements labels Jun 2, 2018
@JozefFlakus JozefFlakus added this to the v0.4.1 milestone Jun 2, 2018
@JozefFlakus JozefFlakus self-assigned this Jun 2, 2018
@JozefFlakus JozefFlakus added this to To do in Roadmap to 1.0.0 via automation Jun 2, 2018
@JozefFlakus JozefFlakus requested a review from a team June 2, 2018 21:02
@codecov
Copy link

codecov bot commented Jun 2, 2018

Codecov Report

Merging #44 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff          @@
##           master    #44   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          24     24           
  Lines         291    291           
  Branches       35     35           
=====================================
  Hits          291    291
Impacted Files Coverage Δ
packages/core/src/http.interface.ts 100% <ø> (ø) ⬆️
.../core/src/operators/matchPath/urlParams.factory.ts 100% <100%> (ø) ⬆️
...core/src/operators/matchPath/matchPath.operator.ts 100% <100%> (ø) ⬆️
packages/core/src/http.listener.ts 100% <100%> (ø) ⬆️
packages/core/src/response/response.handler.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 cb0629e...c7d5cc8. Read the comment docs.

@JozefFlakus JozefFlakus changed the title bugfix: strict function types for http server types fix(core): strict function types for http server types Jun 2, 2018
@JozefFlakus JozefFlakus moved this from To do to In progress in Roadmap to 1.0.0 Jun 2, 2018
@@ -35,5 +35,5 @@ export const httpListener = ({

effect$.subscribe();

return (req: HttpRequest, res: HttpResponse) => request$.next({ req, res });
return (req, res) => request$.next({ req, res });
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did you remove types?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because of strict function types for TS compiler. http.IncomingMessage does not conform to HttpRequest which is a wrapper for it.

@JozefFlakus JozefFlakus merged commit 79cb1fc into master Jun 3, 2018
Roadmap to 1.0.0 automation moved this from In progress to Done Jun 3, 2018
@JozefFlakus JozefFlakus deleted the bugfix/strictFunctionTypes branch June 3, 2018 11:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request example Example related changed / improvements
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

2 participants