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

util: add inspectTag template literal tag #11130

Closed
wants to merge 1 commit into from

Conversation

jasnell
Copy link
Member

@jasnell jasnell commented Feb 2, 2017

Introduces a simple template literal tag that ensures that each constituent part of the template literal passes through util.inspect() using the default options.

This is inspired by recent changes in tests that have tried to convert console.log('message', a, b) to something like:

`message ${a} ${b}`
const inspectTag = require('util').inspectTag;
const m = {a: 1};

console.log(`${m}`);
// Prints: '[object Object]'

console.log(inspectTag`${m}`);
// Prints: '{ a : 1 }'
Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)

util

Introduces a simple template literal tag that ensures that
each constituent part of the template literal passes through
`util.inspect()` using the default options.

```js
const inspectTag = require('util').inspectTag;
const m = {a: 1};

console.log(`${m}`);
// Prints: '[object Object]'

console.log(inspectTag`${m}`);
// Prints: '{ a : 1 }'
```
@jasnell jasnell added semver-minor PRs that contain new features and should be released in the next minor version. util Issues and PRs related to the built-in util module. labels Feb 2, 2017

// Without the tag:
console.log(`${obj}`);
// Prints: '[object Object]'
Copy link
Contributor

@mscdex mscdex Feb 3, 2017

Choose a reason for hiding this comment

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

Can we place these before the line of code it's referring to? To me that seems to be the traditional place for such comments. Also the indentation makes it look out of place.

@cjihrig
Copy link
Contributor

cjihrig commented Feb 3, 2017

Seems like something good for userland. If core needs it, it could be internal, or in test/common.js.

@vsemozhetbyt
Copy link
Contributor

vsemozhetbyt commented Feb 3, 2017

I've just wanted something like this recently for Error.


assert.strictEqual(`${obj}`, '[object Object]');
assert.strictEqual(inspectTag`${obj}`, '{ a: 1 }');
assert.strictEqual(inspectTag`${obj}-${obj}`, '{ a: 1 }-{ a: 1 }');
Copy link
Member

Choose a reason for hiding this comment

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

Worth add a test when the end is a string(n === keys.length), and a test when there are no variables in the template.

Copy link
Contributor

Choose a reason for hiding this comment

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

Is not an empty string always in the end of a template literal?

Copy link
Member

Choose a reason for hiding this comment

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

Ah, yes.

Copy link
Member

@TimothyGu TimothyGu left a comment

Choose a reason for hiding this comment

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

Really like this idea, but I'll defer to other collaborators for comments on code


// With the tag:
console.log(inspectTag`${obj}`);
// Prints: '{ a : 1 }'
Copy link
Member

Choose a reason for hiding this comment

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

Another nit: '{ a: 1 }'

Copy link
Member

Choose a reason for hiding this comment

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

Mind that util.inspect({a: 1}) returns '{ a : 1 }'

Copy link
Member

Choose a reason for hiding this comment

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

> util.inspect({a: 1})
'{ a: 1 }'

It's also this case in the test that comes with it.

Copy link
Member

Choose a reason for hiding this comment

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

Oops, sorry, don't know where this comes from :/ Could be lack of coffee

added: REPLACEME
-->

A template literal tag that ensures each replacement in a template string is
Copy link
Member

Choose a reason for hiding this comment

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

Nit: A template literal tag function

@evanlucas
Copy link
Contributor

Why wouldn't you just use console.log('message', a, b) instead? I agree with @cjihrig. Why wouldn't we just do this in userland? -1 from me.

@jasnell
Copy link
Member Author

jasnell commented Feb 10, 2017

Will close given the feedback.

@jasnell jasnell closed this Feb 10, 2017
@alexweej
Copy link

This seems like an obvious omission to me...?

@BridgeAR
Copy link
Member

@alex-weej is there anything you can't do with console.log('message %s; object: %o; number: %i', 'test', { a: true }, 5) that you would be able to do otherwise (or using util.format(), if it's just about getting back a formatted string)?

@alexweej
Copy link

Really just about ergonomics. Logging is fine, but when you want to 'defer the logging' so to speak by stuffing some stuff in an Error, allowing some other piece of code to choose to log or not log later, you lose a lot of ergonomics.

throw new Error("The thing was broken: " + util.inspect(foo) + ", " + util.inspect(bar) + ", " + util.inspect(baz));

vs.

throw new Error(util.inspect`The thing was broken: ${foo}, ${bar}, ${baz}`);

What I see a lot of is people being lazy and just relying on some of them being strings, which inevitably leads to confusion when those strings contain unprintables, are empty, or the wrong runtime type entirely!

throw new Error("The thing was broken: " + someString);
> test("banana\x00\x01\x02rama")
Uncaught Error: The thing was broken: bananarama
    at test (repl:1:35)

^ Lossy

> test("")
Uncaught Error: The thing was broken: 
    at test (repl:1:35)

^ Confusing, you see this in a production log and it's not that obvious that someone was attempting to log an empty string.

> test(undefined)
Uncaught Error: The thing was broken: undefined
    at test (repl:1:35)
> test("undefined")
Uncaught Error: The thing was broken: undefined
    at test (repl:1:35)

These two are ambiguous. I've definitely debugged situations where the string "undefined" is floating around. Urgh!

> test({foo: 42})
Uncaught Error: The thing was broken: [object Object]
    at test (repl:1:35)

Any JS developer who hasn't seen something like this a zillion times is lying haha.

Hence, whatever we can do to make it easier to create unambiguous serializations for diagnostics would be a win in my book.

Perhaps a way to form a string using the exact same paradigm as the console.log family would be nice?

@mscdex
Copy link
Contributor

mscdex commented Jul 23, 2020

Perhaps a way to form a string using the exact same paradigm as the console.log family would be nice?

util.format()

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver-minor PRs that contain new features and should be released in the next minor version. util Issues and PRs related to the built-in util module.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants