whatwg / console

Console Standard
https://console.spec.whatwg.org/
Other
269 stars 67 forks source link

What should `console.assert(false, { a: 1 })` print? #226

Closed jcbhmr closed 11 months ago

jcbhmr commented 11 months ago

gist of it: which one of these is the "right" way? image image

https://github.com/nodejs/node/issues/49680 https://github.com/mdn/content/issues/29172

it seems from the spec https://console.spec.whatwg.org/#assert that it all gets funneled to Logger() which unfurls all that data stuff the same way regardless of which console.* method was used but then again MDN says this: image as cited here https://github.com/nodejs/node/issues/49680?notification_referrer_id=NT_kwDOA6PV_7M3NzQ0NjE3OTU0OjYxMDY4Nzk5#issuecomment-1722528493 https://developer.mozilla.org/en-US/docs/Web/API/console/assert

...soooo yeah I opened this issue to hopefully get someone who knows how to read specs to clarify which way is the "spec way" or if I'm misunderstanding something 🤷‍♂️😵

domfarolino commented 11 months ago

Can you clarify your question? Which output is unexpected for you, and why?

terinjokes commented 11 months ago

If I'm reading the screenshots and the linked Node.js issue correctly, the unexpected output is "[object Object]" from Node.js converting all data elements to string.

My understanding of the spec is the object {hello: "world"} would get passed as a member of args (eg, ["Assertion failed", {hello: "world"}]) to Printer, which is implementation defined. Converting a POJO to the string "[object Object]" is certainly a valid way of being implementation defined, but I wouldn't judge it "to be maximally useful and informative."

jcbhmr commented 11 months ago

@domfarolino Specifically my question is this: Which output is the "correct" one? Assertion failed: [object Object] or Assertion failed: ▶ { hello: 'world' } ?

Or, more generally, which of these is "correct" according to the spec?

image image

My confusion comes from reading the spec and surmising this sorta equivalence:

console.assert(false, {a:1}, {b:2}) // is sorta like
console.log("Assertion failed", {a:1}, {b:2})

console.assert(false, "it failed %o", {b:2}) // is sorta like
console.log("Assertion failed" + ": " + "it failed %o", {b:2})

but that Node.js seems to do the "Assertion failed: " + data[0] all the time instead of only if it's a string. https://github.com/nodejs/node/blob/cd97e28860d109e1576e05b4b79692aa013a4e6e/lib/internal/console/constructor.js#L435

// nodejs essentially ALWAYS does this even if its an object:
console.assert(false, {a:1}, {b:2}) // is sorta like
console.log("Assertion failed: " + {a:1}, {b:2})

Working as expected/documented. Yes, I'm pretty sure.

https://github.com/nodejs/node/issues/49680

but the spec says: image

so looking for a more authoritative whatwg/console answer on which one (Node.js vs Chrome/Firefox) is the more "correct" implementation ❤

terinjokes commented 11 months ago

I'd argue, as above, that both are correct. How any arguments, outside of specifiers, are printed is implementation defined, so [object Object] is equally correct as {"hello": "world"}. I don't think MDN's language is technically wrong (a string, of some sort, is printed), but it also implies behavior that isn't in the specification, and isn't found in their description of other logging methods.

I would expect, in most environments, objects printed in assert in a similar fashion to how they're printed in the other logging methods. However, as it's implementation defined, they may chose not to.

jcbhmr commented 11 months ago

I found an edge case that exemplifies Node.js' implementation as having some kind of bug:

console.assert(false, Symbol())
// THROWS! becase `Assertion failed: ${Symbol()}` will fail

from this line: https://github.com/nodejs/node/blob/cd97e28860d109e1576e05b4b79692aa013a4e6e/lib/internal/console/constructor.js#L435 switched the Node.js issue to focus on that instead https://github.com/nodejs/node/issues/49680

and I've also changed my MDN issue to focus on improving the wording on a bunch of the console methods. https://github.com/mdn/content/issues/29172

jcbhmr commented 11 months ago

@terinjokes Yep! You answered my question! ❤️ Thanks for clarifying the implementation-defined nature of the printing and explaining the spec logic for me. 👍

terinjokes commented 11 months ago

The fact that Node.js prints [object Object] only for the first object is curious. Without reviewing the code it suggests to me that it's converting the second parameter to a string either to concatenate with the "Assertion failed" message, or to check for specifiers (or both), neither of which would be following this standard.

jcbhmr commented 11 months ago

Yep! Here's the relevant bit:

  assert(expression, ...args) {
    if (!expression) {
      args[0] = `Assertion failed${args.length === 0 ? '' : `: ${args[0]}`}`;
      // The arguments will be formatted in warn() again
      ReflectApply(this.warn, this, args);
    }
  },

https://github.com/nodejs/node/blob/cd97e28860d109e1576e05b4b79692aa013a4e6e/lib/internal/console/constructor.js#L435

Note that it always treats args[0] as a format string (coercing it) which is why ${Symbol()} throws.

I've chosen to focus on that particular edge case to try and exemplify how the implementation could be improved in https://github.com/nodejs/node/issues/49680