| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
There was a problem hiding this comment.
I suggest using the same signature as in the spec: console.debug(object [,object, ...]).
Sorry, something went wrong.
There was a problem hiding this comment.
Then we might want to change the complete doc for console. Because the signature to methods like log are also not matching with spec we are following https://github.com/nodejs/node/blob/master/doc/api/console.markdown#consolelogdata- Do you think I should change them as well?
Sorry, something went wrong.
There was a problem hiding this comment.
Yes, I think changing the signatures to match the spec would be preferred. Can you do so in a second commit?
Sorry, something went wrong.
|
Please also add some tests for these methods here. |
Sorry, something went wrong.
|
Wouldn't this need to be semver-major as it could break anyone who has monkey patched these? (same issue as nooping the undefined ones) |
Sorry, something went wrong.
|
I don't think we have to support broken polyfills or monkey patches, and I think it's unlikely anyone would do so in the case of these methods. |
Sorry, something went wrong.
|
Hm. on the converse side, this lets things designed for the browser run as is, even if they use these features. |
Sorry, something went wrong.
It seems low risk. If code is monkeypatching the console object, that code's new debug/exception methods should (continue to) win. I might be missing something though — do you have a link to the problems arising from noop'ing handy? |
Sorry, something went wrong.
|
@silverwind I have added the tests for this and changed the signture of newly added aliases as per browser spec. Besides that I created a new PR #3584 in which I changed the method signatures which were there previously in doc to follow the browser spec. I haven't changed the signatures of methods which have differnent behaviour in node than in browser (e.g. console.assert). Now I am in confusion if we are actually following the spec? |
Sorry, something went wrong.
|
@aks- thanks, I'll have a look later. This PR is LGTM. |
Sorry, something went wrong.
|
@aks- on second look, what's up with tools/v8-prof/tick-processor-tmp-1221 in this patch? |
Sorry, something went wrong.
- add common aliases as per following spec - https://github.com/DeveloperToolsWG/console-object/blob/master/api.md
|
@silverwind sorry I added that by accident I guess...removed it now |
Sorry, something went wrong.
Sorry, something went wrong.
|
LGTM, will likely land this tomorrow. |
Sorry, something went wrong.
|
I'm not sure this is a good idea actually if we aren't going to follow the rest of the "spec". @aks- Sorry, my fault for pulling you into this console-spec thing. :/ |
Sorry, something went wrong.
|
@Fishrock123 I think we should implement as much of this "spec" as is feasible. What's wrong with these alias methods? |
Sorry, something went wrong.
|
Nothing particularly but it's more things we need to keep supporting. Seems low-value -- stuff works fine if it follows this part but not another part kind of thing. |
Sorry, something went wrong.
|
@Fishrock123 it's okay, I am actually learning a lot about the structure of node codebase :) |
Sorry, something went wrong.
|
Guess I'll put this on hold then. Interestingly, MDN says debug is deprecated and it lists exception only as _exception. I think some more research into browser behaviour is needed. |
Sorry, something went wrong.
|
See #3584 (comment), the situation is generally very silly. |
Sorry, something went wrong.
|
Closing due to lack of activity. Seems like there's very little interest in this in general. |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.