-
Notifications
You must be signed in to change notification settings - Fork 5.4k
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
fix(console) - log function object properties / do not log non-enumerable props by default #9363
Conversation
This changes the output of a bunch of things which also differs from Node (since Node was mentioned as the reason for the change).
@nayeemrmn, any thoughts? |
Good catch @caspervonb . I think the root cause here is that deno logs non-enumerable symbols by default, while node doesn't: user@ubuntu:~/proj/deno$ node -e "let x = {}; let s = Symbol('foo'); Object.defineProperty(x, s, {enumerable: false }); console.log(x)"
{}
user@ubuntu:~/proj/deno$ deno eval "let x = {}; let s = Symbol('foo'); Object.defineProperty(x, s, {enumerable: false }); console.log(x)"
{ [Symbol(foo)]: undefined } If that behavior were changed, then this PR would be aligned with Node for function constructors like Array. It shouldn't hard to add that on. |
…properties. Updating tests to reflect.
I've updated this PR to have Deno.inspect not log non-enumerable properties by default, and I've added a new flag named "showNonEnumerable" to InspectOptions. Setting this flag to true will show non-enumerable properties, similar to setting showHidden=true in node's inspect function. EDIT: I apologize for the failing tests- I will need to update the tests for the new expected output. A lot of integration tests were depending on logging non-enumerable properties. EDIT 2: tests are now updated and passing. |
If there's prior art for this option, maybe we should just adopt that prior art? (Potential bikeshed, but LGTM) |
… with established standards
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Chrome seems doing the different thing for non-enumerable symbols, but we had #3165, so this change make sense to me. LGTM.
@DavidDeSimone Thank you for your contribution! |
Addresses #9333
Running
deno eval "const x=()=>'x'; x.y=()=>'y'; x.z = function z() {return 'z'}; console.log({x});"
now properly logs the function's properties. I've added a basic test for this behavior.