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

Proposal: HTTP – Move bodyHead to data event #550

Closed
ThisIsMissEm opened this issue Jan 22, 2015 · 10 comments
Closed

Proposal: HTTP – Move bodyHead to data event #550

ThisIsMissEm opened this issue Jan 22, 2015 · 10 comments
Labels
feature request Issues that request new features to be added to Node.js. http Issues or PRs related to the http subsystem. semver-major PRs that contain breaking changes and should be released in the next major version. stalled Issues and PRs that are stalled.

Comments

@ThisIsMissEm
Copy link

So, ages and ages ago, for WebSockets support, the idea of an upgradeHead was added. This is for the data that directly trails the headers in a Upgrade or Connect request. Whilst the spec doesn't really say what this is, I propose making this as the first data event, rather than as a extra argument to the upgrade or connect events.

This seems like the better way to do this, although, I know it'd be a huge change in API from what we currently have, which is fairly popularly used at present.

I feel that we should've really implemented it this way round in the first place, but there were a whole bunch of reasons as to why this proved difficult at the time.

Thoughts?

@edef1c
Copy link
Contributor

edef1c commented Jan 22, 2015

a40133d basically tried to fix this, resulting in nodejs/node-v0.x-archive#5557

@vkurchatkin vkurchatkin added the http Issues or PRs related to the http subsystem. label Jan 23, 2015
@ThisIsMissEm
Copy link
Author

Aye, I was talking to nathan when I created this ticket, it would indeed be a breaking change, it would hurt, but it'd mean a more consistent API.

– Micheil

On 22 Jan 2015, at 05:37, Nathan Zadoks notifications@github.com wrote:

https://github.com/joyent/node/commits/a40133d10cdb911b27fe8d46d67a835b0103bbf1 basically tried to fix this, resulting in nodejs/node-v0.x-archive#5557


Reply to this email directly or view it on GitHub.

@edef1c
Copy link
Contributor

edef1c commented Jan 28, 2015

I'd definitely like to see this fixed — I think this legacy cruft has lived for long enough. I'd have vetoed the original node revert if I could.

@stephank
Copy link

stephank commented Feb 2, 2015

Relevant (but old) node.js issue/pr: nodejs/node-v0.x-archive#3036

@Fishrock123
Copy link
Contributor

Maybe also related to nodejs/node-v0.x-archive#4813

@stuartpb
Copy link

@miksago How does this relate to nodejs/node-v0.x-archive#4813? Does one override the necessity of the other?

@ThisIsMissEm
Copy link
Author

I think nodejs/node-v0.x-archive#4813 closes this nicely.

@Fishrock123
Copy link
Contributor

Actually it would be better if this were to close the joyent/node issue. :)

@brendanashworth brendanashworth added the feature request Issues that request new features to be added to Node.js. label Jun 25, 2015
@Trott Trott added the semver-major PRs that contain breaking changes and should be released in the next major version. label Mar 11, 2016
@jasnell
Copy link
Member

jasnell commented Mar 22, 2016

Is this still something we'd want to do?

@jasnell jasnell added the stalled Issues and PRs that are stalled. label Apr 9, 2016
@jasnell
Copy link
Member

jasnell commented Aug 5, 2016

Unfortunately, given the lack of any further discussion or action on this, I'm going to close. We can reopen and revisit later if necessary.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Issues that request new features to be added to Node.js. http Issues or PRs related to the http subsystem. semver-major PRs that contain breaking changes and should be released in the next major version. stalled Issues and PRs that are stalled.
Projects
None yet
Development

No branches or pull requests

9 participants