node: deprecate public access to `process.binding` by vkurchatkin · Pull Request #2768 · nodejs/node · GitHub

This PR introduces a new way to access bindings: require('binding/module_name') and deprecates process.binding. This is a logical continuation of internal modules feature. Bindings are also accessible only in builtin modules or with --expose_internals flag.

R=@nodejs/collaborators

Can you change vars to consts?

Perhaps leave the process.bindings in this test so that the deprecated functionality is still tested before it is removed?

good point

+1. Can the reason that this is semver-major be changed to keep it semver-minor?

No, this should be semver-major. Landing this in 4.0.0 will not be good imo.

We need a docs deprecation first if there isn't one already. That can land in 4.0.0, maybe.

cc @nodejs/tsc this is a quite significantly large change.

be very careful, npm deps have usage of process.binding:

npm $ grep -r process.binding . | grep -v test
./lib/adduser.js:  crypto = process.binding('crypto') && require('crypto')
./node_modules/cmd-shim/node_modules/graceful-fs/fs.js:var src = pre + process.binding('natives').fs + post
./node_modules/core-util-is/float.patch:-  if (isUndefined(uv)) uv = process.binding('uv');
./node_modules/debug/node.js:  var tty_wrap = process.binding('tty_wrap');
./node_modules/fs-vacuum/node_modules/graceful-fs/fs.js:var src = pre + process.binding('natives').fs + post
./node_modules/fs-write-stream-atomic/node_modules/graceful-fs/fs.js:var src = pre + process.binding('natives').fs + post
./node_modules/fstream/node_modules/graceful-fs/fs.js:var src = pre + process.binding('natives').fs + post
./node_modules/node-gyp/node_modules/graceful-fs/fs.js:var src = pre + process.binding('natives').fs + post
./node_modules/npm-registry-client/node_modules/graceful-fs/fs.js:var src = pre + process.binding('natives').fs + post
./node_modules/npm-registry-couchapp/node_modules/couchapp/node_modules/connect/node_modules/debug/node.js:  var tty_wrap = process.binding('tty_wrap');
./node_modules/npm-registry-couchapp/node_modules/couchapp/node_modules/nano/node_modules/debug/node.js:  var tty_wrap = process.binding('tty_wrap');
./node_modules/read-installed/node_modules/graceful-fs/fs.js:var src = pre + process.binding('natives').fs + post
./node_modules/read-package-json/node_modules/graceful-fs/fs.js:var src = pre + process.binding('natives').fs + post
./node_modules/readdir-scoped-modules/node_modules/graceful-fs/fs.js:var src = pre + process.binding('natives').fs + post
./node_modules/sha/index.js:  process.binding('crypto')
./node_modules/sha/node_modules/graceful-fs/fs.js:var src = pre + process.binding('natives').fs + post
./node_modules/tap/node_modules/runforcover/node_modules/bunker/node_modules/burrito/example/web/bs.js:if (!process.binding) process.binding = function (name) {
./node_modules/write-file-atomic/node_modules/graceful-fs/fs.js:var src = pre + process.binding('natives').fs + post

@bmeck There are a lot more.

We need a docs deprecation first if there isn't one already.

process.binding() is undocumented, always has been. I agree this should be semver-major but I think that's as far as it goes.

+1 for deprecating, because it was never actually supported: incompatible changes on the c++ side can be introduced in minor versions given that the js side compensates that. That results in problems if whoever is using those bindings directly.

Partial usage (approx 1/3):

a/acs-1.0.24.tgz/lib/cli.js:62:var natives = process.binding('natives');
b/builtin-modules-1.0.1.tgz/index.js:8:module.exports = Object.keys(process.binding('natives')).filter(function (el) {
c/crossbow-0.4.5.tgz/lib/core/util.js:741:  if (isUndefined(uv)) uv = process.binding('uv');
c/connect-yii-0.0.11.tgz/node-php.js:7:var HTTPParser = process.binding("http_parser").HTTPParser;
c/cli-0.7.0.tgz/cli.js:62:var natives = process.binding('natives');
c/choir-0.0.2.tgz/lib/magic/run_magic.js:11:var Script = process.binding('evals').NodeScript;
c/cattery-command-server-0.1.7.tgz/lib/node/master.js:4:var TCP = process.binding('tcp_wrap').TCP;
c/child_process2-2.0.1.tgz/child_process.js:11:const Process = process.binding('process_wrap').Process;
c/child_process2-2.0.1.tgz/child_process.js:12:const WriteWrap = process.binding('stream_wrap').WriteWrap;
c/child_process2-2.0.1.tgz/child_process.js:13:const uv = process.binding('uv');
c/child_process2-2.0.1.tgz/child_process.js:15:var spawn_sync; // Lazy-loaded process.binding('spawn_sync')
c/child_process2-2.0.1.tgz/child_process.js:16:var constants;  // Lazy-loaded process.binding('constants')
c/child_process2-2.0.1.tgz/child_process.js:33:  return process.binding('pipe_wrap').Pipe;
c/child_process2-2.0.1.tgz/child_process.js:37:  return process.binding('tty_wrap').TTY;
c/child_process2-2.0.1.tgz/child_process.js:41:  return process.binding('tcp_wrap').TCP;
c/child_process2-2.0.1.tgz/child_process.js:45:  return process.binding('udp_wrap').UDP;
c/child_process2-2.0.1.tgz/child_process.js:420:      } else if (handle instanceof process.binding('tcp_wrap').TCP ||
c/child_process2-2.0.1.tgz/child_process.js:421:                 handle instanceof process.binding('pipe_wrap').Pipe) {
c/child_process2-2.0.1.tgz/child_process.js:425:      } else if (handle instanceof process.binding('udp_wrap').UDP) {
c/child_process2-2.0.1.tgz/child_process.js:993:  process.binding('tcp_wrap');
c/child_process2-2.0.1.tgz/child_process.js:994:  process.binding('pipe_wrap');
c/child_process2-2.0.1.tgz/child_process.js:1181:    constants = process.binding('constants');
c/child_process2-2.0.1.tgz/child_process.js:1234:    constants = process.binding('constants');
c/child_process2-2.0.1.tgz/child_process.js:1284:    spawn_sync = process.binding('spawn_sync');
d/dprof-0.9.0.tgz/async_wrap.js:3:var asyncWrap = process.binding('async_wrap');
d/debug-2.2.0.tgz/node.js:146:  var tty_wrap = process.binding('tty_wrap');
d/dune-0.2.1.tgz/lib/dune2.js:30:var NativeModules = process.binding('natives');
e/ethereumjs-lib-0.3.1.tgz/benchmarks/index.js:4:var binding = process.binding('contextify');
f/fh-fhc-1.1.0-39.tgz/lib/cmd/fhc/login.js:18:  crypto = process.binding("crypto") && require("crypto");
f/fh-fhc-1.1.0-39.tgz/lib/utils/sys.js:2:module.exports = require(!!process.binding("natives").util ? "util" : "sys")
f/fh-fhc-1.1.0-39.tgz/lib/utils/fhlogin.js:10:  crypto = process.binding("crypto") && require("crypto");
f/freedom-social-xmpp-0.3.15.tgz/lib/net.js:35:  var Pipe = process.binding('pipe_wrap').Pipe;
f/freedom-social-xmpp-0.3.15.tgz/lib/net.js:47:  var tty = process.binding('tty_wrap');
f/freedom-social-xmpp-0.3.15.tgz/lib/net.js:1144:  var TCP = process.binding('tcp_wrap').TCP;
g/gobbler-0.0.4.tgz/src/parent.js:324:    if (!(this.potluckHandle instanceof process.binding('tcp_wrap').TCP)) {
g/graceful-fs-3.0.7.tgz/fs.js:8:var src = pre + process.binding('natives').fs + post
h/haribo-0.0.1.tgz/lib/util.js:743:  if (isUndefined(uv)) uv = process.binding('uv');
h/http_trace-0.5.0.tgz/http_session.js:1:var HTTPParser = process.binding("http_parser").HTTPParser; // magic alert
h/html5-1.0.5.tgz/example.js:6:            Script = process.binding('evals').Script,
h/http-parser-js-0.3.0.tgz/tests/bench-realworld.js:2:  process.binding("http_parser").HTTPParser = require('../http-parser.js').HTTPParser;
h/http-parser-js-0.3.0.tgz/tests/common.js:22:process.binding('http_parser').HTTPParser = require('../http-parser.js').HTTPParser;
h/http-parser-js-0.3.0.tgz/tests/common.js:309:  var TCP = process.binding('tcp_wrap').TCP;
h/http-parser-js-0.3.0.tgz/tests/bench.js:1://var HTTPParser = process.binding('http_parser').HTTPParser
h/http-parser-js-0.3.0.tgz/tests/simple/test-http-parser.js:25:var HTTPParser = process.binding('http_parser').HTTPParser;
h/http-parser-js-0.3.0.tgz/tests/simple/test-http-parser-bad-ref.js:8:var HTTPParser = process.binding('http_parser').HTTPParser;
i/intercept-require-0.6.1.tgz/lib/index.js:72:  return process.binding("natives").hasOwnProperty(moduleId);
k/karma-assert-1.0.0.tgz/assert.js:816:  if (isUndefined(uv)) uv = process.binding('uv');
l/level-vinyl-0.5.1.tgz/test/src.js:249:  var constants = process.binding('constants')
l/lister-1.0.0.tgz/npm/lib/adduser.js:11:  crypto = process.binding("crypto") && require("crypto")
l/loadmeter-0.0.2.tgz/pakmanaged.js:14450:      var tty_wrap = process.binding('tty_wrap');
m/mock-fs-2.7.0.tgz/node/fs-0.10.28.js:31:var binding = process.binding('fs');
m/mock-fs-2.7.0.tgz/node/fs-0.10.28.js:32:var constants = process.binding('constants');
m/mock-fs-2.7.0.tgz/node/fs-0.10.28.js:1030:  var FSEvent = process.binding('fs_event_wrap').FSEvent;
m/mock-fs-2.7.0.tgz/node/fs-0.11.13.js:31:var binding = process.binding('fs');
m/mock-fs-2.7.0.tgz/node/fs-0.11.13.js:32:var constants = process.binding('constants');
m/mock-fs-2.7.0.tgz/node/fs-0.11.13.js:1044:  var FSEvent = process.binding('fs_event_wrap').FSEvent;
m/mock-fs-2.7.0.tgz/node/fs-0.8.26.js:31:var binding = process.binding('fs');
m/mock-fs-2.7.0.tgz/node/fs-0.8.26.js:32:var constants = process.binding('constants');
m/mock-fs-2.7.0.tgz/node/fs-0.8.26.js:817:  var FSEvent = process.binding('fs_event_wrap').FSEvent;
m/mock-fs-2.7.0.tgz/node/fs-0.12.0.js:31:var binding = process.binding('fs');
m/mock-fs-2.7.0.tgz/node/fs-0.12.0.js:32:var constants = process.binding('constants');
m/mock-fs-2.7.0.tgz/node/fs-0.12.0.js:1154:  var FSEvent = process.binding('fs_event_wrap').FSEvent;
m/mock-fs-2.7.0.tgz/node/fs-1.1.0.js:9:var binding = process.binding('fs');
m/mock-fs-2.7.0.tgz/node/fs-1.1.0.js:10:const constants = process.binding('constants');
m/mock-fs-2.7.0.tgz/node/fs-1.1.0.js:1132:  var FSEvent = process.binding('fs_event_wrap').FSEvent;
m/mock-fs-2.7.0.tgz/node/fs-0.9.12.js:31:var binding = process.binding('fs');
m/mock-fs-2.7.0.tgz/node/fs-0.9.12.js:32:var constants = process.binding('constants');
m/mock-fs-2.7.0.tgz/node/fs-0.9.12.js:1016:  var FSEvent = process.binding('fs_event_wrap').FSEvent;
m/mock-fs-2.7.0.tgz/node/fs-2.0.0.js:9:var binding = process.binding('fs');
m/mock-fs-2.7.0.tgz/node/fs-2.0.0.js:10:const constants = process.binding('constants');
m/mock-fs-2.7.0.tgz/node/fs-2.0.0.js:1132:  var FSEvent = process.binding('fs_event_wrap').FSEvent;
m/mock-fs-2.7.0.tgz/lib/file.js:9:var constants = process.binding('constants');
m/mock-fs-2.7.0.tgz/lib/binding.js:11:var constants = process.binding('constants');
m/mock-fs-2.7.0.tgz/lib/symlink.js:8:var constants = process.binding('constants');
m/mock-fs-2.7.0.tgz/lib/descriptor.js:3:var constants = process.binding('constants');
m/mock-fs-2.7.0.tgz/lib/directory.js:8:var constants = process.binding('constants');
m/mock-fs-2.7.0.tgz/test/lib/binding.spec.js:15:var constants = process.binding('constants');
m/mock-fs-2.7.0.tgz/test/helper.js:17:var constants = process.binding('constants');
m/mini-vm-0.0.2.tgz/index.js:1:var binding = process.binding('contextify');
m/mitm-1.1.0.tgz/lib/internal_socket.js:5:if (!NODE_0_10) var Uv = process.binding("uv")
m/micropromise-0.4.10.tgz/promise-bench/benchmark/lib/timers-ctx.js:22:var Timer = process.binding('timer_wrap').Timer;
m/minode-0.1.8.tgz/lib/http.js:5:var HTTPParser = process.binding("http_parser").HTTPParser;
m/minode-0.1.8.tgz/lib/dns.js:1:var cares = process.binding('cares_wrap');
m/minode-0.1.8.tgz/lib/minsock.js:4:var TCP = process.binding("tcp_wrap").TCP;
m/minode-0.1.8.tgz/lib/minsock.js:5:var Pipe = process.binding("pipe_wrap").Pipe;
m/minode-0.1.8.tgz/lib/minsock.js:52:      crypto = process.binding("crypto");
m/minode-0.1.8.tgz/lib/forkit.js:5:var TCP = process.binding("tcp_wrap").TCP;
m/minode-0.1.8.tgz/lib/forkit.js:6:var Pipe = process.binding("pipe_wrap").Pipe;
m/minode-0.1.8.tgz/lib/forkit.js:7:var Process = process.binding("process_wrap").Process;
m/minode-0.1.8.tgz/lib/forkit.js:8:var constants = process.binding("constants");
m/minode-0.1.8.tgz/lib/utils.js:4:var fs = process.binding("fs");
m/minode-0.1.8.tgz/lib/utils.js:505:  var crypto = process.binding("crypto");
m/minode-0.1.8.tgz/lib/filecache.js:2:var fs = process.binding("fs");
m/minode-0.1.8.tgz/lib/filecache.js:3:var constants = process.binding("constants");
m/minode-0.1.8.tgz/lib/filecache.js:4:var zlib = process.binding("zlib");
m/minode-0.1.8.tgz/lib/memcached.js:2:var cares = process.binding("cares_wrap");
m/mdns-2.2.8.tgz/lib/io_watcher.js:3:    dns_sd.SocketWatcher : process.binding('io_watcher').IOWatcher;
m/mdns-2.2.8.tgz/lib/resolver_sequence_tasks.js:105:  var cares = process.binding('cares_wrap');
m/mdns-2.2.8.tgz/lib/resolver_sequence_tasks.js:141:  _getaddrinfo = process.binding('net').getaddrinfo;
m/mitm-papandreou-1.0.3-patch1.tgz/lib/internal_socket.js:5:if (!NODE_0_10) var Uv = process.binding("uv")
m/meteor-deploy-ssh-1.0.10.tgz/lib/ssh2-master/lib/SFTP/Stats.js:1:var constants = process.binding('constants');
n/nad-bindings-0.3.0.tgz/index.js:23:      : process.binding(name.replace(/\.node$/,''));
n/node-yoctopuce-0.0.5.tgz/lib/node_yoctopuce.js:30:var HTTPParser = process.binding('http_parser').HTTPParser;
n/node-yoctopuce-0.0.5.tgz/examples/yapi/httpRequest.js:30:var HTTPParser = process.binding('http_parser').HTTPParser;
n/npm-2.11.0.tgz/lib/adduser.js:11:  crypto = process.binding("crypto") && require("crypto")
n/npm-registry-couchapp-2.6.9.tgz/test/vdu-auth.js:15:  process.binding('natives')[m] = mod[m]
n/npm-registry-couchapp-2.6.9.tgz/test/pkg-update-copy-fields.js:5:  process.binding('natives')[m] = mod[m]
n/npm-registry-couchapp-2.6.9.tgz/test/pkg-update-metadata.js:5:  process.binding('natives')[m] = mod[m]
n/npm-registry-couchapp-2.6.9.tgz/test/pkg-update-star-nopt.js:5:  process.binding('natives')[m] = mod[m]
n/npm-registry-couchapp-2.6.9.tgz/test/pkg-update-yui-error.js:5:  process.binding('natives')[m] = mod[m]
n/npm-registry-couchapp-2.6.9.tgz/test/pkg-update-hoodie.js:5:  process.binding('natives')[m] = mod[m]
n/npm-registry-couchapp-2.6.9.tgz/test/pkg-update-deprecate-msg.js:5:  process.binding('natives')[m] = mod[m]
n/npm-registry-couchapp-2.6.9.tgz/test/vdu.js:15:  process.binding('natives')[m] = mod[m]
n/neopixels-0.0.5.tgz/lib/neopixels.js:1:var hw = process.binding('hw');
n/nnpm-0.0.0.tgz/lib/adduser.js:11:  crypto = process.binding("crypto") && require("crypto")
n/nolimitid-myprocess-0.0.1.tgz/impostor_child_process.js:26:var Process = process.binding('process_wrap').Process;
n/nolimitid-myprocess-0.0.1.tgz/impostor_child_process.js:29:var constants; // if (!constants) constants = process.binding('constants');
n/nolimitid-myprocess-0.0.1.tgz/impostor_child_process.js:45:  return process.binding('pipe_wrap').Pipe;
n/nolimitid-myprocess-0.0.1.tgz/impostor_child_process.js:49:  return process.binding('tty_wrap').TTY;
n/nolimitid-myprocess-0.0.1.tgz/impostor_child_process.js:53:  return process.binding('tcp_wrap').TCP;
n/nolimitid-myprocess-0.0.1.tgz/impostor_child_process.js:57:  return process.binding('udp_wrap').UDP;
n/nolimitid-myprocess-0.0.1.tgz/impostor_child_process.js:420:      } else if (handle instanceof process.binding('tcp_wrap').TCP ||
n/nolimitid-myprocess-0.0.1.tgz/impostor_child_process.js:421:                 handle instanceof process.binding('pipe_wrap').Pipe) {
n/nolimitid-myprocess-0.0.1.tgz/impostor_child_process.js:425:      } else if (handle instanceof process.binding('udp_wrap').UDP) {
n/nolimitid-myprocess-0.0.1.tgz/impostor_child_process.js:745:  process.binding('tcp_wrap');
n/nolimitid-myprocess-0.0.1.tgz/impostor_child_process.js:746:  process.binding('pipe_wrap');
n/nolimitid-myprocess-0.0.1.tgz/impostor_child_process.js:992:    constants = process.binding('constants');
n/nolimitid-myprocess-0.0.1.tgz/fake_net.js:27:var cares = process.binding('cares_wrap');
n/nolimitid-myprocess-0.0.1.tgz/fake_net.js:34:  var Pipe = process.binding('pipe_wrap').Pipe;
n/nolimitid-myprocess-0.0.1.tgz/fake_net.js:40:  var TCP = process.binding('tcp_wrap').TCP;
n/nolimitid-myprocess-0.0.1.tgz/fake_net.js:46:  var tty = process.binding('tty_wrap');
n/nolimitid-myprocess-0.0.1.tgz/fake_net.js:1100:  var TCP = process.binding('tcp_wrap').TCP;
n/npm-for-cnpm-2.6.0.tgz/lib/adduser.js:11:  crypto = process.binding("crypto") && require("crypto")
n/naver-npm-3.0.3.tgz/lib/adduser.js:11:  crypto = process.binding("crypto") && require("crypto")
o/omegapm-0.0.5.tgz/lib/adduser.js:11:  crypto = process.binding("crypto") && require("crypto")
o/ozero-1.0.10.tgz/ozero.js:593:            var natives             = process.binding('natives');
p/pty.js-0.2.7-1.tgz/lib/pty.js:401:  var tty = process.binding('tty_wrap');
p/promise-bench-0.0.2.tgz/benchmark/lib/timers-ctx.js:22:var Timer = process.binding('timer_wrap').Timer;
p/phantomas-1.10.2.tgz/lib/modules/path.js:406:  process.binding('fs').stat(path, function(err, stats) {
p/phantomas-1.10.2.tgz/lib/modules/path.js:414:    process.binding('fs').stat(path);
p/phantomjs-nodify-0.1.1.tgz/lib/modules/path.js:406:  process.binding('fs').stat(path, function(err, stats) {
p/phantomjs-nodify-0.1.1.tgz/lib/modules/path.js:414:    process.binding('fs').stat(path);
r/rest-ftp-proxy-0.1.0.tgz/src/endpoint.js:3:  var fs = process.binding('fs');
r/rest-ftp-proxy-0.1.0.tgz/src/endpoint.js:4:  var request = process.binding('request');
r/rest-ftp-proxy-0.1.0.tgz/src/endpoint.js:5:  var response = process.binding('response');
r/reworse-1.0.0.tgz/http.js:5:    process.binding("http_parser").HTTPParser = HttpParser.HTTPParser;
r/reworse-1.0.0.tgz/static.js:5:    // process.binding("http_parser").HTTPParser = HttpParser.HTTPParser;
r/reworse-1.0.0.tgz/client.js:5:    process.binding("http_parser").HTTPParser = HttpParser.HTTPParser;
r/radic-cli-0.0.6.tgz/lib/cli/index.js:50:    var natives = process.binding('natives');
r/restrict-0.0.6.tgz/tests/test-restict.js:107:         process.binding('process_wrap');
r/restrict-0.0.6.tgz/tests/test-restict.js:122:         process.binding('evals');
r/requirehit-browser-0.1.0.tgz/lib/process/post-bindings.js:19:  process.env = process.binding( 'env' );
r/requirehit-browser-0.1.0.tgz/lib/process/native-module.js:31:NativeModule._source = process.binding( 'natives' );
r/requirehit-browser-0.1.0.tgz/lib/natives/smalloc.js:26:var smalloc = process.binding('smalloc');
r/requirehit-browser-0.1.0.tgz/lib/natives/util.js:745:  if (isUndefined(uv)) uv = process.binding('uv');
r/requirehit-browser-0.1.0.tgz/lib/natives/buffer.js:26:var smalloc = process.binding( 'smalloc' );
s/ssh2-streams-0.0.13.tgz/lib/sftp.js:9:    constants = process.binding('constants'),
s/slush-atom-shell-dev-0.0.1.tgz/test/cache/v0.17.1/darwin-x64/atom-shell-v0.17.1-darwin-x64/Atom.app/Contents/Resources/atom/common/lib/init.js:14:      return process.binding("atom_" + process.type + "_" + name);
s/slush-atom-shell-dev-0.0.1.tgz/test/cache/v0.17.1/darwin-x64/atom-shell-v0.17.1-darwin-x64/Atom.app/Contents/Resources/atom/common/lib/init.js:18:        return process.binding("atom_common_" + name);
s/sourcegraph-0.10.0.tgz/file.js:4:var natives = process.binding('natives')
s/strong-runner-1.0.2.tgz/test/app1/index.js:38:  var signo = process.binding('constants')[signame];
s/strong-runner-1.0.2.tgz/test/app/index.js:38:  var signo = process.binding('constants')[signame];
s/scriptor-2.5.4.tgz/bin/scriptor.js:9:var constants = process.binding( 'constants' );
s/supreme-0.0.0.tgz/index.js:3:var c = process.binding('constants')
s/supreme-0.0.0.tgz/index.js:5:  , fs = process.binding('fs')
s/ssh-1.8.0.tgz/lib/stat.js:1://TODO: perhaps just use process.binding('constants') to do some of this
t/trace-1.2.0.tgz/tracing_polyfill.js:6:var asyncWrap = process.binding('async_wrap');
t/tabris-js-node-0.0.1.tgz/node_lib/util.js:743:  if (isUndefined(uv)) uv = process.binding('uv');
t/timers-browserify-full-0.0.1.tgz/test/node-test-timers-ordering.js:44:// var Timer = process.binding('timer_wrap').Timer;
t/trireme-support-1.0.0.tgz/index.js:32:  binding = process.binding('trireme-native-support');
t/tcp-bind-1.2.0.tgz/index.js:1:var TCP = process.binding('tcp_wrap').TCP;
v/vm2-0.2.2.tgz/lib/sandbox.js:15:NATIVE_MODULES = parent.process.binding('natives');
v/vm2-0.2.2.tgz/lib/fs.sb.js:31:var binding = process.binding('fs');
v/vm2-0.2.2.tgz/lib/fs.sb.js:32:var constants = process.binding('constants');
v/vm2-0.2.2.tgz/lib/fs.sb.js:1030:  var FSEvent = process.binding('fs_event_wrap').FSEvent;
w/wtfnode-0.2.0.tgz/index.js:73:Timer = process.binding('timer_wrap').Timer;
w/websocket-driver-0.5.4.tgz/lib/websocket/http_parser.js:3:var NodeHTTPParser = process.binding('http_parser').HTTPParser,
w/webpack-1.9.10.tgz/lib/node/NodeTargetPlugin.js:12:   new ExternalsPlugin("commonjs", Object.keys(process.binding("natives"))).apply(compiler);
w/webmake-0.3.39.tgz/lib/parser.js:19:  , nmSource     = process.binding('natives')
y/yapm-2.4.2.tgz/lib/adduser.js:11:  crypto = process.binding("crypto") && require("crypto")
y/yearn-0.4.1.tgz/lib/utils/yearn-utils.js:5:var native_modules = Object.keys( process.binding('natives') );

This is a bit outdated. I will post an updated list if needed.
It does not include nesting deps, each entry is unique and direct here.

/graceful-fs/fs.js

@bmeck please get people to update graceful-fs. It won't work soon without updating anyways.

I know the HTTP parser (access) was undocumented before, but there are modules that use it and having it publicly available in some way would be beneficial IMHO.

+1 to change process.bindings to require('bindings/foo')

but I think we should not remove process.bindings, use util.deprecate instead of removing process.bindings.

@thefourtheye I didn't really add new declarations

@mscdex I agree, but that is another issue, we should expose http parser via safe and stable API

If possible I'd like to land this behind a flag for v5.0.0 so we can urge folks to run with that flag enabled to see if their project breaks. If we get significant pushback on it, we could revert the change, otherwise we'd make it the default in v6.0.0.

-1 on just doing this in a semver-major, +1 for using util.deprecate first, it may be undocumented but it's so commonly used that we need to give warning or it'll just come back on us with people blaming us for breaking their stuff, "it wasn't documented" isn't a good enough response to that.

I agree with @rvagg; the npm team has mostly landed the changes necessary for this to not be a problem for npm, but I think npm is a bellwether here. Deprecating first is a pain for the Node team, but not deprecating it is a major pain for users. In particular, it's going to take a while for people to stop using versions of debug that use process.binding -- that package is everywhere.

To be clear, npm/npm@2b97a57 removes the only direct use of process.binding() in npm (which is also what's picked up by nnpm, naver-npm, omegapm, and yapm, and probably others, as forks of npm), and we're most of the way through the process of upgrading npm to use graceful-fs@4 everywhere as well. (See #2714 (comment) for details.)

Once that work is done, which should happen as of the next versions of npm@2 and npm@3, this change won't be a problem for new versions of npm, but will mean that users upgrading to whatever major version of Node turns this off will be required to upgrade, which is something that's only happened a few times before, and is likely to be disruptive.

Isn't there some sort of precedence we have for commonly used "internals"? Like one such underscored header on http?

Also, we have received valid bug reports from users tapping into the direct bindings. It's been helpful to get user feedback using those functions that I highly doubt would have happened if it were behind a flag.

-1 from me unless someone can point me to issues having it exposed has caused.

@trevnorris graceful-fs is a fine example

@vkurchatkin IIRC graceful-fs was doing worse than tapping into a binding.

To add, it's still easy to bypass a chunk of what's accessible via bindings by abusing things like .constructor. If we want to more throughly hide API then we should use a similar practice as SetupNextTick(). I don't think hiding bindings is a fundamentally correct solution to the problem it's attempting to address.

IIRC graceful-fs was doing worse than tapping into a binding.

process.binding('natives') exposes some.. erm, interesting stuff. Maybe we should just deprecate it? ('natives')

Updated list: https://gist.github.com/ChALkeR/40731eb9cabf7dc49d4f
Based on 100% of current versions of npm packages, but it could contain both false negatives and false positives. That one had a lot of false positives due to modules that were defining their own process.binding to thow an exception.

Updated list, using process.binding( string: https://gist.github.com/ChALkeR/2e308b9f888456bfa4b0.
1471 lines in 412 modules.
Out of that, process.binding('natives') — 228 lines in 115 modules.

LGTM once the merge conflicts get sorted out.

@trevnorris I want to bake the bindings into the snapshot (eventually) but that won't work as long as process.bindings() is still around and in use.

Apart from that, we had to revert an improvement in lib/fs.js a while ago because of graceful-fs, ffs. That alone is a good reason to deprecate and remove it.

@bnoordhuis I'm not sure if there was any resolution, but I'm still eager to pursue this. People abuse this API without even thinking or giving any warning at least. A fresh example: https://github.com/mafintosh/why-is-node-running.

@vkurchatkin This looks like a good change to me, but afaik it's blocked by #3307 at least, and perhaps some other valid use-cases.

If I recall correctly the last time the CTC discussed this it came down to the potential ecosystem breakage being too significant to do this any time soon. It may be worth looking at again. If we can come up with a good transition path then maybe it's something we can do in an upcoming major post v6

Here is the revised plan:

Introduce binding/ namespace for internal require. It can be safely used instead of process.binding in all new code and internal modules. (semver patch)
Migrate process.binding -> require. This can break packages that evaluate builtins, so we can refrain from doing it in fs for some time. It can also break packages that monkey-patch process.binding, but I guess they break all the time anyway. (technically, patch, bad can be delayed until minor or major release)
Deprecate process.binding (major)

Returning to this... That approach SGTM @vkurchatkin

-1 from me, it is sometimes the only way to write useful things (for example, in node-spdy)

Perhaps that just means we need to do a better job at exposing a better
public API?
On Apr 21, 2016 11:39 AM, "Fedor Indutny" notifications@github.com wrote:

-1 from me, it is sometimes the only way to write useful things (for
example, in node-spdy)

—
You are receiving this because you are on a team that was mentioned.
Reply to this email directly or view it on GitHub
#2768 (comment)

@jasnell I agree. However this task is much more complicated than what is suggested here, and will take quite a lot of time. Thus, -1 from me.

the only way to write useful things (for example, in node-spdy)

Can you explain, why? I seriously doubt that, but if it's the case, than we have a serious gap in API. It might be a shortcut for someone who know internals to make things faster, but the downside is that node-spdy and all dependents are inherently unstable, and people who use them are unaware of these facts.

this task is much more complicated than what is suggested here, and will take quite a lot of time

Any suggestions are welcome. Time is not the issue, but I believe it's important to move in this direction. Step 1 is pretty harmless, for example

node-spdy provides compatibility with core's http.Server by manually calling http parser callbacks, and doing lots of other tricks with the virtual sockets. The former requires access to the http_parser binding, since it exposes many constants that are otherwise unavailable, and also exposes method array.

@indutny I suppose you are talking about this: https://github.com/indutny/http-deceiver/blob/master/lib/deceiver.js

It's exactly the kind of thing I'm trying to prevent.

@vkurchatkin And I think @indutny's saying that's exactly what he needs. There's no reason to disallow access to everything in process.binding(). We have examples of places where it shouldn't be allowed, but giving access to tcp_wrap hardly seems a problem. I'd be more for it if we "properly" exposed such low level bindings as a public API. Not layered on with streams the way it is now.

And preventing the user from accessing these in many cases is futile. e.g.

net.createServer().listen(8081)._handle.constructor === process.binding('tcp_wrap').TCP

In these cases it especially doesn't make sense to remove access to the constructor via a simple API call.

Given the -1's and the lack of further discussion on this, closing.

@trevnorris

We have examples of places where it shouldn't be allowed, but giving access to tcp_wrap hardly seems a problem.

Perhaps we should stick to the proposal in #2768 (comment), but only the two first steps for now and on case-by-case basis? I.e. do this for some cases that shouldn't be allowed, if I understood things correctly, but allow (keep accessible via process.binding(…)) those that seem fine.

We could start with hiding those ones that have a clear replacement, e.g. process.binding('os').

@ChALkeR Sounds like a reasonable place to start.

Is this needed?

Only if process.binding is actually deprecated

No, where is require('binding/… used in this test? Note that it doesn't seem to call hasMultiLocalhost.

No idea, it was too long ago. Don't bother reviewing, I'm gonna redo this from scratch

As an idea: we can make a separate folder for tests that require expose_internals and set it implicitly.

We could start with hiding those ones that have a clear replacement, e.g. process.binding('os').

Also, constants should be hidden asap =).

Before 6.3.0, process.binding('constants') === require('constants'). In 6.3.0, since #6534, process.binding('constants') changed significantly.

There is nothing valueable in process.binding('constants') to the user that couldn't be used though require('constants') or through corresponding modules.

net.js:10
const cares = process.binding('cares_wrap');
^

Error: EFILE

Please Help, why am I getting this error and what is the way out ...

vkurchatkin added the semver-major PRs that contain breaking changes and should be released in the next major version. label Sep 9, 2015

brendanashworth reviewed Sep 10, 2015
View reviewed changes

ChALkeR mentioned this pull request Sep 10, 2015

process.binding is being deprecated. rlidwka/yapm#30

Open

evanlucas mentioned this pull request Sep 10, 2015

util: add guessHandleType method #2797

Closed

ChALkeR mentioned this pull request Sep 10, 2015

process.binding is being deprecated. npm/npm#9544

Closed

kenany mentioned this pull request Sep 11, 2015

Remove process.binding usage ForbesLindesay/sha#11

Merged

Fishrock123 closed this Sep 14, 2015

Fishrock123 reopened this Sep 14, 2015

AndreasMadsen mentioned this pull request Sep 23, 2015

Tracing WG Meeting 2015-09-25 nodejs/diagnostics#21

Closed

vkurchatkin mentioned this pull request Sep 26, 2015

src: log warning on process.binding('natives') #2741

Closed

AndreasMadsen mentioned this pull request Dec 1, 2015

AsyncWrap for LTS Argon nodejs/Release#59

Closed

Trott force-pushed the master branch from 1e896a6 to 082cc8d Compare December 27, 2015 02:00

jasnell added the stalled Issues and PRs that are stalled. label Apr 21, 2016

estliberitas force-pushed the master branch 2 times, most recently from 7da4fd4 to c7066fb Compare April 26, 2016 05:22

sindresorhus mentioned this pull request Apr 29, 2016

re-implement errname resolution sindresorhus/execa#31

Closed

jasnell closed this Aug 6, 2016

ChALkeR reviewed Aug 28, 2016
View reviewed changes

sholladay mentioned this pull request Sep 23, 2016

Create connection via file descriptor instead of port hapijs/hapi#3204

Closed

jnj16180340 mentioned this pull request Oct 26, 2016

errno constants aren't properly defined in node 6 raszi/node-tmp#95

Closed

mohsen1 mentioned this pull request Mar 25, 2018

Add deprecated process.bindings to node.js DefinitelyTyped/DefinitelyTyped#24521

Closed

16 tasks

netlify-team-account-1 mentioned this pull request Oct 19, 2021

[paperwork] take node: prefix as indicator for being a core module dependents/node-precinct#88

Merged

		@@ -1,4 +1,5 @@
		'use strict';
		// Flags: --expose_internals

node: deprecate public access to `process.binding` by vkurchatkin · Pull Request #2768 · nodejs/node · GitHub

Uh oh!

Uh oh!

node: deprecate public access to process.binding - #2768

node: deprecate public access to process.binding#2768vkurchatkin wants to merge 1 commit intonodejs:masternodejs/node:masterfrom vkurchatkin:deprecate-bindingvkurchatkin/node:deprecate-bindingCopy head branch name to clipboard

Conversation

vkurchatkin commented Sep 9, 2015

Uh oh!

thefourtheye commented Sep 9, 2015

Uh oh!

brendanashworth Sep 10, 2015

Choose a reason for hiding this comment

Uh oh!

vkurchatkin Sep 10, 2015

Choose a reason for hiding this comment

Uh oh!

brendanashworth commented Sep 10, 2015

Uh oh!

Fishrock123 commented Sep 10, 2015

Uh oh!

bmeck commented Sep 10, 2015

Uh oh!

ChALkeR commented Sep 10, 2015

Uh oh!

bnoordhuis commented Sep 10, 2015

Uh oh!

ChALkeR commented Sep 10, 2015

Uh oh!

Fishrock123 commented Sep 10, 2015

Uh oh!

mscdex commented Sep 10, 2015

Uh oh!

yosuke-furukawa commented Sep 10, 2015

Uh oh!

vkurchatkin commented Sep 10, 2015

Uh oh!

chrisdickinson commented Sep 10, 2015

Uh oh!

rvagg commented Sep 10, 2015

Uh oh!

othiym23 commented Sep 11, 2015

Uh oh!

trevnorris commented Sep 14, 2015

Uh oh!

vkurchatkin commented Sep 14, 2015

Uh oh!

trevnorris commented Sep 14, 2015

Uh oh!

Fishrock123 commented Sep 14, 2015

Uh oh!

ChALkeR commented Sep 26, 2015

Uh oh!

bnoordhuis commented Sep 26, 2015

Uh oh!

vkurchatkin commented Feb 16, 2016

Uh oh!

ChALkeR commented Feb 16, 2016

Uh oh!

jasnell commented Feb 17, 2016

Uh oh!

vkurchatkin commented Feb 18, 2016

Uh oh!

jasnell commented Apr 21, 2016

Uh oh!

indutny commented Apr 21, 2016

Uh oh!

jasnell commented Apr 21, 2016

Uh oh!

indutny commented Apr 21, 2016

Uh oh!

vkurchatkin commented Apr 21, 2016

Uh oh!

indutny commented Apr 21, 2016

Uh oh!

vkurchatkin commented Apr 21, 2016

Uh oh!

trevnorris commented Apr 26, 2016

Uh oh!

jasnell commented Aug 6, 2016

Uh oh!

node: deprecate public access to process.binding#2768
vkurchatkin wants to merge 1 commit into
nodejs:masterfrom
vkurchatkin:deprecate-binding

ChALkeR commented Aug 24, 2016 •
edited
Loading

ChALkeR commented Aug 28, 2016 •
edited
Loading