fix: request.end accepted arguments #1591
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
I'm making this change it's own commit so I have a place to comment the findings. Digging through Node git history, I found the change that created the breaking change in nock (ref nock PR 929). nodejs/node@a10bdb5#diff-286202fdbdd74ede6f5f5334b6176b5cL779 Before Node v8, `OutgoingMessage`, which is extended by `ClientRequest`, would literally do what it says in the docs if data was provided. It would call `this.write(data, encoding)`. This meant that nock could wrap only the `write` method when recording and gather all the chunks even if the last chunk was sent to `end`. But, the above changed that to call an internal function dual used by `end` and `write`.
Fixes nock#1549 The method now correctly accepts all the permutations allowed. request.end(data, encoding, callback) request.end(data, callback) request.end(data, encoding) request.end(data) request.end(callback) request.end() And a few tests were added to ensure all cases are explicitly covered.
mastermatt
changed the title
|
🎉 This PR is included in version 11.0.0-beta.21 🎉 The release is available on: Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 11.0.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
gr2m
pushed a commit
that referenced
this pull request
Sep 4, 2019
* Fix typo about wrapping request.end. I'm making this change it's own commit so I have a place to comment the findings. Digging through Node git history, I found the change that created the breaking change in nock (ref nock PR 929). nodejs/node@a10bdb5#diff-286202fdbdd74ede6f5f5334b6176b5cL779 Before Node v8, `OutgoingMessage`, which is extended by `ClientRequest`, would literally do what it says in the docs if data was provided. It would call `this.write(data, encoding)`. This meant that nock could wrap only the `write` method when recording and gather all the chunks even if the last chunk was sent to `end`. But, the above changed that to call an internal function dual used by `end` and `write`. * fix: request.end accepted arguments. Fixes #1549 The method now correctly accepts all the permutations allowed. request.end(data, encoding, callback) request.end(data, callback) request.end(data, encoding) request.end(data) request.end(callback) request.end() And a few tests were added to ensure all cases are explicitly covered.
gr2m
pushed a commit
that referenced
this pull request
Sep 4, 2019
* Fix typo about wrapping request.end. I'm making this change it's own commit so I have a place to comment the findings. Digging through Node git history, I found the change that created the breaking change in nock (ref nock PR 929). nodejs/node@a10bdb5#diff-286202fdbdd74ede6f5f5334b6176b5cL779 Before Node v8, `OutgoingMessage`, which is extended by `ClientRequest`, would literally do what it says in the docs if data was provided. It would call `this.write(data, encoding)`. This meant that nock could wrap only the `write` method when recording and gather all the chunks even if the last chunk was sent to `end`. But, the above changed that to call an internal function dual used by `end` and `write`. * fix: request.end accepted arguments. Fixes #1549 The method now correctly accepts all the permutations allowed. request.end(data, encoding, callback) request.end(data, callback) request.end(data, encoding) request.end(data) request.end(callback) request.end() And a few tests were added to ensure all cases are explicitly covered.
gr2m
pushed a commit
that referenced
this pull request
Sep 5, 2019
* Fix typo about wrapping request.end. I'm making this change it's own commit so I have a place to comment the findings. Digging through Node git history, I found the change that created the breaking change in nock (ref nock PR 929). nodejs/node@a10bdb5#diff-286202fdbdd74ede6f5f5334b6176b5cL779 Before Node v8, `OutgoingMessage`, which is extended by `ClientRequest`, would literally do what it says in the docs if data was provided. It would call `this.write(data, encoding)`. This meant that nock could wrap only the `write` method when recording and gather all the chunks even if the last chunk was sent to `end`. But, the above changed that to call an internal function dual used by `end` and `write`. * fix: request.end accepted arguments. Fixes #1549 The method now correctly accepts all the permutations allowed. request.end(data, encoding, callback) request.end(data, callback) request.end(data, encoding) request.end(data) request.end(callback) request.end() And a few tests were added to ensure all cases are explicitly covered.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #1549
The method now correctly accepts all the permutations allowed.
And a few tests were added to ensure all cases are explicitly covered.