-
Notifications
You must be signed in to change notification settings - Fork 286
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 attrs/args - toString is undefined #1189
Conversation
getClassName already handles this case
@patricklx seems like we had a failing test. I restarted them in case it was a fluke, but may need to look into it. |
@@ -712,7 +712,7 @@ module('Ember Debug - Object Inspector', function(hooks) { | |||
|
|||
let message = await inspectObject(proxy); | |||
|
|||
assert.ok(message.name.includes('(unknown)')); | |||
assert.ok(message.name.includes('ObjectProxy'), 'object name should start with <ObjectProxy:'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@patricklx Is this the intended output? I suppose it is better than unknown
, but just want to make sure things are working as intended.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, since getClassName replaces that part with the class name. But i think it only does it dev builds. in prod it will show unknown again
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there not a way to show the same things in both?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no, since the the class name comes from constructor.name, which will show the uglified value in production
* fix attrs/args - toString is undefined getClassName already handles this case * object name is now from toString * Update object-inspector-test.js
getClassName already handles this case