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

Fix object formatting #745

Merged
merged 1 commit into from
Apr 15, 2022
Merged

Fix object formatting #745

merged 1 commit into from
Apr 15, 2022

Conversation

wbamberg
Copy link
Collaborator

@wbamberg wbamberg commented Apr 1, 2022

I think this is a fix for #710.

I've confirmed that the issue there is a regression introduced in #578. That PR was a fix for #574, which was an error that happened with certain weird objects that don't have a constructor: #574 (comment).

The fix was in formatObject(), which is a function in the mock console that takes an object to be logged and tests to see if it's any of various types that want special-case formatting. If the object isn't one of these types, then formatObject() just returns the original object.

The resolution for #574 was to add another special case for weird objects that don't have a constructor or a prototype, (not sure why the non-prototype bit is important) that formats these weird objects.

However, it looks like the condition to check that had an error:

if (!objectName.constructor || !objectName.prototype) {

First because this is || it will be true if either condition is true, which isn't what we want (at least according to the description in the issue). But also, I think objectName isn't necessarily the object, it's the result of calling:

var objectName = input.constructor ? input.constructor.name : input;

So in the case of the const example, input is a TypeError object, so objectName is a string "TypeError". And then it doesn't have a prototype, so we run that special-case code, which gives us "Object { }".

What we want to happen here, I think, and what used to happen, is to skip all the special cases in this function and return input.

I'm guessing this will happen not only for TypeError but for many other types that didn't match any of the other special cases, and they will just fall through into this one.

So, I've tightened up that conditional so it's only going to catch these weird types.

I have checked that this fixes the const case and the others listed in #710 (comment), and doesn't regress #574, and have tried a few other examples. But, this is tricky code as we've seen. I don't know if we have good tests for it, but it certainly needs careful review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Indicates an unexpected problem or unintended behavior. community Contributions by the community
Projects
Development

Successfully merging this pull request may close these issues.

3 participants