Plan of process.binding() deprecation

[joyeecheung]

This issue is not new but motivated by nodejs/node#50687, which proposed to

1. Ship the runtime deprecation in Node.js v22 --deprecated-process-binding on by default.
2. --no-deprecated-process-binding becomes the default in Node.js v23
3. process.binding(...) is removed entirely in Node.js v24, with the CLI option becoming a no-op

For background in 2021 the plan suggested in nodejs/node#37485 (review) and followed since nodejs/node#37819 was:

1. Do comprehensive research into what exact features ecosystem modules get through process.binding() (this isn't actually going to be a huge API surface!)
2. Patch process.binding() so that it returns objects that cover those use cases, implemented in terms of public APIs
3. Optionally, runtime-deprecate process.binding() only for the cases that users aren't commonly encountering
4. Eventually, remove support for those cases specifically
5. Leave it at that, don't runtime-deprecate process.binding() as a whole and leave the shim in place forever

I am opening this issue trying to slice the issue into different questions instead of bundling all together into just two different plans, in the hope of figuring out a better compromise.

There are several questions to the deprecation of process.binding():

1. What's our end-goal when the deprecation reaches EOL?
   a) Delete process.binding completely so that when trying to invoke it, users get an error.
   b) A function that does no more than emitting a warning and returning empty objects. Users almost always get an error when they try to access an unsupported property, but if the code path is unused, e.g. only loading a process.binding() from the top-level but never actually hit the code path using anything from it, it would not fail.
   c) Leave a low-maintenance and harmless shim in place forever. Users get an error only when they try to use a specific unsupported property.
2. How should the list of bindings (currently 20) be deprecated?
   a) Individually, based on how problematic they are/how difficult it is to provide alternatives
   b) Together at once
3. Should the runtime deprecation start before or after alternatives/shims are completed
   a) We investigate the impact (e.g. through CITGM), develop and release alternatives on the way, monitor the changes in the impact, before deciding when it's ready to runtime-deprecate it (individually or all together).
   b) Runtime-deprecate it directly and mitigate the impact afterwards based on the bug reports/feature requests we receive.
4. How should the warnings be rolled out when we start to emit warnings (there are a few more combination possibilities with 2)
   a) Emit runtime warnings unconditionally
   c) Non-node_module warnings first, node_module warnings in the next cycle

I think we as @nodejs/tsc need to take a deeper look into these questions and make some decisions before finishing a plan of the deprecation.

[targos]

1. I would advocate for a, with b and c as possible intermediary steps.
2. You mean runtime-deprecated? It depends on the impact. If there is no popular module that uses any of them, we can deprecate all at once.
3. If we plan to develop alternatives, we should not runtime-deprecate before the alternative is available. But once we are confident the alternative is enough, we can runtime-deprecate.
4. Not sure non-node_module warnings would be useful in this case. Top-level apps probably don't use process.binding.

[joyeecheung]

I would advocate for a, with b and c as possible intermediary steps.

Note that there are multiple errors we can throw as the end-goal

1. TypeError: process.binding is not a function
2. TypeError: process.binding(...).foo is not a function
3. Error [ERR_UNSUPPORTED_OPERATION]: process.binding() is not supported anymore
4. [ERR_UNSUPPORTED_OPERATION]: process.binding(...).foo is not supported anymore

You mean runtime-deprecated? It depends on the impact.

Yes I guess that comes back to 3 (the ordering question)

Not sure non-node_module warnings would be useful in this case.

