Replace '.client' with '.socket' as the former was deprecated in 2.2.0. #1615
Conversation
|
Travis errors exactly because it has version 2.2.0 that runs |
This is backwards – it's io.js that has introduced a breaking change in a minor version and is now incompatible with This patch shouldn't be accepted until and unless it adds support for |
Exactly what I had in my mind too. |
|
Yep both properties should be supported, and that should be implemented by simply converting the property to the one or the other before it's been used for the first time, in any of the contexts where it's used. |
|
@othiym23 |
That is indeed a bug and will be fixed in 2.2.1, as already mentioned. But it would stil emit a deprecation warning. |
res.client = res.client || res.socket |
|
@simov Why? Just why? Edit: ok. Not true for nodejs version 0.1.x (below 0.1.90): it had And setting to |
|
To summarize my discussion with @ChALkeR on IRC: my initial comment about just swapping the property names being insufficient was in error: since It shouldn't be necessary, though, because there's no compelling reason to have removed this property from the object in the first place (and any further discussion along those lines should probably be had over in nodejs/io.js anyway, as this point isn't really related to |
|
Thanks for pointing that out @othiym23 I was wondering if I'm missing out something, while I was catching up with the issue. |
|
The deprecation was completely reverted in 2.2.1. When I was filing this PR, I assumed that the deprecation would be fixed instead (as nodejs/node#1852 was doing at that moment). With this, there is no actual reason to merge this, except for using a (poorly) documented property instead of a non-documented one. If you want to merge this, I could change the commit message. If not, just close it.
I consider |
Replace '.client' with '.socket' as the former was deprecated in 2.2.0.
|
I just saw that request is a bundled dependency in npm .. anyway 2.57 is out |
|
Thanks. |
… real requests `ClientMessage.socket` is an instance of our `socket` class. As that class’ constructor sets `authorized` to `true` on `https` requests, the code here that sets `response.socket.authorized` is pointless. `ClientMessage.client` was deprecated in iojs 2.2 and per request/request#1615 but is still aliased: https://github.com/nodejs/node/blob/2e613a9c301165d121b19b86e382860323abc22f/lib/_http_incoming.js#L67 The code to set `response.client.authorized` is being replaced with that aliasing. This ensures that `response.client` and `response.socket` point to the same thing, as is the case with a non-overridden request. This may help with compatibility with libraries which depend on that undocumented property, such as some very old versions of `request`. This also ensures that `response.client.authorized` remains set, without needing to set it directly. This is being changed now to remove two bits of unreachable code in the conditional.
… real requests `ClientMessage.socket` is an instance of our `socket` class. As that class’ constructor sets `authorized` to `true` on `https` requests, the code here that sets `response.socket.authorized` is pointless. `ClientMessage.client` was deprecated in iojs 2.2 and per request/request#1615 but is still aliased: https://github.com/nodejs/node/blob/2e613a9c301165d121b19b86e382860323abc22f/lib/_http_incoming.js#L67 The old code, which sets `response.client.authorized`, is being replaced with new code which aliases `response.client`. This ensures that `response.client` and `response.socket` point to the same thing, which provides parity with a non-overridden request. This may help with compatibility with libraries which depend on that undocumented property, such as some very old versions of `request`. This also ensures that `response.client.authorized` remains set, without needing to set it directly. This is being changed now to remove two bits of unreachable code in the conditional.
… real requests `ClientMessage.socket` is an instance of our `socket` class. As that class’ constructor sets `authorized` to `true` on `https` requests, the code here that sets `response.socket.authorized` is pointless. `ClientMessage.client` was deprecated in iojs 2.2 and per request/request#1615 but is still aliased: https://github.com/nodejs/node/blob/2e613a9c301165d121b19b86e382860323abc22f/lib/_http_incoming.js#L67 The old code, which sets `response.client.authorized`, is being replaced with new code which aliases `response.client`. This ensures that `response.client` and `response.socket` point to the same thing, which provides parity with a non-overridden request. This may help with compatibility with libraries which depend on that undocumented property, such as some very old versions of `request`. This also ensures that `response.client.authorized` remains set, without needing to set it directly. This is being changed now to remove two bits of unreachable code in the conditional.
… real requests `ClientMessage.socket` is an instance of our `socket` class. As that class’ constructor sets `authorized` to `true` on `https` requests, the code here that sets `response.socket.authorized` is pointless. `ClientMessage.client` was deprecated in iojs 2.2 and per request/request#1615 but is still aliased: https://github.com/nodejs/node/blob/2e613a9c301165d121b19b86e382860323abc22f/lib/_http_incoming.js#L67 The old code, which sets `response.client.authorized`, is being replaced with new code which aliases `response.client`. This ensures that `response.client` and `response.socket` point to the same thing, which provides parity with a non-overridden request. This may help with compatibility with libraries which depend on that undocumented property, such as some very old versions of `request`. This also ensures that `response.client.authorized` remains set, without needing to set it directly. This is being changed now to remove two bits of unreachable code in the conditional.
… real requests `ClientMessage.socket` is an instance of our `socket` class. As that class’ constructor sets `authorized` to `true` on `https` requests, the code here that sets `response.socket.authorized` is pointless. `ClientMessage.client` was deprecated in iojs 2.2 and per request/request#1615 but is still aliased: https://github.com/nodejs/node/blob/2e613a9c301165d121b19b86e382860323abc22f/lib/_http_incoming.js#L67 The old code, which sets `response.client.authorized`, is being replaced with new code which aliases `response.client`. This ensures that `response.client` and `response.socket` point to the same thing, which provides parity with a non-overridden request. This may help with compatibility with libraries which depend on that undocumented property, such as some very old versions of `request`. This also ensures that `response.client.authorized` remains set, without needing to set it directly. This is being changed now to remove two bits of unreachable code in the conditional.
… real requests `ClientMessage.socket` is an instance of our `socket` class. As that class’ constructor sets `authorized` to `true` on `https` requests, the code here that sets `response.socket.authorized` is pointless. `ClientMessage.client` was deprecated in iojs 2.2 and per request/request#1615 but is still aliased: https://github.com/nodejs/node/blob/2e613a9c301165d121b19b86e382860323abc22f/lib/_http_incoming.js#L67 The old code, which sets `response.client.authorized`, is being replaced with new code which aliases `response.client`. This ensures that `response.client` and `response.socket` point to the same thing, which provides parity with a non-overridden request. This may help with compatibility with libraries which depend on that undocumented property, such as some very old versions of `request`. This also ensures that `response.client.authorized` remains set, without needing to set it directly. This is being changed now to remove two bits of unreachable code in the conditional.
Actually, it's not only deprecated, checking it with
.hasOwnProperty('client')is broken in 2.2.0, but that's going to be fixed in 2.2.1.See nodejs/node#1572 nodejs/node#1850 nodejs/io.js#1852/files