Skip to content
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

Document no-op console methods #16755

Closed
unional opened this issue Nov 4, 2017 · 10 comments
Closed

Document no-op console methods #16755

unional opened this issue Nov 4, 2017 · 10 comments
Labels
console Issues and PRs related to the console subsystem. doc Issues and PRs related to the documentations. good first issue Issues that are suitable for first-time contributors.

Comments

@unional
Copy link

unional commented Nov 4, 2017

I just noticed starting from Node@8, console.debug() is defined but it is a no-op.

This causes my library to not behaving correctly.

It would be better to at least mention in the documentation that they are there but are no-op.

Are any of these has the same problem?
#1716 (comment)

@mscdex mscdex added console Issues and PRs related to the console subsystem. doc Issues and PRs related to the documentations. labels Nov 4, 2017
@maclover7
Copy link
Contributor

relevant #12675

@Trott Trott added the good first issue Issues that are suitable for first-time contributors. label Nov 4, 2017
@Tiriel
Copy link
Contributor

Tiriel commented Nov 5, 2017

Hi there! I'd like to pick this one up as my first contrib to core.
Already forked and built+tested the repo.

Any more information you can give me on this one, just in case?

@Tiriel
Copy link
Contributor

Tiriel commented Nov 5, 2017

From what I see, based on #12675 and on lib/console.js, these functions are all non-op:

console.debug                 console.dirxml                console.markTimeline
console.profile               console.profileEnd            console.table
console.timeStamp             console.timeline              console.timelineEnd 

Can someone confirm/infirm?

@daguej
Copy link
Contributor

daguej commented Nov 6, 2017

These are NOT actually no-ops!

All of the new-to-node console methods (that @Tiriel listed above) are real implementations for the Inspector. ie, they may appear to be no-ops unless you're running node with the --inspect option and then open chrome://inspect in Chrome and attach to your process. In the inspector's console, you'll see the results of using these methods (eg console.debug, console.profile, etc...); but you will not see anything on stdout.

There is no indication of these methods' existence in lib/console.js because they are not implemented in node. The methods were added by V8 starting with node v8.0 and are copied to node's console global.


Also, depending on how #15579 shakes out, this may be moot since that PR may end up removing these undocumented console methods.

@Tiriel
Copy link
Contributor

Tiriel commented Nov 7, 2017

Thanks for the input @daguej !
I'll check what comes out of this PR and see if this issue needs closing then.

If not, what about adding a new section 'Inspector only methods' explaining the deal and listing these methods?

@Tiriel
Copy link
Contributor

Tiriel commented Nov 13, 2017

Okay, trying to wrap my head around a first draft just in case.

As a first timer would-be committer, is there anything I should know? Are there some things automatically generated? Am I to write directly yhe markdown in the doc folder or is it somewhere else? In short, if someone as any kind of advice/how-to to provide me, that would be great.

@TimothyGu
Copy link
Member

@Tiriel There are nothing in doc/ that is automatically generated; directly editing the markdown files is enough.

@Tiriel
Copy link
Contributor

Tiriel commented Nov 13, 2017

@TimothyGu Awesome, thanks!

@unional
Copy link
Author

unional commented Nov 19, 2017

a commit implementing it just landed: 45e6642

Thanks @Tiriel 🌷

Tiriel added a commit to Tiriel/node that referenced this issue Nov 19, 2017
Description inspired by dev tools reference and inspector err messages

Added:
* intro
* console.debug()
* console.dirxml()
* console.markTimeline()
* console.profile()
* console.profileEnd()
* console.table()
* console.timeStamp()
* console.timeline()
* console.timelineEnd()

Fixes: nodejs#16755
Tiriel added a commit to Tiriel/node that referenced this issue Nov 19, 2017
Following comments on original PR, added warning in each method's description

Fixes: nodejs#16755
Refs: nodejs#17004 (comment)
Tiriel added a commit to Tiriel/node that referenced this issue Nov 19, 2017
Following review, changed one ref and added missing ones.
Also corrected casing on non-primitive values.

Fixes: nodejs#16755
Tiriel added a commit to Tiriel/node that referenced this issue Nov 19, 2017
Tiriel added a commit to Tiriel/node that referenced this issue Nov 19, 2017
Following review, removed tab references.

Fixes: nodejs#16755
Tiriel added a commit to Tiriel/node that referenced this issue Nov 19, 2017
As seen in PR nodejs#17033, console.debug() is being fully implemented.
Removing its description from here to include it in the other PR

Fixes: nodejs#16755
Refs: nodejs#17033 (comment)
Tiriel added a commit to Tiriel/node that referenced this issue Nov 19, 2017
Following review, replaced pseudo version number with REPLACEME

Refs: nodejs#17004 (review)
Fixes: nodejs#16755
MylesBorins pushed a commit that referenced this issue Dec 12, 2017
Description inspired by dev tools reference and inspector err messages

Added:
* intro
* console.debug()
* console.dirxml()
* console.markTimeline()
* console.profile()
* console.profileEnd()
* console.table()
* console.timeStamp()
* console.timeline()
* console.timelineEnd()

PR-URL: #17004
Fixes: #16755
Reviewed-By: Khaidi Chu <i@2333.moe>
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
MylesBorins pushed a commit that referenced this issue Dec 12, 2017
Description inspired by dev tools reference and inspector err messages

Added:
* intro
* console.debug()
* console.dirxml()
* console.markTimeline()
* console.profile()
* console.profileEnd()
* console.table()
* console.timeStamp()
* console.timeline()
* console.timelineEnd()

PR-URL: #17004
Fixes: #16755
Reviewed-By: Khaidi Chu <i@2333.moe>
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
MylesBorins pushed a commit that referenced this issue Dec 10, 2018
Description inspired by dev tools reference and inspector err messages

Added:
* intro
* console.debug()
* console.dirxml()
* console.markTimeline()
* console.profile()
* console.profileEnd()
* console.table()
* console.timeStamp()
* console.timeline()
* console.timelineEnd()

PR-URL: #17004
Fixes: #16755
Reviewed-By: Khaidi Chu <i@2333.moe>
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
console Issues and PRs related to the console subsystem. doc Issues and PRs related to the documentations. good first issue Issues that are suitable for first-time contributors.
Projects
None yet
Development

No branches or pull requests

8 participants
@mscdex @daguej @Trott @TimothyGu @maclover7 @unional @Tiriel and others