From 300eeb343efa00d9572d3befa47ca88fb51c7ac6 Mon Sep 17 00:00:00 2001 From: Asher Gomez Date: Thu, 25 Jan 2024 06:09:49 +1100 Subject: [PATCH] feat: deprecate `Deno.{stdin,stdout,stderr}.rid` (#22073) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit For removal in Deno v2. There are two issues: 1. Any script being run causes the output of `warnOnDeprecatedApi()` to be printed, even when none of the `rid` properties are called. 2. `.rid` of these classes is used in multiple tests. I'm not sure how to account for that. I thought of having `STDIN_RID`, and friends, constants, whose values can be shared between the tests and the classes themselves. Should we go with that or do something else? --------- Co-authored-by: Bartek Iwańczuk --- cli/tsc/dts/lib.deno.ns.d.ts | 27 ++++++++--- ext/io/12_io.js | 64 ++++++++++++++++++------- ext/node/polyfills/_process/streams.mjs | 13 +++-- 3 files changed, 77 insertions(+), 27 deletions(-) diff --git a/cli/tsc/dts/lib.deno.ns.d.ts b/cli/tsc/dts/lib.deno.ns.d.ts index 743152442e..1ac86db981 100644 --- a/cli/tsc/dts/lib.deno.ns.d.ts +++ b/cli/tsc/dts/lib.deno.ns.d.ts @@ -2727,8 +2727,13 @@ declare namespace Deno { * @category I/O */ export const stdin: Reader & ReaderSync & Closer & { - /** The resource ID assigned to `stdin`. This can be used with the discreet - * I/O functions in the `Deno` namespace. */ + /** + * The resource ID assigned to `stdin`. This can be used with the discreet + * I/O functions in the `Deno` namespace. + * + * @deprecated Use {@linkcode Deno.stdin} instance methods instead. + * {@linkcode Deno.stdin.rid} will be removed in Deno 2.0. + */ readonly rid: number; /** A readable stream interface to `stdin`. */ readonly readable: ReadableStream; @@ -2769,8 +2774,13 @@ declare namespace Deno { * @category I/O */ export const stdout: Writer & WriterSync & Closer & { - /** The resource ID assigned to `stdout`. This can be used with the discreet - * I/O functions in the `Deno` namespace. */ + /** + * The resource ID assigned to `stdout`. This can be used with the discreet + * I/O functions in the `Deno` namespace. + * + * @deprecated Use {@linkcode Deno.stdout} instance methods instead. + * {@linkcode Deno.stdout.rid} will be removed in Deno 2.0. + */ readonly rid: number; /** A writable stream interface to `stdout`. */ readonly writable: WritableStream; @@ -2797,8 +2807,13 @@ declare namespace Deno { * @category I/O */ export const stderr: Writer & WriterSync & Closer & { - /** The resource ID assigned to `stderr`. This can be used with the discreet - * I/O functions in the `Deno` namespace. */ + /** + * The resource ID assigned to `stderr`. This can be used with the discreet + * I/O functions in the `Deno` namespace. + * + * @deprecated Use {@linkcode Deno.stderr} instance methods instead. + * {@linkcode Deno.stderr.rid} will be removed in Deno 2.0. + */ readonly rid: number; /** A writable stream interface to `stderr`. */ readonly writable: WritableStream; diff --git a/ext/io/12_io.js b/ext/io/12_io.js index 2a0d589a75..acb54f6483 100644 --- a/ext/io/12_io.js +++ b/ext/io/12_io.js @@ -3,6 +3,7 @@ // Interfaces 100% copied from Go. // Documentation liberally lifted from them too. // Thank you! We love Go! <3 + import { core, internals, primordials } from "ext:core/mod.js"; const { op_stdin_set_raw, @@ -179,31 +180,41 @@ function concatBuffers(buffers) { return contents; } +const STDIN_RID = 0; +const STDOUT_RID = 1; +const STDERR_RID = 2; + class Stdin { + #rid = STDIN_RID; #readable; constructor() { } get rid() { - return 0; + internals.warnOnDeprecatedApi( + "Deno.stdin.rid", + new Error().stack, + "Use `Deno.stdin` instance methods instead.", + ); + return this.#rid; } read(p) { - return read(this.rid, p); + return read(this.#rid, p); } readSync(p) { - return readSync(this.rid, p); + return readSync(this.#rid, p); } close() { - core.tryClose(this.rid); + core.tryClose(this.#rid); } get readable() { if (this.#readable === undefined) { - this.#readable = readableStreamForRid(this.rid); + this.#readable = readableStreamForRid(this.#rid); } return this.#readable; } @@ -214,75 +225,87 @@ class Stdin { } isTerminal() { - return op_is_terminal(this.rid); + return op_is_terminal(this.#rid); } } class Stdout { + #rid = STDOUT_RID; #writable; constructor() { } get rid() { - return 1; + internals.warnOnDeprecatedApi( + "Deno.stdout.rid", + new Error().stack, + "Use `Deno.stdout` instance methods instead.", + ); + return this.#rid; } write(p) { - return write(this.rid, p); + return write(this.#rid, p); } writeSync(p) { - return writeSync(this.rid, p); + return writeSync(this.#rid, p); } close() { - core.close(this.rid); + core.close(this.#rid); } get writable() { if (this.#writable === undefined) { - this.#writable = writableStreamForRid(this.rid); + this.#writable = writableStreamForRid(this.#rid); } return this.#writable; } isTerminal() { - return op_is_terminal(this.rid); + return op_is_terminal(this.#rid); } } class Stderr { + #rid = STDERR_RID; #writable; constructor() { } get rid() { - return 2; + internals.warnOnDeprecatedApi( + "Deno.stderr.rid", + new Error().stack, + "Use `Deno.stderr` instance methods instead.", + ); + return this.#rid; } write(p) { - return write(this.rid, p); + return write(this.#rid, p); } writeSync(p) { - return writeSync(this.rid, p); + return writeSync(this.#rid, p); } close() { - core.close(this.rid); + core.close(this.#rid); } get writable() { if (this.#writable === undefined) { - this.#writable = writableStreamForRid(this.rid); + this.#writable = writableStreamForRid(this.#rid); } return this.#writable; } isTerminal() { - return op_is_terminal(this.rid); + return op_is_terminal(this.#rid); } } @@ -299,9 +322,14 @@ export { readAllSync, readSync, SeekMode, + Stderr, stderr, + STDERR_RID, stdin, + STDIN_RID, + Stdout, stdout, + STDOUT_RID, write, writeSync, }; diff --git a/ext/node/polyfills/_process/streams.mjs b/ext/node/polyfills/_process/streams.mjs index 166d099c83..09c53eb9a2 100644 --- a/ext/node/polyfills/_process/streams.mjs +++ b/ext/node/polyfills/_process/streams.mjs @@ -37,7 +37,14 @@ export function createWritableStdioStream(writer, name) { } }, }); - stream.fd = writer?.rid ?? -1; + let fd = -1; + + if (writer instanceof io.Stdout) { + fd = io.STDOUT_RID; + } else if (writer instanceof io.Stderr) { + fd = io.STDERR_RID; + } + stream.fd = fd; stream.destroySoon = stream.destroy; stream._isStdio = true; stream.once("close", () => writer?.close()); @@ -117,7 +124,7 @@ export function setReadStream(s) { // https://github.com/nodejs/node/blob/v18.12.1/lib/internal/bootstrap/switches/is_main_thread.js#L189 /** Create process.stdin */ export const initStdin = () => { - const fd = io.stdin?.rid; + const fd = io.stdin ? io.STDIN_RID : undefined; let stdin; const stdinType = _guessStdinType(fd); @@ -172,7 +179,7 @@ export const initStdin = () => { } stdin.on("close", () => io.stdin?.close()); - stdin.fd = io.stdin?.rid ?? -1; + stdin.fd = io.stdin ? io.STDIN_RID : -1; Object.defineProperty(stdin, "isTTY", { enumerable: true, configurable: true,