Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
errors: extract type detection & use in
ERR_INVALID_RETURN_VALUE
#43558errors: extract type detection & use in
ERR_INVALID_RETURN_VALUE
#43558Changes from 1 commit
74be6fe
572c162
4a4204e
b757821
4871b85
96dbd0e
d2eded5
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Well that's just untrue x)
Object.create(null) instanceof Object === false
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.
Technically untrue, yes, but I think being helpful and a tiny bit wrong is better.
I had originally put
'an instance of Object (null Prototype)'
, but decided that was TMI; if I put that in, would it address your concern? (Or perhaps'type object (null Prototype)'
or any other similar flavour)Otherwise you end up with a error message like
Expected an instance of Map, but instead got [Object: null prototype] {}
(which looks ugly and kind of confusing)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.
I very much disagree here, I feel quite strongly that error messages must always be technically correct first, to me I believe one can't be helpful if they are misleading.
I think it should not be special cased,
type object ([Object: null prototype] {})
is good enough to me. What is it that you don't like about it exactly?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.
It looks bizarre before your suggested change below—it didn't have
type
(I think it's better now with your suggestion), it was justgot [Object: null prototype] {}
);type object (null Prototype)
seems more human-friendly to me, and similar to other type outputs (extype number (2)
), but I can live withtype object ([Object: null prototype] ...)
.Note that the actual output with your suggestion below is
type object ([Object: null prototype] ...)
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.
I definitely prefer the former output: limiting the string was only done for primitives and functions. Objects have not been inspected closer and just the most outer layer was inspected. That's not the case anymore and would possibly hide important information. The reason not to add
type object
is that it should be obvious that it's an object as everything is that's not a primitive (or function). It was therefore a way to limit the output to the necessary information.Please change that back.