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.

Sign up for GitHub

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

Jump to bottom

doc: clarify util.inspect usage intent #17375

Closed
wants to merge 1 commit into from
Closed

doc: clarify util.inspect usage intent #17375

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions doc/api/util.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -348,8 +348,9 @@ changes:
line. Defaults to 60 for legacy compatibility.

The `util.inspect()` method returns a string representation of `object` that is
primarily useful for debugging. Additional `options` may be passed that alter
certain aspects of the formatted string.
intended for debugging. The output of `util.inspect` may change at any time
and should not be depended upon programmatically. Additional `options` may be
passed that alter certain aspects of the formatted string.
Copy link
Member

Choose a reason for hiding this comment

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

Somehow this sounds a bit harsh to me. Would you be fine to change it to something like

The `util.inspect()` method returns a string representation of `object` that is
primarily intended for debugging. Additional `options` may be passed that alter
certain aspects of the formatted string.
Note that the output of `util.inspect` may undergo minor changes from time to
time without a semver-major step. Programmatically relying on the output is
therefore not recommended.

Copy link
Member Author

Choose a reason for hiding this comment

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

i kinda want it to be harsh, there's a reason stuff like internalBinding has to be a thing now

Copy link
Member

Choose a reason for hiding this comment

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

With your comment in place as is I would argue that landing #17576 as new default without putting it behind a option could be done as a semver-patch. And I do not see that at all.

Copy link
Member

Choose a reason for hiding this comment

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

With your comment in place as is I would argue that landing #17576 as new default without putting it behind a option could be done as a semver-patch. And I do not see that at all.

I'd rather we be harsher in the docs , but more conservative when actually landing possibly breaking PRs, so +1 on keeping this wording, but making #17576 major.

Realistically we know that if enough people depend on the output of util.inspect, no matter how internal, we won't be able to change it. But if this warning helps dissuade people from relying on that output, then that seems like a good thing.

Also if this PR makes #17576 a patch, that would make this PR semver-major 😁 .

`util.inspect()` will use the constructor's name and/or `@@toStringTag` to make an
identifiable tag for an inspected value.

Expand Down