Refactor to ES6 #23
Refactor to ES6 #23
Conversation
|
|
||
| module.exports = { | ||
| getClientIpFromXForwardedFor, |
There was a problem hiding this comment.
I don't think we need to export getClientIpFromXForwardedFor. Should be a private method for getClientIp.
There was a problem hiding this comment.
Keeping as it makes it explicitly testable on its own.
There was a problem hiding this comment.
Makes sense, anything in the name of testabliity 👍
|
Very nice! Love the added JSDocs and ESLint. I'll review it Any idea why the coverage dropped from 100% to 85%, just curious? |
It's not making it to the of the block; I removed some logic that made some test-only changes to the headers. A better way would be to just mock it rather than making a request, which I can work on. |
|
That makes sense if its not making it to some of the test only logic. That'd be really cool to see a mock POC, it would definitely make the tests run much faster. We could leave the request tests as the integration tests and the mocks could the quick sanity checks. |
|
@pbojinov Updated! |
test/index.js
Outdated
| @@ -326,6 +327,56 @@ test('req.connection.socket.remoteAddress', (t) => { | |||
| }); | |||
| }); | |||
|
|
|||
| test('getClientIp - req.connection.remoteAddress', (t) => { | |||
| t.plan(1); | |||
| const found = requestIp.getClientIp({ | |||
Howdy,
As requested in #22 , I've refactored to ES6.
What changed:
.editorconfigthat matches project's existing style - see http://editorconfig.org for details2.0.0(ES6 vs the ES51.xreleases)istanbulfor successor,nyc- see https://github.com/istanbuljs/nyc for detailsCode coverage is at 84%.
Let me know if you have any questions, comments or concerns, happy to help and discuss!