Node: util.format('%s', o) fails to call String(o) in certain cases

[cpcallen]

• Version: v12.12.0
• Platform: Darwin [redacted] 18.7.0 Darwin Kernel Version 18.7.0: Sat Oct 12 00:02:19 PDT 2019; root:xnu-4903.278.12~1/RELEASE_X86_64 x86_64
• Subsystem: util

I was very surprised to discover that in node v12, util.format('%s', v) does not consistently call String(v). Checking the docs, I read:

%s: String will be used to convert all values except BigInt, Object and -0. BigInt values will be represented with an n and Objects that have no user defined toString function are inspected using util.inspect() with options { depth: 0, colors: false, compact: 3 }.

This is a surprising (and in my view undesirable) change from the previous behaviour, which was to consistently call String in response to %s. It also appears to be a semantically breaking change introduced, via #27621 and #27799, in 12.3.0, despite that not being a new major version. If so, it should be entirely reverted (and good riddance, in my opinion).

If there is some compelling argument for why the new behaviour is actually desirable, then at least the code should be made consistent with the documentation or vice versa.

In particular, whether a user defined toString function is called depends in non-documented and non-obvious ways on the value of the .constructor property of the object, which is otherwise normally of no significance:

// ES6 subclassing:
class A {
  toString() { return 'custom A'; }
}
class B extends A {}

// ES5 subclassing:
function C() {}
C.prototype.toString = function() { return 'custom C'; };

function D() { C.call(this); }
D.prototype = Object.create(C.prototype);
// What if we forget to set the .constructor?
// D.prototype.constructor = D;

console.log('%s', new A());
console.log('%s', new B());
console.log('%s', new C());
console.log('%s', new D());

// Fix forgotten .constructor:
D.prototype.constructor = D;
console.log('%s', new D());

Actual output:

custom A
B {}
custom C
custom C
D {}

Expected / documented output:

custom A
custom A
custom C
custom C
custom C

Consistently applying the documented behaviour would go some way to remedying the breaking nature of this change, since at least user supplied .toString implementations would always be called, as was previously the case.

[BridgeAR]

@cpcallen thank you for the bug report! This is indeed not intended and it seems like we are missing a couple of test cases (so thank you very much for highlighting the different solutions).

I will have a look at it next week.

[Himself65]

It's okay on node v10.17.0 and the same problem on v13.1.0

[cpcallen]

It's okay on node v10.17.0 and the same problem on v13.1.0

Yes; as noted above it was introduced in v12.3.0.