-
Notifications
You must be signed in to change notification settings - Fork 30.3k
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
util: fix module inspection & instanceof check during inspect #36178
Conversation
@ExE-Boss I added a reference to the issue description but I'd just close the PR manually when this PR lands. |
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.
Actual fix LGTM (pending fixed messages changed in tests probably?)
Also - what semverness should this have?
Probably semver‑patch and be backported to v14.x, given that #33449 wasn’t a semver‑major change either. |
d330cd1
to
3199002
Compare
Signed-off-by: Ruben Bridgewater <[email protected]> Fixes: nodejs#36151
Signed-off-by: Ruben Bridgewater <[email protected]> Fixes: nodejs#35730
3199002
to
cb9eec9
Compare
@@ -534,6 +534,14 @@ function getEmptyFormatArray() { | |||
return []; | |||
} | |||
|
|||
function isInstanceof(object, proto) { | |||
try { | |||
return object instanceof proto; |
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.
TIL: instanceof
can throw
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.
Thanks so much for fixing this up. Really nice to surface the null prototype too.
The |
|
@ExE-Boss the failure is an old run and not properly updated :) |
Landed in 738cd60...1e66509 |
Signed-off-by: Ruben Bridgewater <[email protected]> Fixes: #36151 PR-URL: #36178 Fixes: #35730 Refs: #35754 Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Guy Bedford <[email protected]> Reviewed-By: James M Snell <[email protected]>
Signed-off-by: Ruben Bridgewater <[email protected]> Fixes: #35730 PR-URL: #36178 Fixes: #36151 Refs: #35754 Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Guy Bedford <[email protected]> Reviewed-By: James M Snell <[email protected]>
Signed-off-by: Ruben Bridgewater <[email protected]> Fixes: #36151 PR-URL: #36178 Fixes: #35730 Refs: #35754 Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Guy Bedford <[email protected]> Reviewed-By: James M Snell <[email protected]>
Signed-off-by: Ruben Bridgewater <[email protected]> Fixes: #35730 PR-URL: #36178 Fixes: #36151 Refs: #35754 Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Guy Bedford <[email protected]> Reviewed-By: James M Snell <[email protected]>
Is this likely to be backported to Edit: trying to follow https://github.com/nodejs/node/blob/master/doc/guides/backporting-to-release-lines.md myself! |
Signed-off-by: Ruben Bridgewater <[email protected]> Fixes: nodejs#36151 PR-URL: nodejs#36178 Fixes: nodejs#35730 Refs: nodejs#35754 Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Guy Bedford <[email protected]> Reviewed-By: James M Snell <[email protected]>
Signed-off-by: Ruben Bridgewater <[email protected]> Fixes: nodejs#35730 PR-URL: nodejs#36178 Fixes: nodejs#36151 Refs: nodejs#35754 Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Guy Bedford <[email protected]> Reviewed-By: James M Snell <[email protected]>
Backport-PR-URL: nodejs#37100 Signed-off-by: Ruben Bridgewater <[email protected]> Fixes: nodejs#36151 PR-URL: nodejs#36178 Fixes: nodejs#35730 Refs: nodejs#35754 Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Guy Bedford <[email protected]> Reviewed-By: James M Snell <[email protected]>
Backport-PR-URL: nodejs#37100 Signed-off-by: Ruben Bridgewater <[email protected]> Fixes: nodejs#35730 PR-URL: nodejs#36178 Fixes: nodejs#36151 Refs: nodejs#35754 Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Guy Bedford <[email protected]> Reviewed-By: James M Snell <[email protected]>
Backport-PR-URL: nodejs#37100 Signed-off-by: Ruben Bridgewater <[email protected]> Fixes: nodejs#36151 PR-URL: nodejs#36178 Fixes: nodejs#35730 Refs: nodejs#35754 Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Guy Bedford <[email protected]> Reviewed-By: James M Snell <[email protected]>
Backport-PR-URL: nodejs#37100 Signed-off-by: Ruben Bridgewater <[email protected]> Fixes: nodejs#35730 PR-URL: nodejs#36178 Fixes: nodejs#36151 Refs: nodejs#35754 Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Guy Bedford <[email protected]> Reviewed-By: James M Snell <[email protected]>
Backport-PR-URL: nodejs#37100 Signed-off-by: Ruben Bridgewater <[email protected]> Fixes: nodejs#36151 PR-URL: nodejs#36178 Fixes: nodejs#35730 Refs: nodejs#35754 Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Guy Bedford <[email protected]> Reviewed-By: James M Snell <[email protected]>
Backport-PR-URL: nodejs#37100 Signed-off-by: Ruben Bridgewater <[email protected]> Fixes: nodejs#35730 PR-URL: nodejs#36178 Fixes: nodejs#36151 Refs: nodejs#35754 Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Guy Bedford <[email protected]> Reviewed-By: James M Snell <[email protected]>
Backport-PR-URL: nodejs#37100 Signed-off-by: Ruben Bridgewater <[email protected]> Fixes: nodejs#36151 PR-URL: nodejs#36178 Fixes: nodejs#35730 Refs: nodejs#35754 Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Guy Bedford <[email protected]> Reviewed-By: James M Snell <[email protected]>
Backport-PR-URL: nodejs#37100 Signed-off-by: Ruben Bridgewater <[email protected]> Fixes: nodejs#35730 PR-URL: nodejs#36178 Fixes: nodejs#36151 Refs: nodejs#35754 Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Guy Bedford <[email protected]> Reviewed-By: James M Snell <[email protected]>
Signed-off-by: Ruben Bridgewater <[email protected]> Backport-PR-URL: #37100 PR-URL: #36178 Fixes: #35730 Fixes: #36151 Refs: #35754 Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Guy Bedford <[email protected]> Reviewed-By: James M Snell <[email protected]>
Signed-off-by: Ruben Bridgewater <[email protected]> Backport-PR-URL: #37100 PR-URL: #36178 Fixes: #35730 Fixes: #36151 Refs: #35754 Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Guy Bedford <[email protected]> Reviewed-By: James M Snell <[email protected]>
Signed-off-by: Ruben Bridgewater <[email protected]> Backport-PR-URL: #37100 PR-URL: #36178 Fixes: #35730 Fixes: #36151 Refs: #35754 Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Guy Bedford <[email protected]> Reviewed-By: James M Snell <[email protected]>
Fixes: #36151
Fixes: #35730
Refs: #35754
Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passes