Libraries should still see the warning in their tests and get a reminder, if anyone is still watching for their test output (if they happen to run their tests with --throw-deprecation, it's even more noticable).

[jasnell]

1. I prefer (a) as the end result.
2. I prefer (b) ... together at once
3. Given my answers to 1 and 2, I prefer we land a runtime deprecation now (as per my PR)
4. Just a regular runtime deprecation per my PR.

[davidmurdoch]

I couldn't find a way of doing incremental (synchronous) Zlib inflate on an open file stream using the built in node.js APIs so I reached for the Zlib binding.

My issue is that I have a stream with a bunch of deflate blocks. I know the offset of the start of the deflate block I need to inflate, but I don't know where the end of the deflate block is. While I could read in the whole file and pass that to zlib.inflateSync, I'd much rather process it in chunks until I get to end end of the deflate stream.

My implementation is something like (to others: don't use this... it's certainly not robust for general purpose applications):

function inflate(
  zlib: Zlib, // an `init`ialized instance of `process.binding("zlib").Zlib`
  state: Uint32Array,
  reader: Reader, // helper class to read from a file stream
  outSize: number,
) {
  const chunkSize = Math.max(constants.Z_MIN_CHUNK, outSize);
  const output = Buffer.allocUnsafe(outSize);

  let totalInflated = 0;
  let outOffset = 0;

  while (totalInflated < outSize) {
    const chunk = reader.peek(chunkSize);
    reader.seek(chunk.byteLength);

    let inOffset = 0;
    let availInBefore = chunk.byteLength;
    let availOutBefore = outSize - outOffset;

    // continue running while there is still data to process
    while (availInBefore > 0 && availOutBefore > 0) {
      // `Z_BLOCK` will process the zlib header and then return. The next time
      // it runs it will "inflate" up to the end of the input data, or the end
      // of the deflate block (which ever comes first) then return. It will
      // *not* continue to the next block of compressed data.
      const flush = constants.Z_BLOCK;
      zlib.writeSync(
        flush,
        chunk,
        inOffset,
        availInBefore,
        output,
        outOffset,
        availOutBefore,
      );

      const [availOutAfter, availInAfter] = state;
      const inBytesRead = availInBefore - availInAfter;
      const outBytesWritten = availOutBefore - availOutAfter;

      inOffset += inBytesRead;
      outOffset += outBytesWritten;
      totalInflated += outBytesWritten;
      availInBefore = availInAfter;
      availOutBefore = availOutAfter;
    }
  }

  return output;
}

Maybe there was a way and I just didn't see it?

Anyway, before officially deprecating this api a replacement would be really appreciated!

[Jarred-Sumner]

@joyeecheung I wonder if V8 has an equivalent of masqueradesAsUndefined? When you do typeof openDatabase in Safari, it returns undefined and if you do !!openDatabase it will return false. But you can still call the function.

Something like that might be useful as an in-between step for changes like this

[mcollina]

@joyeecheung @jasnell given there was no progress on this front, can we get nodejs/node#50687 reopened and landed for v23?

[joyeecheung]

I don't think I can commit enough time to this to get consensus and spin a vote, so I won't block if someone wants to reland that, but if someone else wants to initiate a vote I would vote -1 to console my conscience. For example this is what you see in a yarn + swc-node session if the Node.js release start to emit runtime warnings for process.binding() unconditionally (the worst part IMO is the first one where the warning disturbs a prompt, and in the install session where the warning disturbs the progress bar)

$ yarn init
yarn init v1.22.22
question name (test-yarn): (node:5005) [DEP0111] DeprecationWarning: Access to process.binding('natives') is deprecated.
(Use `node --trace-deprecation ...` to show where the warning was created)

question version (1.0.0):
question description:
question entry point (index.js):
question repository url:
question author:
question license (MIT):
question private:
success Saved package.json
Done in 4.43s.

$ touch yarn.lock

$ yarn set version berry

(node:5026) [DEP0111] DeprecationWarning: Access to process.binding('natives') is deprecated.
(Use `node --trace-deprecation ...` to show where the warning was created)
(node:5037) [DEP0111] DeprecationWarning: Access to process.binding('util') is deprecated.
(Use `node --trace-deprecation ...` to show where the warning was created)
➤ YN0000: You don't seem to have Corepack enabled; we'll have to rely on yarnPath instead
➤ YN0000: Downloading https://repo.yarnpkg.com/4.3.1/packages/yarnpkg-cli/bin/yarn.js
➤ YN0000: Saving the new release in .yarn/releases/yarn-4.3.1.cjs
➤ YN0000: Done with warnings in 0s 71ms


$ yarn add @swc-node/register @swc/core typescript

(node:5058) [DEP0111] DeprecationWarning: Access to process.binding('util') is deprecated.
(Use `node --trace-deprecation ...` to show where the warning was created)
➤ YN0000: · Yarn 4.3.1
➤ YN0000: ┌ Resolution step
➤ YN0085: │ + @swc-node/register@npm:1.10.3, @swc/core@npm:1.6.13, and 40 more.
➤ YN0000: └ Completed in 0s 297ms
➤ YN0000: ┌ Post-resolution validation
➤ YN0086: │ Some peer dependencies are incorrectly met by dependencies; run yarn explain peer-requirements for details.
➤ YN0000: └ Completed
➤ YN0000: ┌ Fetch step
➤ YN0000: ⠼ =======================================-----------------------------------------
(node:5058) [DEP0111] DeprecationWarning: Access to process.binding('util') is deprecated.
(Use `node --trace-deprecation ...` to show where the warning was created)
(node:5058) [DEP0111] DeprecationWarning: Access to process.binding('util') is deprecated.
(Use `node --trace-deprecation ...` to show where the warning was created)
(node:5058) [DEP0111] DeprecationWarning: Access to process.binding('util') is deprecated.
(Use `node --trace-deprecation ...` to show where the warning was created)
(node:5058) [DEP0111] DeprecationWarning: Access to process.binding('util') is deprecated.
➤ YN0000: ⠼ =============================================-----------------------------------
(node:5058) [DEP0111] DeprecationWarning: Access to process.binding('util') is deprecated.
(Use `node --trace-deprecation ...` to show where the warning was created)
(node:5058) [DEP0111] DeprecationWarning: Access to process.binding('util') is deprecated.
(Use `node --trace-deprecation ...` to show where the warning was created)
(node:5058) [DEP0111] DeprecationWarning: Access to process.binding('util') is deprecated.
(Use `node --trace-deprecation ...` to show where the warning was created)
(node:5058) [DEP0111] DeprecationWarning: Access to process.binding('util') is deprecated.
(Use `node --trace-deprecation ...` to show where the warning was created)
(node:5058) [DEP0111] DeprecationWarning: Access to process.binding('util') is deprecated.
(Use `node --trace-deprecation ...` to show where the warning was created)
(node:5058) [DEP0111] DeprecationWarning: Access to process.binding('util') is deprecated.
(Use `node --trace-deprecation ...` to show where the warning was created)
(node:5058) [DEP0111] DeprecationWarning: Access to process.binding('util') is deprecated.
(Use `node --trace-deprecation ...` to show where the warning was created)
(node:5058) [DEP0111] DeprecationWarning: Access to process.binding('util') is deprecated.
(Use `node --trace-deprecation ...` to show where the warning was created)
(node:5058) [DEP0111] DeprecationWarning: Access to process.binding('util') is deprecated.
(Use `node --trace-deprecation ...` to show where the warning was created)
(node:5058) [DEP0111] DeprecationWarning: Access to process.binding('util') is deprecated.
➤ YN0000: ⠴ =========================================================-----------------------
(node:5058) [DEP0111] DeprecationWarning: Access to process.binding('util') is deprecated.
➤ YN0013: │ 24 packages were added to the project (+ 103.57 MiB).
➤ YN0000: └ Completed in 1s 429ms
➤ YN0000: ┌ Link step
➤ YN0000: │ ESM support for PnP uses the experimental loader API and is therefore experimental
➤ YN0007: │ @swc/core@npm:1.6.13 [201f2] must be built because it never has been before or the last one failed
➤ YN0000: └ Completed in 0s 361ms
➤ YN0000: · Done with warnings in 2s 108ms

$ echo 'console.log("Hello world")' > index.ts

$ yarn node --loader @swc-node/register/esm index.ts

(node:5115) [DEP0111] DeprecationWarning: Access to process.binding('util') is deprecated.
(Use `node --trace-deprecation ...` to show where the warning was created)
(node:5126) [DEP0111] DeprecationWarning: Access to process.binding('fs') is deprecated.
(Use `node --trace-deprecation ...` to show where the warning was created)
Hello world

And I know that at least the fs binding use case (monkey-patching) is already known for years and no one has been actively working on it to my knowledge, so I don't see the plan of "let's deprecate and get requests" working out well for our users, because there is no guarantee how long it will take to actually release a solution to the request, meanwhile users have to suffer through disruptions like this in their CLIs, especially the ones that prompt and spin progress bars, and that looks very similar to the Buffer constructor situation we had a few years ago.

[RafaelGSS]

@joyeecheung That's why I changed the warning to process.binding() instead of a warning to each process.binding('key') call (nodejs/node#48568 (comment)). Wouldn't it solve it?

[joyeecheung]

The disruption to prompts and progress bars would be the same as long as the warning is emitted on anyaprocess.binding() call, as the CLI could invoke it while it is providing these interactive experiences. Notice how in yarn init, the package name prompt is disrupted by the warning about process.binding('natives'), and in yarn add, the progress bar is disrupted by the warning about process.binding('util'). And the warning won't go away any time soon because there is no guarantee that someone will shepherd through a fix to their request, given that the entire surface of process.binding() is too big and there are cases like fs monkey patching that have never been addressed for years even though it's already known.

[joyeecheung]

This is what you'll see with nodejs/node#48568 in a colored console (I am not sure why I am doing this, research like this should be part of any PR proposing warnings to process.binding IMO)

[mcollina]

@joyeecheung apart from yarn, where you able to catch what other popular module was affected?

[joyeecheung]

I am pretty sure whoever proposing the breakage can easily try it out on other popular tools and report back. I don’t want to do it myself because

1. I am just doing it for my good conscience for trying not to ruin the UX for our users. My conscience is worth an easy investigation of a use case I know, but not really worth a significant bigger amount of time surveying the ecosystem. IMO it should be done by who’s proposing the breakage.
2. I think something would be wrong with the process if there is no obligation for whoever proposing a breakage known to be potentially high-impact to do any impact research or developing alternative APIs at all, and if no one else does the research or develops the alternative API, instead of finishing the work we just forget it and proceed breaking users. That already happened with Buffer constructor before. What’s worse with this case is that unlike the Buffer constructor which had alternative APIs, with process.binding() there are known popular use case(like fs monkey patching) with no alternative API to migrate to, no one is working on developing an alternative API, and we are intentionally leaving users living with the warning for an indefinite amount of time. Also, the Buffer constructor change was justified by security reasons, but this is rushed due to unwillingness to do impact research or develop migration paths, it’s not under any emergency AFAIK.

If others think that the breakage is acceptable without further investigation from whoever proposing the change, and we should just leave users with the warning without any commitment into developing a migration path, I rest my case. I can reconcile with my conscience with the work I committed so far.

[mcollina]

Thanks @joyeecheung, I concur.