Skip to content

Commit

Permalink
feat: deprecate Deno.{stdin,stdout,stderr}.rid (#22073)
Browse files Browse the repository at this point in the history
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 <biwanczuk@gmail.com>
  • Loading branch information
iuioiua and bartlomieju authored Jan 24, 2024
1 parent 547468e commit 300eeb3
Show file tree
Hide file tree
Showing 3 changed files with 77 additions and 27 deletions.
27 changes: 21 additions & 6 deletions cli/tsc/dts/lib.deno.ns.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<Uint8Array>;
Expand Down Expand Up @@ -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<Uint8Array>;
Expand All @@ -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<Uint8Array>;
Expand Down
64 changes: 46 additions & 18 deletions ext/io/12_io.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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;
}
Expand All @@ -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);
}
}

Expand All @@ -299,9 +322,14 @@ export {
readAllSync,
readSync,
SeekMode,
Stderr,
stderr,
STDERR_RID,
stdin,
STDIN_RID,
Stdout,
stdout,
STDOUT_RID,
write,
writeSync,
};
13 changes: 10 additions & 3 deletions ext/node/polyfills/_process/streams.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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,
Expand Down

0 comments on commit 300eeb3

Please sign in to comment.