Skip to content

support for Node.js 12 - #64

Merged
indexzero merged 2 commits into
flatiron:masterfrom
Xmader:patch-1
Jan 18, 2024
Merged

indexzero merged 2 commits into
flatiron:masterfrom
Xmader:patch-1

Conversation

@Xmader

@Xmader Xmader commented Jun 4, 2019

Copy link
Copy Markdown
Contributor
[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

Comment thread lib/response-stream.js
this._headers = this.response._headers = this.response._headers || {};

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

@lamweili lamweili Feb 15, 2020

Copy link
Copy Markdown

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 || {};  
}

Copy link
Copy Markdown

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

kefir01 commented Feb 20, 2020

Copy link
Copy Markdown

When can we expect this patch to be published?

@Saeger Saeger left a comment

Copy link
Copy Markdown

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
Copy Markdown

@Xmader Mind making the suggested changes?

@mirabilos

Copy link
Copy Markdown

ping?

@lamweili

lamweili commented Jan 14, 2024

Copy link
Copy Markdown

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
Copy Markdown
Contributor

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