-
Notifications
You must be signed in to change notification settings - Fork 30.9k
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
console.assert(false, Symbol())
throws
#49680
Comments
Working as expected/documented. Try this instead: console.assert(false, "%o", { hello: "world" })
// or even just:
console.assert(false, "", { hello: "world" }) It wouldn't be hard to use
You're welcome to open a pull request and see how it's received if you still think it's a good idea but be prepared for rejection. |
This comment was marked as outdated.
This comment was marked as outdated.
Yes, I'm pretty sure. The more human-readable prose from the MDN lemma (emphasis mine):
It's only when you use a message format string that object formatting comes into play:
|
After further investigation, here's the erroneous line: node/lib/internal/console/constructor.js Line 435 in cd97e28
👆 this isn't precisely spec compliant. Why? Because there's supposed to be this: which means something more like this: assert(expression, ...args) {
if (!expression) {
if (typeof args[0] === "string") {
// if you provide a fmt string, it still gets used (first arg)
args[0] = `Assertion failed: ${args[0]}`
} else {
// but if it's anything else, it's not stringified as a format string;
// its just an object to be printed. so we prepend a fmt string arg (with no specifiers
// that would eat any user-supplied objects) as a nice "Assertion failed" message
ArrayPrototypeUnshift(args, "Assertion failed")
}
// The arguments will be formatted in warn() again
ReflectApply(this.warn, this, args);
}
}, you can see a bug here from the current Node.js impl that highlights what's happening: const mySymbol = Symbol()
console.assert(false, mySymbol)
// THROWS because Symbol() cant be stringified like `symbol string: ${Symbol()}`! the mdn thing you cited:
this same language "string representations" is also used in console.info() and other console methods to mean the formatted pretty output not the also backed up by someone from the whatwg/console repo:
so in summary it would seem:
i also am making an effort to clarify the mdn wording that had us all confused 🤣 mdn/content#29172 |
console.assert(false, { hello: "world" })
logs [object Object]
instead of inspecting the objectconsole.assert(false, Symbol())
throws
Changed title and body to reflect exact buggy impl: console.assert(false, Symbol()) |
@bnoordhuis I believe it's fine to handle it that way. It aligns well with the other APIs where the object would be printed in a human readable way. There's also already an open PR and I guess we could reopen the issue as such? |
Yeah, I'm fine with that, I don't really have a strong opinion either way except that changing the default should be a semver-major change. |
fixes #49680 PR-URL: #49722 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Matteo Collina <[email protected]>
fixes nodejs#49680 PR-URL: nodejs#49722 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Matteo Collina <[email protected]>
Version
v20.6.1
Platform
Linux PIG-2016 5.15.90.1-microsoft-standard-WSL2 #1 SMP Fri Jan 27 02:56:13 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux
Subsystem
console
What steps will reproduce the bug?
node -e 'console.assert(false, Symbol())'
How often does it reproduce? Is there a required condition?
No response
What is the expected behavior? Why is that the expected behavior?
What do you see instead?
Additional information
No response
The text was updated successfully, but these errors were encountered: