proposal-record-tuple icon indicating copy to clipboard operation
proposal-record-tuple copied to clipboard

Record toString: useful, useless or thrown(in implicit conversions) ?

Open ljharb opened this issue 5 years ago • 25 comments

per https://github.com/tc39/proposal-record-tuple/pull/135#discussion_r446541165

At the very least, I'd expect Records to have a Symbol.toStringTag of "Record", which would Object.prototype.toString.call(record) produce [object Record].

However, String(record), `${record}`, etc, according to #135, will produce "[record]". This doesn't seem particularly useful at all; if someone wants to know it's a record, they'll typeof it.

Objects have always had a useless toString, but since everything inherits from Object, it's a tough sell to come up with something broadly useful for it to do. Arrays' toString has problems, and could be much better if legacy didn't hold it back, but is still useful since it stringifies its contents. I would hope that Records can have a better user story around stringification than objects.

ljharb avatar Jun 28 '20 20:06 ljharb

I don't have a strong opinion on what String(record) should output, I think that a static string or a JSON-ified version seem reasonable to me.

On Symbol.toStringTag, If the prototype is null, then that won't be possible.

rickbutton avatar Jun 29 '20 16:06 rickbutton

It would be possible if all records had an own Symbol.toStringTag property.

ljharb avatar Jun 29 '20 17:06 ljharb

About @ljharb 's "at the very least" suggestion, I think it would be a reasonable starting point to make ToString(record) === Object.prototype.toString.call(record) === "[object Record]". I thought that might be confusing, since it's not an object, but probably JS programmers understand this more as a fixed pattern since Object.prototype.toString uses that for everything. We could do this even without making an own Symbol.toStringTag property, since Record wrappers could be checked for explicitly as other types are in lines 5-13 of Object.prototype.toString. It wouldn't be so crazy to make an own Symbol.toStringTag property, following module namespace exotic objects, but I also don't see the motivation.

On the question of whether ToString on Records and Tuples should be useful: I've sat on this some more, and I definitely see the developer experience benefit--that's the goal here, really. Ideally, we'd get another operation in JavaScript that's more like repr than str, but, well, we're working with what we have. So, if we wanted to make ToString on Records and Tuples useful, I'd suggest the following starting point for algorithm:

  • There's an enclosing #{/#[, a space after , and :, and double quotes around record keys
  • It's fully recursive, calling ToString on components until you get down to nothing
  • Symbols are allowed to be converted to strings when nested in Records and Tuples too

Thoughts? How should we make the consistency vs utility tradeoff? I was initially leaning towards "consistency" but now I feel myself leaning more towards "utility".

littledan avatar Jul 06 '20 21:07 littledan

Your suggestion seems like the best mix of both:

  1. Object.prototype.toString explicitly returns [object Record] when given a Record primitive
  2. I think there should be an own Symbol.toStringTag property (altho a Record.prototype would be a much more sensible place for this) because Object.prototype.toString.call(Object(record)) === Object.prototype.toString.call(record) should be true, like it is for every other non-nullish primitive
  3. Stringifying records with String(record) or the equivalent should do what everyone will expect anyways, your suggestion :-)

ljharb avatar Jul 06 '20 22:07 ljharb

because Object.prototype.toString.call(Object(record)) === Object.prototype.toString.call(record) should be true

I agree this is important to preserve. I'd suggest that the explicit case be for the [[RecordData]] internal slot, not for being a Record primitive. I think this will Just Work (TM) since Object.prototype.toString calls ToObject in step 3. So, I still think the Symbol.toStringTag is not necessary.

littledan avatar Jul 06 '20 22:07 littledan

There's an enclosing #{/#[, a space after , and :, and double quotes around record keys

+1, I was initially thinking that it would output JSON.parse-able {} but including the hash makes it more obvious that it is the "string representation" and not "the json version".

This would also generate a single line of "string representation". Does a better default format make sense, i.e. newlines and indenting object keys?

rickbutton avatar Jul 07 '20 22:07 rickbutton

I want to suggest we start by keeping it simple; prettyprinting gets very complicated to do well. Also note that my algorithm would still leave off the n on BigInts. I would leave fancier printing to DevTools or analogous libraries; this is just a default starting point for visibility.

littledan avatar Jul 07 '20 23:07 littledan

let obj = { __proto__: null, foo: 'bar' }; let rec = #{ foo: 'bar' };

assert(obj.__proto__ === null) assert(rec.__proto__ === null);

String(obj) -> Uncaught TypeError: can't convert obj to string

therefore it makes sense that

String(rec) -> Uncaught TypeError: can't convert rec to string

It's a really annoying quirk of JS that there's random toString implementations alll over the place. Let's avoid making this mistake for future additions.

celmaun avatar Sep 24 '21 03:09 celmaun

