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

support for Node.js 12 #64

Merged
merged 2 commits into from
Jan 18, 2024
Merged

support for Node.js 12 #64

merged 2 commits into from
Jan 18, 2024

Conversation

Xmader
Copy link
Contributor

@Xmader Xmader commented Jun 4, 2019

[DEP0066] DeprecationWarning: OutgoingMessage.prototype._headers is deprecated
[DEP0066] DeprecationWarning: OutgoingMessage.prototype._headerNames is deprecated

https://nodejs.org/api/deprecations.html#deprecations_dep0066_outgoingmessage_prototype_headers_outgoingmessage_prototype_headernames

this._headers = this.response._headers = this.response._headers || {};

// Patch to node core
this.response._headerNames = this.response._headerNames || {};
Copy link

@lamweili lamweili Feb 15, 2020

Choose a reason for hiding this comment

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

Shouldn't this not be in the catch, but in another try-catch?

// Patch to node core
try {
  this.response._headerNames = this.response.getHeaderNames() || {};
} catch (err) {
  this.response._headerNames = this.response._headerNames || {};  
}

Choose a reason for hiding this comment

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

It has been commented in PR #77 that there is no need for this suggestion.
@darrendatbui has did some testing and the original committed code works.

I rest my case.

@kefir01
Copy link

kefir01 commented Feb 20, 2020

When can we expect this patch to be published?

Copy link

@Saeger Saeger left a comment

Choose a reason for hiding this comment

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

Include @peteriman suggestion

@lamweili
Copy link

@Xmader Mind making the suggested changes?

@mirabilos
Copy link

ping?

@lamweili
Copy link

lamweili commented Jan 14, 2024

It has been >2 years and @Xmader's hasn't made the suggested changes.
@darrendatbui has made the changes in PR #77 which superseded this PR.
Let's continue there and close this PR.

This fixes issue #68.

This will supersede PR #71 and PR #77; these 2 PRs can be closed.

Ping @indexzero @jcrugzz

@indexzero
Copy link
Member

Appreciate everyone trying to keep this code alive

@indexzero indexzero merged commit 76d7f3d into flatiron:master Jan 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants