-
Notifications
You must be signed in to change notification settings - Fork 195
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
fix [DEP0066] warning for node > 10.x #367
base: master
Are you sure you want to change the base?
Conversation
Support it. Great. Well done. |
Yeah but it needs improvement to support node < 7.x |
I added a ternary variable assignment to check either the new |
…ded on tests to support node 12.x
Any news on this issue? |
Nope. No one seems to be available to help on tests with node > 10 that I don’t have time to go deeper in to understand. Moreover they have to allow pull request also with no one responding... |
@daviddias Can you please take a look and invest 5 mins of your time here? It's an easy fix and people really need it to move forward. Thanks a lot, it will be highly appreciated! |
Thanks for the nudge @franz-josef-kaiser. I'm not actively maintaining this package for a very long time, having passed that baton to @jacobheun. Let me ping him so that he can review it better :) |
Hi @FlorentinTh, thanks for the fix! I ran test/client-test.js separately with
on Node v14.17.4 (LTS) and noticed that POST requests and their "after each" hooks are failing in SSL mode. If I move the it('should send GET request' test after it('should send POST request', the tests are passing. So GET after POST is working, but the other way it isn't. In general the order in which the tests are executed should not matter, probably some resource is not cleaned up properly. I hope the maintainers can take a look at this. |
Nice work @attila-boros. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@indutny Similar to spdy-http2/http-deceiver#7 (comment), can you review this as well? /cc @alexander-akait |
@jacobheun @daviddias Any news on this one? |
Wanted to bump this as well as I'm getting warnings on my http2/express server using the Also, does anyone on this thread know the potential effects of this deprecation? Can it affect/drop headers or is more along the lines of using an existing (but unsupported) API? Thanks! |
@indutny would you take a few minutes to review this PR or grant some of us the permission to accept PR (and publish on npm)? Otherwise I'm willing to take the load to fork this package and get this PR published to npm. Appreciate the contribution from everyone 🙏 |
I've forked this package into I may try to look into other issues and pull request upon suggestions. Appreciate the contributions from everyone. |
Just a simple change from :
this._headers
tothis.getHeaders()
in line 18 ofresponse.js
. It should be enough to remove the warning[DEP0066] DeprecationWarning: OutgoingMessage.prototype._headers is deprecated
for users of Node > 10.x