Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Symbol.toStringTag dominates default q stringify #604

Merged
merged 2 commits into from
Mar 12, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 19 additions & 10 deletions packages/ses/src/error/stringify-utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,22 +58,31 @@ const bestEffortStringify = (payload, spaces = undefined) => {
return '[Seen]';
}
seenSet.add(val);
if (Promise.resolve(val) === val) {
return '[Promise]';
}
if (val instanceof Error) {
return `[${val.name}: ${val.message}]`;
}
if (Object.keys(val).length === 0 && Symbol.toStringTag in val) {
// Note that this test is `Object.keys` rather than `Refect.ownKeys`.
// Like `JSON.stringify`, `Object.ownKeys` will enumerate only
// string-named enumerable own properties, which will therefore
// omit Symbol.toStringTag even if it is own and enumerable.
// This case will happen to do a good job with presences without
if (Symbol.toStringTag in val) {
erights marked this conversation as resolved.
Show resolved Hide resolved
// For the built-ins that have or inherit a `Symbol.toStringTag`-named
// property, most of them inherit the default `toString` method,
// which will print in a similar manner: `"[object Foo]"` vs
// `"[Foo]"`. The exceptions are
// * `Symbol.prototype`, `BigInt.prototype`, `String.prototype`
// which don't matter to us since we handle primitives
// separately and we don't care about primitive wrapper objects.
// * TODO
// `Date.prototype`, `TypedArray.prototype`.
// Hmmm, we probably should make special cases for these. We're
// not using these yet, so it's not urgent. But others will run
// into these.
//
// Once #2018 is closed, the only objects in our code that have or
// inherit a `Symbol.toStringTag`-named property are remotables
// or their remote presences.
// This printing will do a good job for these without
// violating abstraction layering. This behavior makes sense
// purely in terms of JavaScript concepts. That's some of the
// motivation for choosing that representation of remotables
// in the first place.
// and their remote presences in the first place.
return `[${val[Symbol.toStringTag]}]`;
}
return val;
Expand Down
6 changes: 2 additions & 4 deletions packages/ses/test/error/test-assert-log.js
Original file line number Diff line number Diff line change
Expand Up @@ -400,7 +400,7 @@ test('q as best efforts stringify', t => {
];
t.is(
`${q(challenges)}`,
'["[Promise]","[Function foo]","[[hilbert]]","[undefined]","undefined","[URIError: wut?]",["[33n]","[Symbol(foo)]","[Symbol(bar)]","[Symbol(Symbol.asyncIterator)]"],{"NaN":"[NaN]","Infinity":"[Infinity]","neg":"[-Infinity]"},18014398509481984,{"superTagged":"[Tagged]","subTagged":"[Tagged]","subTaggedNonEmpty":{"foo":"x"}}]',
'["[Promise]","[Function foo]","[[hilbert]]","[undefined]","undefined","[URIError: wut?]",["[33n]","[Symbol(foo)]","[Symbol(bar)]","[Symbol(Symbol.asyncIterator)]"],{"NaN":"[NaN]","Infinity":"[Infinity]","neg":"[-Infinity]"},18014398509481984,{"superTagged":"[Tagged]","subTagged":"[Tagged]","subTaggedNonEmpty":"[Tagged]"}]',
);
t.is(
`${q(challenges, ' ')}`,
Expand All @@ -427,9 +427,7 @@ test('q as best efforts stringify', t => {
{
"superTagged": "[Tagged]",
"subTagged": "[Tagged]",
"subTaggedNonEmpty": {
"foo": "x"
}
"subTaggedNonEmpty": "[Tagged]"
}
]`,
);
Expand Down