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

minor request on logging classes #32270

Closed
jimmywarting opened this issue Mar 14, 2020 · 9 comments
Closed

minor request on logging classes #32270

jimmywarting opened this issue Mar 14, 2020 · 9 comments
Labels
feature request Issues that request new features to be added to Node.js. util Issues and PRs related to the built-in util module.

Comments

@jimmywarting
Copy link

jimmywarting commented Mar 14, 2020

When logging object, functions and classes you sometimes see function and classes being printed the same way as such:

Skärmavbild 2020-03-14 kl  21 28 43

I wish they where a bit more distinguish if it is a function or a class so you know if you have to call the "function" class with the new keyword and expect something back
kinda like chrome's devtool:

Skärmavbild 2020-03-14 kl  21 32 30

with the prefixed class and f in the beginning

maybe with an other color (same as the number color - orange) or like my atom editor:
Skärmavbild 2020-03-14 kl  21 38 41

it's not so much about styling, (the above are just some suggestions)
the request feature is just more about distinguish classes from functions in the terminal

(don't mind as much if this is being discarded and closed since it's not so important to me - just wishful)

@addaleax addaleax added feature request Issues that request new features to be added to Node.js. util Issues and PRs related to the built-in util module. labels Mar 15, 2020
@addaleax
Copy link
Member

I think implementing this could call .toString() on the function and check whether that starts with class – that should be doable.

@BridgeAR
Copy link
Member

BridgeAR commented Mar 16, 2020

@addaleax I already have a local implementation that does exactly that. It is a bit tricky handling the edge cases though. E.g. const a = { class () {} }. We have to make sure we only match correct entries.

@jimmywarting
Copy link
Author

It's a start...

typeof v === 'function'
  && v.prototype // undefined if arrow fn
  && v.constructor?.name !== 'GeneratorFunction'
  && /^class\s/.test(Function.prototype.toString.call(v))

maybe should be some native builtin way to handle it.

on a side note i notice that chrome devtool also prints f* name(){} for generator functions - that * is also useful

@devsnek
Copy link
Member

devsnek commented Mar 16, 2020

I'm -1 on testing for class because that fails on babel output and just people who still write classes using function. the fact of the matter is that some js functions need to be called with new (ones created with the class keyword, builtins, babel output, etc), and some don't, and there's no way to know which.

@BridgeAR
Copy link
Member

@devsnek if I understand correct you worry about people relying upon the suggested class output in a way that it's always possible to invoke all functions without new?

I would not worry about that since it's no guarantee, just an indicator?

@devsnek
Copy link
Member

devsnek commented Mar 16, 2020

based on the OP it seems like people would rely on this. my concern is that something this finicky it might just end up being noise.

@jimmywarting
Copy link
Author

jimmywarting commented Mar 16, 2020

honestly I wouldn't rely on it that much. I just don't see classes as a function so i don't think they should be named as such to begin with.

classes are extensible program-code-template for creating objects, providing initial values for state (member variables) and implementations of behavior (member functions or methods).

i.e: not a function that you can call and expect something to happen or give you a result. it's something you instantiate.

it's like @BridgeAR said: it's no guarantee, just an indicator to help you distinguish things out from the mass. and helping you figure out what things are a little bit earlier.

--

@BridgeAR, do you mind sharing your local implementation that solves this?

@devsnek
Copy link
Member

devsnek commented Mar 16, 2020

@jimmywarting regardless of where this issue goes, i'd suggest changing your mental model of classes because they're just sugar for
function YourClass() { if (new.target === undefined) { throw new TypeError() } }

@BridgeAR
Copy link
Member

@jimmywarting I just opened a PR to implement this. It includes lots of edge cases in the tests in case you are interested in that. Chromium just prints the plain stringified value while this strips comments and adds further information.

codebytere pushed a commit that referenced this issue Jun 18, 2020
This outlines the basic class setup when inspecting a class.

Signed-off-by: Ruben Bridgewater <ruben@bridgewater.de>

PR-URL: #32332
Fixes: #32270
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com>
codebytere pushed a commit that referenced this issue Jul 6, 2020
This outlines the basic class setup when inspecting a class.

Signed-off-by: Ruben Bridgewater <ruben@bridgewater.de>

PR-URL: #32332
Fixes: #32270
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Issues that request new features to be added to Node.js. util Issues and PRs related to the built-in util module.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants