util: allow styleText text param to accept non-string scalars#64452
util: allow styleText text param to accept non-string scalars#64452SparshGarg999 wants to merge 1 commit into
Conversation
Allow util.styleText() to accept
umber, �oolean, and �igint`nvalues for the ext parameter, automatically coercing them to string
via String(text).
Previously, passing anything other than a string would throw
ERR_INVALID_ARG_TYPE. This forced callers to wrap every non-string
value manually:
util.styleText('red', String(count)) // before
util.styleText('red', count) // after
The coercion is applied before the fast path so there is no extra
branching cost for the common string case. Values that cannot be
meaningfully stringified (null, undefined, Symbol, Function, Object)
still throw ERR_INVALID_ARG_TYPE as before.
Fixes: nodejs#63841
Signed-off-by: SparshGarg999 <sparshgarg999@gmail.com>
2e48257 to
e899ec4
Compare
bnb
left a comment
There was a problem hiding this comment.
I have a feeling this could end up being a bit of a foot gun, I do like the explicitness of having to String() anything that's not a string that I'm inputting into something specifically including text in the name. If I accidentally pass a number or boolean, I wouldn't want that to succeed silently.
Not opposed if there's other +1s, but I do feel like the current structure is a ~decent type safety gaurd.
|
It's also worth noting that TC39 decided to stop coercing arguments in all new APIs (modulo consistency concerns), so the language itself will be throwing more often in the future rather than silently accepting a coercible type. |
|
Thanks for the feedback! That makes sense—I hadn't considered the type-safety aspect or the recent TC39 direction around avoiding implicit coercion in new APIs. I can see how silently accepting numbers or booleans could hide bugs. In that case, I don't think automatic coercion is the right approach here. Would improving the error message (for example, suggesting String(value) when a scalar is passed) or adding an example to the documentation be a better direction instead? |
|
I wouldn't block this change based on my preference, but yes, I'd certainly prefer better error messages over implicit coercion :-) |
Summary
Allow
util.styleText()to acceptnumber,boolean, andbigintvalues for thetextparameter, automatically coercing them to a string internally usingString(text).Description
Currently, passing a non-string value (such as a number, boolean, or bigint) to
util.styleText()immediately throwsERR_INVALID_ARG_TYPE. This requires callers to manually cast non-string values before formatting them:This commit updates
util.styleTextto check the type oftextand automatically coerce scalar values (number, boolean, bigint) to strings. The coercion happens before the fast path checks to avoid adding extra branching overhead for the common string case.Non-string values that cannot be meaningfully coerced without potential issues (e.g.
null,undefined,Symbol,Function,Object) still throwERR_INVALID_ARG_TYPE.Verification
Existing tests in
test/parallel/test-util-styletext.jshave been updated to check for scalar coercion behavior, and to verify that incorrect non-string types still throw.Fixes: #63841