improv: display String objects similar to node

[Pranav2612000]

What does this PR do?

This commit adds special condition to print String Objects as [String "<string>"] in green. This is the same as node.
This fixes #17663

• Code changes

How did you verify your code works?

Ran a script locally to ensure that the output is similar to Node

• I included a test for the new code, or an existing test covers it

[190n]

See comment on console-log.js

[190n]

This prints differently than Node.js if the string contains characters which need to be escaped:

// log.js
console.log(new String("\n"));
console.log(new String("'"));
console.log(new String("'\""));
console.log(new String("'\"`"));

$ node log.js
[String: '\n']
[String: "'"]
[String: `'"`]
[String: '\'"`']
$ bun-debug log.js
[String: '
']
[String: ''']
[String: ''"']
[String: ''"`']

I think it's acceptable, at least for now, to not exactly match Node's decision for which style of quote to use. But we should try to ensure that the thing after String: is a valid JS string literal. A decent compromise might be, if the string contains any characters that need to be escaped, to use similar logic to the if (this.quote_strings case on line 2114 which prints it as a JSON string (so it will always use double quotes).

[190n]

Actually, it seems like Bun's util.inspect code already handles these edge cases escaping new String correctly, so maybe you could rely on the implementation there?

[Pranav2612000]

Actually, it seems like Bun's util.inspect code already handles these edge cases escaping new String correctly, so maybe you could rely on the implementation there?

Do you mean this function

bun/src/js/internal/util/inspect.js

Line 890 in 11979f6

function strEscape(str) {

?

I'm guessing just the quotes part would be enough. Right? Or do we also need to do all the regex replaces ( example )

[190n]

On reflection I think it will be much easier to use the JSON format case that I first mentioned:

A decent compromise might be, if the string contains any characters that need to be escaped, to use similar logic to the if (this.quote_strings case on line 2114 which prints it as a JSON string (so it will always use double quotes).

rather than call into, or duplicate the logic of, a bunch of JavaScript code. This won't match Node.js perfectly but it'll be better than status quo. I do think the most important part is just that the thing after String: is any kind of valid string literal, which it isn't always in the current state of your PR but it will be if you detect when escaping is needed and do a JSON-style escape with double quotes.

[190n]

btw, set environment variable BUN_DEBUG_ALL=0 to make your local build less spammy.

[Pranav2612000]

@190n Is there anything else I can do to make this PR merge ready?

[190n]

Sorry, you don't need to do anything. The test failures aren't related to your changes but because of them I don't have permission to merge this. I've asked if someone else on the team can.

…

On Wed, Mar 5, 2025, 8:00 PM Pranav Joglekar ***@***.***> wrote: @190n <https://github.com/190n> Is there anything else I can do to make this PR merge ready? — Reply to this email directly, view it on GitHub <#17761 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AB3HNDONGPA4IAAS74HEKD32S7B6HAVCNFSM6AAAAABX7V5A2CVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDOMBSG4ZTMNJRG4> . You are receiving this because you were mentioned.Message ID: ***@***.***> [image: Pranav2612000]*Pranav2612000* left a comment (oven-sh/bun#17761) <#17761 (comment)> @190n <https://github.com/190n> Is there anything else I can do to make this PR merge ready? — Reply to this email directly, view it on GitHub <#17761 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AB3HNDONGPA4IAAS74HEKD32S7B6HAVCNFSM6AAAAABX7V5A2CVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDOMBSG4ZTMNJRG4> . You are receiving this because you were mentioned.Message ID: ***@***.***>