@Salmatron "annoying" is subjective, and I don't think that's universally agreed. It's actually quite annoying to me that null objects throw in a string context - to me, that is the mistake.

ljharb avatar Sep 24 '21 04:09 ljharb

Subjective, yes. I am of the opinion that developer mistakes should be pointed out early rather than swept under the rug. All objects without an explicit toString() implementation should throw IMO. How is '[object Object]' useful in any way? All that does is mask actual bugs. Putting a non-toString() type into a string context is a mistake that should be caught. (and Symbol.toStringTag should be read the proper way).

Like let obj2 = {}; obj2[obj] = 123 -> { '[object Object]': 123 } // <- Novices would appreciate being pointed to ES Map instead of being confused

Then there's array.sort() which casts everything to string. If Record has an expensive toString() method that dumps some JSON-like representation, that's gonna make things crawl on arrays with large data structures.

celmaun avatar Sep 24 '21 04:09 celmaun

What would the behavior of String(box) be? Following the current path, I guess it could result in:

`Box(${String(content)})`

What if the object contained in a Box has a custom toString which attempts to call String(containingBox) again, aka creates a cycle (possibly through a record or tuple.

const obj = {
  toString() {
    return `{${Object.entries.map(([key, value]) => `${key}: ${String(value)}`).join(', ')}}`;
  }
};
obj.box = Box(obj);

String(obj); // ?

mhofman avatar Sep 30 '21 02:09 mhofman

Same thing a toString would do that has an infinite loop - halt the program.

ljharb avatar Sep 30 '21 02:09 ljharb

What would the behavior of String(box) be?

If we want to constrain access to unbox then potentially no other built-in should use that operation. JSON.stringify with a custom replacer would effectively be another way to unbox. So maybe for consistency String also shouldn’t be an implicit call to unbox? Though it does not appear to be the same level of risk if it did.

acutmore avatar Sep 30 '21 04:09 acutmore

If we want to constrain access to unbox then potentially no other built-in should use that operation.

That's a very good point I totally forgot about, and probably the biggest drawback to allowing virtualization through Box.unbox instead of leaving it on the prototype.

mhofman avatar Sep 30 '21 14:09 mhofman

We are considering the option of throwing here for implicit conversions. This would have the effect of solving #289 by throwing on operators.

If passing explicitely to the String constructor, we would prefer a useless string as anyone passing to that constructor could also pass it to JSON.stringify where they would have more control over the conversion.

What do you think @ljharb?

rricard avatar Jul 08 '22 10:07 rricard

I would hope that Records can have a better user story around stringification than objects.

Why wouldn’t toString just defer to JSON.stringify, if toJSON ensures that JSON.stringify prints something useful?

ljharb avatar Jul 08 '22 14:07 ljharb

Why wouldn’t toString just defer to JSON.stringify, if toJSON ensures that JSON.stringify prints something useful?

Tuples follow Arrays behavior for toString and JSON.stringify (following the general design theme of basing Tuple methods on Arrays).

String( [1,  [ [ [2]]], 3]); // '1,2,3'
String(#[1, #[#[#[2]]], 3]); // '1,2,3'
JSON.stringify( [1,  [ [ [2]]], 3]); // '[1,[[[2]]],3]'
JSON.stringify(#[1, #[#[#[2]]], 3]); // '[1,[[[2]]],3]'

It is potentially odd that a Tuple would serialize differently when passed to String directly compared to when its nested in a Record.

String(#[1, #[#{ t: 2 }]]); // `1,{"t": 2}`
String(#{ t: #[#[2]] }); // `{"t":[[2]]}`

acutmore avatar Jul 08 '22 16:07 acutmore

The usefulness is for debugging, and i don't think list-likes and dictionary-likes need to be consistent with each other.

ljharb avatar Jul 08 '22 16:07 ljharb

Summary post for reference


Context

Without explicit handling either by the application, or in the spec itself, treating a record as a string can't produce a useful result - in fact right now implicit conversion will throw an error. This is because Records do not have any of the methods that are used in the string conversion routine: toString, valueOf, and Symbol.toPrimitive and they can't do because they do not have a prototype to put these on.

const rec = #{ prop: 42 };
rec.toString === undefined;
Object.prototype.toString.call(rec); // `[object Record]`
String(rec); // [object Record]
JSON.stringify(rec); // `{"prop":42}`

const msg = `my record is: ${rec}`; // throws TypeError
// instead of `my record is: [object Record]`

Returning "[object Record]" is consistent with how objects behave by default in JavaScript. If objects want a custom string representation they need to explicitly implement toString.

const obj = { prop: 42 };
const set = new Set([1, 2, 3]);

console.log(`obj: ${obj} set: ${set}`); // "obj: [object Object] set: [object Set]"

For this reason, when logging, it can be preferable to leave the stringification of values to the platform/library rather than performed at the individual call sites:

$ node -e "console.log('logging:', { prop: 42 })"
logging: { prop: 42 }

We expect a similar expierence for Records, so when passed directly to debugging mechanisms a rich representation is produced.

Throwing

We hypothesize that implicit conversion of a record to a string is a programming error based on the fact that the string would display no 'helpful' information about the record's contents. Similar to when someone accidentally gets "[object Object]". This is why it throws an error, to help catch this.

There is some precedent for throwing on implicit string conversion of a primitive as this is what happens with symbol.

If we imagine an application that captures logs during execution, perhaps it is a webserver. An issue is reported, when the developers go to check the logs around the time of the issue they see: 2022-07-12T08:09:38.747Z I: data: [object Record] and only then realise that they have not been logging useful information. Instead what will happen is they would have immediately had an error produced pointing out the accidental implicit string conversion.

A counter-argument to this is if the application only logs when there is an error, the log is in a catch block for example. In this situation the accidental implicit string conversion could go unnoticed until the error happens. The implicit string conversion error could then mask the original error. Note: This is a problem in general with catch blocks, the catch handler should ideally be resilient to this category of problem.

A safer catch block
function safeHandler(originalError, handler) {
    try {
        return handler(originalError);
    } catch (handlerError) {
        if (handlerError === originalError) {
            throw originalError;
        } else {
            throw new AggregateError([originalError, handlerError]);
        }
    }
}

try {
    ...
} catch (err) {
    // the safeHandler ensures we don't lose `err` when our logging goes wrong
    safeHandler(err, () => {
        // whoops, this will throw when the record is converted to a string
        console.log(`error during processing of record: ${record}`);
    });
}

A useful string

Instead of producing "[object Record]" the spec could include logic to produce a detailed string based on the contents of the record.

// Hypothetical:
const rec = #{ foo: #{bar: 42} };
`rec is ${rec}`; // `rec is #{"foo": #{"bar": 42}}`

This is similar to languages like Python and Java

from collections import namedtuple

Point = namedtuple('Point', 'x y')
pt1 = Point(1.0, 5.0)

print(f'pt1 is: {pt1}') # "pt1 is: Point(x=1.0, y=5.0)"

# String interpolation is different to string concatenation in Python:
'pt1 is: ' + pt1 # TypeError: can only concatenate str (not "Point") to str
public class Main {
  private record Point(float x, float y) { }

  public static void main(String args[]) {
    Point p = new Point(1.0f, 5.0f);

    String message = String.format("p is %s", p);
    System.out.println(message); // "p is Point[x=1.0, y=5.0]"
  }
}

The easiest (least amount of spec text) would be to reuse JSON.stringify. However this has downsides, not all primitives can be directly reproduced in JSON, specifically undefined, symbol, bigint, NaN. These values are either converted to null, skipped or throw. While these could be worked around, e.g. displaying bigint as a string this could cause confusion and/or errors where someone is unaware of these silent conversions and is using the output as a JSON parsable input to something else. It would be clearer if the string was immediately clear that it is not JSON so that it is very unlikely to be mistaken for JSON. This can be achieved by retaining the #{ and #[ opening brackets or records and tuples.

// Hypothetical JSON-inspired but bespoke string representation, that recursively stringifies its elements
const rec = #{ foo: #[undefined, NaN, Symbol("s"), 1n] };
`rec is ${rec}`; // `rec is #{"foo": #[undefined, NaN, Symbol(s), 1]}`

The downsides to this are:

  • No reliable built-in way to parse this back to the original record.
    • e.g. The string of a bigint does not include the n suffix and thus would be indistinguishable from a number
    • Identity would not always be preserved (if there are symbols), or records containing symbols could continue throwing because symbols throw when implicitly stringified
  • Code may couple itself to this string format
    • this prevents it from evolving
  • Large subjective design space (should it have indentation? trailing commas? only keys and no values?).
  • The only limit on how large this string could become is the string limits themselves
    • Unless an arbitrary limit (e.g. on recursion depth) is also part of the spec
  • Not possible to customise when defining the record.
    • in Python and Java there are __str__ and toString methods that can be implemented
  • Not possible to configure globally.
    • e.g. Record.setStringFormatting(indentation) would introduce global state, clashes and a communications channel
  • Tuples inside Records get special treatment that differs from how they would usually toString
    • Tuples follow Array behavior of using join(",") when converted to a string
    • Though this is the same as explicit JSON.stringify(arr) vs arr.toString()

Conclusion

While producing a "useful" string was an interesting thought experiment, the champion group's position is to retain consistency with default object behavior of returning the static string "[object Record]" for explicit conversion, and to help avoid mistakes by throwing for implicit conversion. Leaving the stringification to userland provides more flexibility and time&space to evolve.

acutmore avatar Jul 14 '22 13:07 acutmore

I don't think that consistency should be sought. If Objects hadn't made the mistake of being everything's base class, I think Object.prototype.toString would do exactly what we're asking for here. Object is special; things shouldn't strive to be consistent with it.

ljharb avatar Jul 14 '22 16:07 ljharb

I think this precedent is more recent than Object.prototype.toString. There could have been a Map.prototype.toString which produced a detailed representation similar to Date.prototype.toString. I’m not sure if that was discussed during ES6.

acutmore avatar Jul 14 '22 18:07 acutmore

Map's purpose is to have object keys, and objects don't have a useful string serialization for the reasons discussed, so I think it's a consequence of the same precedent.

Records can't have objects, so they're free of that constraint.

ljharb avatar Jul 14 '22 18:07 ljharb

Thinking purely as a user:

  1. Records are basically just Tuples where the keys can be arbitrary strings instead of integer indices. In particular, we know that keys won't be Symbols and values won't be Objects, so that screams "easy to stringify!"
  2. Tuple stringification throws on Symbol values while happily dealing with all other primitives. Since the value space for Records is exactly the same, I am led to expect that Record stringification will do similarly.
  3. However, we immediately have two problems due to Tuple's hard-alignment with Array: #[undefined,null,3] stringifies as ,,3, yet #{ x: undefined, y: null, z: 3 } cannot reasonably omit braces or use '' for undefined and null.

As such, I believe it is inappropriate to view this lopsidedly as an issue with Record and not with Tuple.

  • If Tuple hard-aligns with Array, then Record probably should hard-align with Object. (This would at least avoid the nasty situation where a Record value prevents a Tuple from being stringified.)
  • If we want Records to stringify nicely (which I do believe users would prefer!), then Tuple should not simply defer to Array.prototype.join. We should be designing a coherent solution for this proposal as a whole.

rkirsling avatar Aug 10 '22 21:08 rkirsling

Thanks @rkirsling. How much of a risk do you think there would be if Tuple.prototype.toString diverged from Array.prototype.toString? For example when refactoring some code from arrays to tuples.

acutmore avatar Aug 11 '22 06:08 acutmore

I would hope nobody is relying on array toString when join exists - and i doubt that anyone doing such a refactor would miss the difference.

ljharb avatar Aug 11 '22 07:08 ljharb

I agree with what @ljharb said, but moreover, I feel like the only reason we'd be aligning with Array to begin with is because its behavior is "just good enough" to accept. But it too has clear room for improvement, and if Record and Tuple are in clear alignment on a "better" solution, I think the divergence from Array would be enthusiastically forgiven.

rkirsling avatar Aug 11 '22 17:08 rkirsling

I agree that returning "[object Record]" is not useful and that throwing on implicit conversion to string would be better, so that mistakes can be found early.

However, in the interest of developer ergonomics, I suggest a new method: Record.stringify(value[, replacer[, options]]). It is not called toString to signal that this method is not called on implicit conversion, but explicitly by the developer.

While it doesn't avoid all of the downsides in acutmore's comment, since it's not called implicitly these are IMHO less dangerous.

This also opens up the opportunity for adding a Record.parse method if/when a reliable format can be determined. Until such time, the format produced by Record.stringify should be restricted enough to allow for reliable parsing in the future. Unless it can be fully worked out in this proposal. Alternatively, it could be decided right now that the format produced by Record.stringify is never meant to be parsable, and have it be (largely) implementation-defined. In that case, it should probably have a different name in order to avoid expected symmetry JSON.stringify & JSON.parse.

Naturally, I would make the same suggestion for a Tuple.stringify.

In any case, having a built-in way to represent Records (and Tuples) as strings is more useful than consistency in the form of "[object Record]". But maybe it doesn't have to be by way of an implicitly called toString.

kninnug avatar Aug 19 '22 14:08 kninnug

I do see some value in having a nicer toString behavior for Records and Tuples, but it becomes a bit hard to limit the scope of such a project. In prettyprinters for data structures in several languages, there are mechanisms to limit the depth and the length traversed, as well as possibly control whitespace. This is exactly what console.log gives you. Here, we need something much more simple and minimal, but a useful toString for records opens up exactly this can of worms.

littledan avatar Aug 22 '22 10:08 littledan

PR #354 opened so implicit toString conversion of a Record/Tuple to a string gives a useful string representation.

One small thing we would lose with this is that it's no longer an error if someone attempts to use a Record as an object property key. There is the potential to add a check to toPropertyKey that would throw if the type is record or tuple. So that would be one place where implicit conversion would still not be allowed. What do people think?

acutmore avatar Sep 05 '22 09:09 acutmore

I think a ToPropertyKey check would be reasonable; since my overarching feeling is that Record and Tuple should have similar behavior, it would also make sense not to have a tuple property key just work.

rkirsling avatar Sep 06 '22 21:09 rkirsling