Skip to content
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

fix(console): deprecate Deno.customInspect #10035

Merged
merged 9 commits into from
Jun 25, 2021
32 changes: 18 additions & 14 deletions cli/dts/lib.deno.ns.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -426,7 +426,7 @@ declare namespace Deno {

/**
* @deprecated Use iter from https://deno.land/std/io/util.ts instead. Deno.iter will be removed in Deno 2.0.
*
*
* Turns a Reader, `r`, into an async iterator.
*
* ```ts
Expand Down Expand Up @@ -465,7 +465,7 @@ declare namespace Deno {

/**
* @deprecated Use iterSync from https://deno.land/std/io/util.ts instead. Deno.iterSync will be removed in Deno 2.0.
*
*
* Turns a ReaderSync, `r`, into an iterator.
*
* ```ts
Expand Down Expand Up @@ -847,7 +847,7 @@ declare namespace Deno {

/**
* @deprecated Use Buffer from https://deno.land/std/io/buffer.ts instead. Deno.Buffer will be removed in Deno 2.0.
*
*
* A variable-sized buffer of bytes with `read()` and `write()` methods.
*
* Deno.Buffer is almost always used with some I/O like files and sockets. It
Expand Down Expand Up @@ -930,7 +930,7 @@ declare namespace Deno {

/**
* @deprecated Use readAll from https://deno.land/std/io/util.ts instead. Deno.readAll will be removed in Deno 2.0.
*
*
* Read Reader `r` until EOF (`null`) and resolve to the content as
* Uint8Array`.
*
Expand All @@ -954,7 +954,7 @@ declare namespace Deno {

/**
* @deprecated Use readAllSync from https://deno.land/std/io/util.ts instead. Deno.readAllSync will be removed in Deno 2.0.
*
*
* Synchronously reads Reader `r` until EOF (`null`) and returns the content
* as `Uint8Array`.
*
Expand All @@ -978,7 +978,7 @@ declare namespace Deno {

/**
* @deprecated Use writeAll from https://deno.land/std/io/util.ts instead. Deno.readAll will be removed in Deno 2.0.
*
*
* Write all the content of the array buffer (`arr`) to the writer (`w`).
*
* ```ts
Expand All @@ -1003,7 +1003,7 @@ declare namespace Deno {

/**
* @deprecated Use writeAllSync from https://deno.land/std/io/util.ts instead. Deno.writeAllSync will be removed in Deno 2.0.
*
*
* Synchronously write all the content of the array buffer (`arr`) to the
* writer (`w`).
*
Expand Down Expand Up @@ -1967,10 +1967,10 @@ declare namespace Deno {
*
* If `stdout` and/or `stderr` were set to `"piped"`, they must be closed
* manually before the process can exit.
*
*
* To run process to completion and collect output from both `stdout` and
* `stderr` use:
*
*
* ```ts
* const p = Deno.run({ cmd, stderr: 'piped', stdout: 'piped' });
* const [status, stdout, stderr] = await Promise.all([
Expand Down Expand Up @@ -2097,14 +2097,14 @@ declare namespace Deno {
* console.log(obj); // prints same value as objAsString, e.g. { propA: 10, propB: "hello" }
* ```
*
* You can also register custom inspect functions, via the `customInspect` Deno
* symbol on objects, to control and customize the output.
* You can also register custom inspect functions, via the symbol `Symbol.for("Deno.customInspect")`,
* on objects, to control and customize the output.
*
* ```ts
* class A {
* x = 10;
* y = "hello";
* [Deno.customInspect](): string {
* [Symbol.for("Deno.customInspect")](inspect): string {
* return "x=" + this.x + ", y=" + this.y;
* }
* }
Expand Down Expand Up @@ -2289,9 +2289,13 @@ declare namespace Deno {
*/
export const args: string[];

/** A symbol which can be used as a key for a custom method which will be
/**
* @deprecated A symbol which can be used as a key for a custom method which will be
* called when `Deno.inspect()` is called, or when the object is logged to
* the console. */
* the console.
*
* This symbol is deprecated since 1.9. Use `Symbol.for("Deno.customInspect")` instead.
*/
export const customInspect: unique symbol;

/** The URL of the entrypoint module entered from the command-line. */
Expand Down
15 changes: 2 additions & 13 deletions op_crates/console/02_console.js
Original file line number Diff line number Diff line change
Expand Up @@ -907,22 +907,11 @@
inspectOptions,
) {
if (customInspect in value && typeof value[customInspect] === "function") {
return String(value[customInspect]());
}
// This non-unique symbol is used to support op_crates, ie.
// in op_crates/web we don't want to depend on unique "Deno.customInspect"
// symbol defined in the public API. Internal only, shouldn't be used
// by users.
const nonUniqueCustomInspect = Symbol.for("Deno.customInspect");
if (
nonUniqueCustomInspect in value &&
typeof value[nonUniqueCustomInspect] === "function"
) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing this logic means that Deno.customInspect will stop working. I think we should still support it until 2.0

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The non-unique custom inspect which we use for op crates implementations should be renamed to something else since the public custom inspect symbol has stolen its name. We handle the internal one with its own logic (i.e. pass it the inspect() function as an arg) so the difference should be preserved.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should always pass the inspect function as an arg - even for the public one.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK. I'll rename internal one to something else and keep the different handlings of public one and internal one.

// TODO(nayeemrmn): `inspect` is passed as an argument because custom
// inspect implementations in `op_crates` need it, but may not have access
// to the `Deno` namespace in web workers. Remove when the `Deno`
// namespace is always enabled.
return String(value[nonUniqueCustomInspect](inspect));
return String(value[customInspect](inspect));
}
if (value instanceof Error) {
return String(value.stack);
Expand Down Expand Up @@ -1771,7 +1760,7 @@
}
}

const customInspect = Symbol("Deno.customInspect");
const customInspect = Symbol.for("Deno.customInspect");

function inspect(
value,
Expand Down
2 changes: 2 additions & 0 deletions runtime/js/90_deno_ns.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@
truncateSync: __bootstrap.fs.truncateSync,
truncate: __bootstrap.fs.truncate,
errors: __bootstrap.errors.errors,
// TODO(kt3k): Remove this export at v2
// See https://github.com/denoland/deno/issues/9294
customInspect: __bootstrap.console.customInspect,
inspect: __bootstrap.console.inspect,
env: __bootstrap.os.env,
Expand Down