Skip to content

provide custom inspect behaviour in Node without using require#303

Merged
davidchambers merged 1 commit intomasterfrom
davidchambers/inspect
Jun 16, 2021
Merged

provide custom inspect behaviour in Node without using require#303
davidchambers merged 1 commit intomasterfrom
davidchambers/inspect

Conversation

@davidchambers
Copy link
Copy Markdown
Member

Fixes #300

@dotnetCarpenter and I came up with this solution last night on Gitter. :)

Assuming that CommonJS implies Node.js has caused people problems in several environments. It's exciting to be able to provide nice string representations of def-defined functions in the REPL without using the problematic require ('util')!

@davidchambers davidchambers requested a review from a team June 16, 2021 16:40
Comment thread package.json
Comment on lines +16 to +18
"engines": {
"node": ">=10.12.0"
},
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://nodejs.org/en/blog/release/v10.12.0/:

Copy link
Copy Markdown
Member

@Avaq Avaq left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I followed along with the conversation you had with @dotnetCarpenter. LGTM! :)

@davidchambers davidchambers merged commit c6caf1e into master Jun 16, 2021
@davidchambers davidchambers deleted the davidchambers/inspect branch June 16, 2021 21:42
Comment thread index.js
wrapped.toString = always0 (typeSignature (typeInfo));
/* istanbul ignore else */
if (
typeof process !== 'undefined' &&
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@davidchambers This is a great addition to sanctuary-def - I will try it out on scrimba.com soon.

I only want to add that process !== 'undefined' && process != null seems redundant.

process != null should be enough.

null == undefined // true
null != undefined // false

The Abstract Equality Comparison Algorithm says in step 2: If x is null and y is undefined, return true. Step 3 is similar but with x and y switched.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah, you don't want to create a global variable called process in non-node environments... Got it! Please disregard my comment ;)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also I think you pointed out in the chat that not guarding process will throw an error....

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The expression is verbose, but I don't think any part of it can safely be omitted.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CommonJS environment without "util" module

3 participants