Specify console.countReset

[domfarolino]

We've discussed adding a method countReset to console in #89. We've got two major implementations that have it, so it seems worth speccing, however the two behave differently. There are two things it seems we have to specify: How calls to the API should behave with a label that has:

• An associated count - console.countReset("labelWithCount")
  • In Edge, I believe "labelWithCount: 0" is produced
  • In Node, no output is produced.
• No associated count - console.countReset("labelWithNoCount")
  • In Edge, "null: 0" is produced the first time, and nothing is produced subsequent times
  • In Node, no output is produced when we reset the count that doesn't exist.

Here are the proposals I have in this PR:

• An associated count
  • I personally that no output should be produced upon resetting a valid label/count. It doesn't seem too worthwhile to print "0", since we know (after speccing) that the count will go back down to 0 so there's nothing new.
• No associated count
  • Here I learn towards something like how timeEnd() prints a warning indicating that no timer exists for the given label. All implementations do this right now (though I can't confirm Edge).

If we can get agreement on this from Edge (@xirzec / @TheLarkInn) and Node (@jasnell) and converge on either the behavior specified in this PR or something else, that would be great and we could finish the spec here and get other implementations on board. Thoughts?

Note this PR fixes another linking error unrelated to this change

[domenic]

The Edge reports aren't quite correct. They're more like:

• A string: console.countReset("labelWithCount")
  • Always produces labelWithCount: 0 the first time
  • Always produces no output subsequent times
• No arguments: console.countReset()
  • Always produces null: 0 the first time
  • Always produces no output subsequent times

[domfarolino]

The two points under "No arguments" also apply to console.countReset("labelWithNoCount") right?

[domenic]

No, then it produces the two points under "A string".

[domenic]

Actually, here's what's going on...

• When you call count("x"), Edge outputs x: 1 to the console
• When you call count("x"), Edge outputs x: 0 to the console
• If you subsequently call count("x"), Edge goes back and updates the previous output by incrementing it
• If you subsequently call countReset("x"), Edge goes back and updates the previous output by setting it to 0

I remember us discussing this "update previous stuff" behavior in some other thread. I think it's generally meant to be covered by https://console.spec.whatwg.org/#printer-ux-innovation.

So there are basically two questions:

• Should countReset call Printer with the new count of 0? (This may result in a separate log entry, or may result in updating a previous log entry.)
• Should countReset allow "resetting" (and perhaps subsequently printing) counters that don't exist yet?

[domfarolino]

IMO I don't think countReset should call the printer. I'd like to see it only reset the internal count instead, but since Edge is one of the only two implementations that do this, we'll have to see what they think.

Regarding the second question, I'd like to align this as closely with timeEnd() as possible (the two sets of functions have nice parallels I think). So, print a warning and don't touch any counters. Thoughts?

[domfarolino]

Pinging @Fishrock and @devsnek for Node as well

[devsnek]

imo:

if the count doesn't exist, return, optionally reporting a warning to the console.
otherwise, set map[label] to 0.
perform logger("count", concat).

as this method is a part of console, explicitly calling the logger makes a lot of sense in my mind.

[jasnell]

I'd rather countReset not invoke the printer and I'm just fine with a warning when the counter being reset does not exist.

I think we should also be clear that calling countReset on a non-existent counter does not create the counter.

[xirzec]

I'm fine with changing the behavior to not print/update the count when calling countReset().

If we are updating previous instances though, should a call to count() after countReset() still update the previous log entry? Or should countReset() cause the next count() to start a new entry that gets updated?

cc: @TheLarkInn

[domfarolino]

I agree with @jasnell on:

• Not invoking Printer
• Giving a warning when a count does not exist
• Not setting a count for a not-already-counted label

These things are currently covered by the spec I have proposed.

@xirzec, Given Domenic's previous comment about Printer UX innovation, it seems either of those options are valid. If it were up to me I kind of like the idea that countReset() causes the next count() to start a new update-able entry, but either way...

Since we have agreement on this from Edge and Node, I'll merge this right after web-platform-tests/wpt#10722 and file some browser bugs.

[fmartin5]

I think it would be possible to align the specs for console.countReset() and console.timeEnd()
more closely if the optionality of the warning step was the same in both specs.
Currently, that step is optional in console.timeEnd() but not in console.countReset(). The warnings could be both required or both optional.

Also, the spec for console.timeEnd() does not explicitly prescribe what 'logLevel' should be used for the warning, whereas console.countReset() does.

[domfarolino]

I agree with aligning the specs for console.countReset() and console.timeEnd(). The warning is required in countReset(), and there is actually no mention of it for timeEnd(). That should be specced as mandatory IMO, and every implementation I've tested does it.

No, the spec for both countReset() and timeEnd() does indicate logLevel. They are just unique, to give implementations freedom, and as the spec mentions, are often grouped under similar but not hardened categories.

[fmartin5]

My apologies, I was wrong. It is console.time() which specifies an optional warning, with no particular logLevel for it:

If the associated timer table contains an entry with key label, return, optionally reporting a warning to the console indicating that a timer with label label has already been started.

These warnings have something in common, being not the primarily intended output of the method calls, but alerting the developer about an incorrect usage of the console API.

I am not questioning the idea of having every console method pass its own name as the logLevel for its usual output, I think it's a great design choice.

But I'm not sure the same logLevel should be used alone for special output, because in the future a given console method could have the ability to display either its usual output or a special warning, and implementers might need the freedom to make the warning look different from the usual output, to help developers more easily distinguish between them.

Hence my allusion to the possibility of leaving the logLevel of countReset()'s warning as unspecified for now, since the warning is not an usual output, and console.time() does the same.

[domfarolino]

Bugs:

• Node: console: countReset() should dispatch a warning on non-existent counts nodejs/node#20524
• Safari: https://bugs.webkit.org/show_bug.cgi?id=185307
• Firefox: https://bugzilla.mozilla.org/show_bug.cgi?id=1459279
• Chrome: https://bugs.chromium.org/p/chromium/issues/detail?id=839947
• Edge: https://developer.microsoft.com/en-us/microsoft-edge/platform/issues/17446852/ (@xirzec )

---

@fmartin5 Thanks for your input! I very much agree with your thoughts on not using the existing infrastructure for reporting errors, and I think this is a great task for #57. The solution for this should define some infrastructure responsible for printing things other than intended console output (and define hook-able infra for other specs to tap into). I can make this one of my next priorities because I agree it is pretty important. For now, I think we should use the current infrastructure (Logger/Printer etc) until this changes, if that makes sense?