util: remove instance check on empty object

[baylesa-dev]

Remove a useless check on Object descriptor. It seems like js browsers engine dont do this check.

Fixes: #35730

Checklist

• make -j4 test (UNIX), or vcbuild test (Windows) passes
• tests and/or benchmarks are included
• commit message follows commit guidelines

[aduh95]

Hi @baylesa-dev 👋 Thanks for opening this! Could you add a test to ensure the related issue doesn't happen again if we make other changes to this code in the future?

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/33799/

[codecov-io]

Codecov Report

Merging #35754 into master will decrease coverage by 0.06%.
The diff coverage is 100.00%.

@@ Coverage Diff @@## master #35754 +/- ## ========================================== - Coverage 87.92% 87.85% -0.07%  ========================================== Files 477 477 Lines 113090 113088 -2 Branches 24632 24606 -26 ========================================== - Hits 99431 99356 -75 - Misses 7948 8020 +72 - Partials 5711 5712 +1 

Impacted Files	Coverage Δ
lib/internal/util/inspect.js	93.41% <100.00%> (-2.26%)	⬇️
src/connect_wrap.h	25.00% <0.00%> (-75.00%)	⬇️
lib/internal/repl/history.js	88.16% <0.00%> (-4.15%)	⬇️
src/inspector_agent.cc	83.88% <0.00%> (-0.86%)	⬇️
lib/internal/encoding.js	84.06% <0.00%> (-0.53%)	⬇️
src/node_http_parser.cc	80.75% <0.00%> (-0.45%)	⬇️
lib/internal/url.js	96.12% <0.00%> (-0.41%)	⬇️
lib/_http_server.js	97.83% <0.00%> (-0.11%)	⬇️
src/module_wrap.cc	70.84% <0.00%> (+0.24%)	⬆️

[baylesa-dev]

@aduh95 Do you know why some checks weren't successful ? What could I do to resolve them?

[BridgeAR]

@baylesa-dev the check is not useless. It's a safeguard to find the correct prototype. To fix the issue, we'll have to wrap the instanceof check in a try / catch and return false in the error case.

This should be a full fix:

diff --git a/lib/internal/util/inspect.js b/lib/internal/util/inspect.js index e6787760fe..c281d36630 100644 --- a/lib/internal/util/inspect.js+++ b/lib/internal/util/inspect.js@@ -534,6 +534,14 @@ function getEmptyFormatArray(){return []} +function isInstanceof(object, proto){+ try{+ return object instanceof proto;+ } catch{+ return false;+ }+}+ function getConstructorName(obj, ctx, recurseTimes, protoProps){let firstProto; const tmp = obj; @@ -542,7 +550,7 @@ function getConstructorName(obj, ctx, recurseTimes, protoProps){if (descriptor !== undefined && typeof descriptor.value === 'function' && descriptor.value.name !== '' && - tmp instanceof descriptor.value){+ isInstanceof(tmp, descriptor.value)){ if (protoProps !== undefined && (firstProto !== obj || !builtInObjects.has(descriptor.value.name))){diff --git a/test/parallel/test-util-inspect.js b/test/parallel/test-util-inspect.js index 70f2ff6432..a8caf6f891 100644 --- a/test/parallel/test-util-inspect.js+++ b/test/parallel/test-util-inspect.js@@ -2866,6 +2866,17 @@ assert.strictEqual( )} +// Regression test for https://github.com/nodejs/node/issues/35730+{+ function Func(){}+ Func.prototype = null;+ const object ={};+ object.constructor = Func;++ assert.strictEqual(util.inspect(object), '{constructor: [Function: Func] }');+}+ // Test changing util.inspect.colors colors and aliases.{const colors = util.inspect.colors;

[ExE-Boss]

This change broke tests inspecting the prototype object, which is not an instance of its constructor.

[Trott]

Should this PR be closed or is work ongoing?

[ExE-Boss]

This PR has been superseded by #36178, which has now been merged.

Wrong button