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

avoid loading spdy if not used and prevent deprecation warning #1943

Merged
merged 2 commits into from
Apr 12, 2023

Conversation

massimocandela
Copy link
Contributor

@massimocandela massimocandela commented Feb 13, 2023

First of all, thanks for the great Restify!!
I've been using it for years in several projects (including this).

Pre-Submission Checklist

  • [x ] Opened an issue discussing these changes before opening the PR
  • [x ] Ran the linter and tests via make prepush
  • [x ] Included comprehensive and convincing tests for changes (same coverage)

Issues

Closes:

Support for Node14 will end in 2 months, we need to move to Node 16+.
However, restify imports spdy, which raises

DeprecationWarning: Access to process.binding('http_parser') is deprecated.

The good news is that most of the time spdy is not used since you need to pass options.spdy.

Changes

We can simply avoid deprecation warnings (and also avoid to run unused code) by importing spdy on request (as currently done for http2). This could be both a permanent or temporary solution waiting for spdy to be patched.

@acommodari
Copy link

Would love for this to get merged 🙏 Getting rid of this deprecation warning would be a godsend

@mmarchini mmarchini merged commit 89e7ac8 into restify:master Apr 12, 2023
@NormandoHall
Copy link

So, when you release it?

@jalopez
Copy link

jalopez commented Jul 19, 2023

Is there any chance this PR will be included in the next release?

@tokidoki11
Copy link

I wonder if this pr is in any of releases so far?

@NormandoHall
Copy link

Restify is dead. Migrate to express

@XVincentX
Copy link

I was having the same problem and fixed it by using patch-package with the following content on Restify 1.11:

diff --git a/node_modules/restify/lib/server.js b/node_modules/restify/lib/server.js
index a586532..412e850 100644
--- a/node_modules/restify/lib/server.js
+++ b/node_modules/restify/lib/server.js
@@ -11,7 +11,6 @@ var _ = require('lodash');
 var assert = require('assert-plus');
 var errors = require('restify-errors');
 var mime = require('mime');
-var spdy = require('spdy');
 var vasync = require('vasync');
 
 var Chain = require('./chain');
@@ -187,6 +186,7 @@ function Server(options) {
     ];
 
     if (options.spdy) {
+        var spdy = require('spdy');
         this.spdy = true;
         this.server = spdy.createServer(options.spdy);
     } else if (options.http2) {

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.

7 participants