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
Plan of process.binding() deprecation #1482
Comments
|
Note that there are multiple errors we can throw as the end-goal
Yes I guess that comes back to 3 (the ordering question)
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 |
|
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 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! |
This issue is not new but motivated by nodejs/node#50687, which proposed to
For background in 2021 the plan suggested in nodejs/node#37485 (review) and followed since nodejs/node#37819 was:
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()
: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.
a) Individually, based on how problematic they are/how difficult it is to provide alternatives
b) Together at once
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.
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.
The text was updated successfully, but these errors were encountered: