-
Notifications
You must be signed in to change notification settings - Fork 72
console.context() #193
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
Comments
it was added as an experiment a while ago in Chrome: https://bugs.chromium.org/p/chromium/issues/detail?id=728767 we don't have such thing in Firefox as it's not part of the standard, but I can see it being useful |
Interesting, it is a shame that it was never standardized in Chrome. I've posted https://groups.google.com/a/chromium.org/g/devtools-dev/c/umeCeS3Bgcs to see if I can get someone from the Chromium DevTools team to lend a hand here, especially since @nchevobbe seems to think that this would be useful, and thus we have a shot at getting multi-vendor support. |
I filed a bug for Firefox: https://bugzilla.mozilla.org/show_bug.cgi?id=1948870 |
I'm planning to attempt specifying this. @domfarolino I see you assigned it to you, do you already started looking into it? |
Thanks for the update @nchevobbe, feel free to take a look. I've co-assigned you. |
I was hoping I could use this to solve other problems with console but it didn't work as my intuition would have guessed. I expected it to be a copy of the console object, including a copy of its state (active group, profile timer etc) The problem I have with console is I often want to group messages into a meaningful group or context console.group('example-request');
let data = await longRequest();
console.log('got data');
let image = await longRequest(data.url);
console.log('got image');
console.groupEnd(); The problem here is that the await here means we yield to executing other code and so all other logs will end up in our group so you might see in the log
i.e. unexpected nesting if the request is called multiple times and random other logs included in the group console.context() would seem like the perfect solution: const ctx = console.context('example-request')
ctx.group('request-group')
let data = await longRequest();
ctx.log('got data');
... (same as above but using ctx) This way our group wouldn't be polluted by other messages during the awaits
I think before we standardise console.context(), it would be good to specify how it should handle internal state, and if it cannot separate state I'd argue for renaming for clarity |
Thank you @nchevobbe for planning to work on the spec changes. Note that we, on the Edge team, have been working on a plan to improve contextual logging in Chromium, and have an explainer document almost ready to go. Here is the PR: MicrosoftEdge/DevTools#323 if you want to see the current state of our proposal. Let me at-mention the Edge PM who's leading this proposal so she's aware of this thread: @leahmsft. Note that our explainer comes with a proposal to give contexts a color, and that we'd love for |
Thanks @captainbrosset for the heads up. I wonder if we should go even further and accept |
I'd keep the context customization rather simple and I think (although that would be implementation dependent) it should only apply to a context badge that appears before the message itself. The reason for this is that, this way, the context color(s) don't interfere with message colors, such as those coming from the warn and error methods, or the %c styling that a user might have done at the message level already. So, I think I'm on board with the idea of color + backgroundcolor (and for a way for it to work in light and dark modes), but I wouldn't go further than this. |
That said, if the way to make this work requires users to provide 4 colors (text and background colors for both light and dark themes), that's not great either. So, maybe accepting |
Our explainer is live at: https://github.com/MicrosoftEdge/MSEdgeExplainers/blob/main/ContextualLoggingWithConsoleContext/explainer.md Please open an issue for feedback on the explainer. Any feedback would be greatly appreciated! |
This specifies the console.context(label) method which returns a new console namespace object with an optional context name label. Tests: TBD Fixes whatwg#193
Started a WIP PR for this method. |
While working on this, I wondered about a few things:
Let's discuss :) |
Seeing that a non-negligible number of websites already use try {
-1 !== i.userAgent.indexOf("Chrome") && (sa = 0,
a.console.context().debug(Object.defineProperty(Error(), "stack", {
get: function() {
return sa++,
""
}
})))
} catch (t) {} My guess is that this is part of a library that is used by many, and that they're doing this because it gives them a way to filter debug logs per file. I have not tested, but I think omitting the context name in Chrome ends up creating a new entry under the Verbose category in the Console sidebar named after the source file name:
Forcing it to be unique means apps have to pass the context object around. Wouldn't it be simpler if you could just call |
Thanks for you answers @captainbrosset
Sounds good
I'm not sure I'm on board with this. If multiple libraries use the same context name (or no context name), this would be confusing to consider them being the same context (even if in the end it shouldn't matter too much for the user). For example if you have something like this: /* myScript.js */
const logger = console.context();
logger.time();
// …
logger.timeEnd();
/* someLibrary.js */
const logger = console.context();
logger.time();
// …
logger.timeEnd(); since |
In my PR, console.time("hello");
console.context().timeEnd() You get the following warning: I think the issue with
So I don't think there's anything specific we need to do at the moment; we should file bugs on implementers trackers so they would implement the spec (for Firefox we have https://bugzilla.mozilla.org/show_bug.cgi?id=1441771). console.log("message 1");
console.group("group 1");
console.context("myContext").log("message 2");
console.log("message 3") the "correct" output would be:
that's also something that would be hard to do in the terminal (e.g. for Node/deno) |
Groups are implementation defined, as is most of the UI of console. For example, I'd expect a server implementation to include group and context information as part of the key-values for a logfmt log line. If implementors find that interactive groups while displaying multiple contexts is too confusing to users, they can certainly display them in other ways. |
In the spirit of #27 and #74 I'm filing issues about two additional console members I've spotted as part of foolip/mdn-bcd-collector#917.
console.context(name)
is available in Chrome and Node.js, and returns a new object with a lot of theconsole
members. This test gives some clues about what it does:https://chromium.googlesource.com/v8/v8/+/23d0a6a5125d407655f2b7f6bb7263b4e05019d8/test/inspector/runtime/console-context.js
It looks like
console.context('bla').log('hello')
doesn't log anything in Node.js, but logs "hello" just likeconsole.log("hello")
would in Chrome.Does anyone recognize what this is, and would it be useful to standardize? I expect not, but want to check.
The text was updated successfully, but these errors were encountered: