Quote Symbol description when one is provided
source link: https://bugzilla.mozilla.org/show_bug.cgi?id=1695141
Go to the source link to view the article. You can view the picture content, updated content and better typesetting reading experience. If the link is broken, please click the button below to view the snapshot at that time.
Quote Symbol description when one is provided
Categories
(DevTools :: Shared Components, defect, P3)
Tracking
(firefox88 fixed)
88 Branch
People
(Reporter: nchevobbe, Assigned: rsawra)
Details
(Keywords: good-first-bug)
Steps to reproduce
- Open the console
- Evaluate
Symbol("hi")
Expected results
The result shows Symbol("hi")
Actual results
The result shows Symbol(hi)
Hi Nicolas,I would like to give it a try.
Can you please help me locate the code?
Hello Rahul, thank you for helping us :)
So I think this should be about setting this property to true
devtools/client/shared/components/reps/reps/symbol.js#47 , and then updating all the tests that are going to fail.
There are jest tests for Reps that can be run with
cd devtools/client/shared/components/test/node/
yarn
yarn test
We also have end to end test that might check Symbols too, like https://searchfox.org/mozilla-central/source/devtools/client/webconsole/test/browser/browser_webconsole_object_inspector_symbols.js (that you can run with ./mach test browser_webconsole_object_inspector_symbols.js
). Let me know if you have any question.
Hi Nicolas,I tried to reproduce the bug, but i got the actual result as Symbol(hi)
and not as Symbol("hi")
.I am using Nightly version.
So the actual result should be Symbol("hi")
and not Symbol(hi)
.
Hi Nicolas, after changing useQuotes:true
running yarn test
the following test failed:
components/reps/symbol.test.js
test Symbol with long string › renders the expected content
expect(received).toEqual(expected) // deep equality
Expected: "Symbol(aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa…)"
Received: "Symbol(\"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa…\")"
);
expect(renderedComponent.text()).toEqual(
| ^
"Symbol(aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa…)"
);
expectActorAttribute(renderedComponent, stub.actor); `
Could you please explain what is probably happening here?
Hi Nicolas,while debugging the error more i found that the value passed i.e name
is checked to be of type longString, you can see here https://searchfox.org/mozilla-central/rev/f1159268add2fd0959e9f91b474f5da74c90f305/devtools/client/shared/components/reps/reps/symbol.js#42, if name
is of type longString the output is correct ( for e.g you can test in console, type Symbol("aa".repeat(10000))
).
So i wanted to know why name.type === "longString"
is explicitly checked in if condition?
If i remove name.type
condition it works perfectly fine!
Hello Rahul,
It's expected that tests are failing, since we're changing how the Symbols look.
So here https://searchfox.org/mozilla-central/rev/6a023272d590409c80458d373986e379b3ad86f4/devtools/client/shared/components/test/node/components/reps/symbol.test.js#58 , we should change this to:
expect(renderedComponent.text()).toEqual(
`Symbol("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa…")`
);
(with the description wrapped in quotes)
Hi Nicolas, (In reply to RAHUL SAWRA[:rsawra] from comment #6)
Hi Nicolas,while debugging the error more i found that the value passed i.e
name
is checked to be of type longString, you can see here https://searchfox.org/mozilla-central/rev/f1159268add2fd0959e9f91b474f5da74c90f305/devtools/client/shared/components/reps/reps/symbol.js#42, ifname
is of type longString the output is correct ( for e.g you can test in console, typeSymbol("aa".repeat(10000))
).
So i wanted to know whyname.type === "longString"
is explicitly checked in if condition?
If i removename.type
condition it works perfectly fine!
Hi Nicolas, just changing useQuotes:true
doesn't seems to be working.
What are your thoughts on above comment where longString seems to be working fine, we just need to figure out for other Strings, you can checkout the jest test for other strings. https://searchfox.org/mozilla-central/rev/6a023272d590409c80458d373986e379b3ad86f4/devtools/client/shared/components/test/node/components/reps/symbol.test.js#27
(In reply to RAHUL SAWRA[:rsawra] from comment #6)
Hi Nicolas,while debugging the error more i found that the value passed i.e
name
is checked to be of type longString, you can see here https://searchfox.org/mozilla-central/rev/f1159268add2fd0959e9f91b474f5da74c90f305/devtools/client/shared/components/reps/reps/symbol.js#42, ifname
is of type longString the output is correct ( for e.g you can test in console, typeSymbol("aa".repeat(10000))
).
So i wanted to know whyname.type === "longString"
is explicitly checked in if condition?
If i removename.type
condition it works perfectly fine!
Ah sorry, I read your comment too quick.
Yes, I think we could only check if (name)
, so we would handle all the cases
Yes (In reply to Nicolas Chevobbe [:nchevobbe] from comment #9)
(In reply to RAHUL SAWRA[:rsawra] from comment #6)
Hi Nicolas,while debugging the error more i found that the value passed i.e
name
is checked to be of type longString, you can see here https://searchfox.org/mozilla-central/rev/f1159268add2fd0959e9f91b474f5da74c90f305/devtools/client/shared/components/reps/reps/symbol.js#42, ifname
is of type longString the output is correct ( for e.g you can test in console, typeSymbol("aa".repeat(10000))
).
So i wanted to know whyname.type === "longString"
is explicitly checked in if condition?
If i removename.type
condition it works perfectly fine!Ah sorry, I read your comment too quick.
Yes, I think we could only checkif (name)
, so we would handle all the cases
Yes, this should work. Thank you for your quick reply Nicolas :). I will update you soon.
Hi Nicolas,
it seems to be working fine now.
There are 2 jest tests failing at
Expected: "Object { x: 10, Symbol(): \"first unnamed symbol\", Symbol(): \"second unnamed symbol\", Symbol(named): \"named symbol\", Symbol(Symbol.iterator): () }"
Received: "Object { x: 10, Symbol(): \"first unnamed symbol\", Symbol(): \"second unnamed symbol\", Symbol(\"named\"): \"named symbol\", Symbol(\"Symbol.iterator\"): () }"
This is in reference to first test.Can you guide me as to how should i make these tests pass?
Hi Nicolas , did you get a chance to see the above tests?
Hello Rahul,
You need to update the test as we now do wrap the description in quotes (Symbol(named)
=> Symbol("named")
)
Seeing that, I think there are some name we don't want to wrap: Symbol.iterator
and Symbol.asyncIterator
I think that's because of this: https://searchfox.org/mozilla-central/source/devtools/client/shared/components/reps/reps/symbol.js#69
Now that the text is always a StringRep, we are trying to set the title
attribute with it. And since title
should be a string, the StringRep gets stringified (hence the [object Object]
part).
So for that part, we could try to call https://searchfox.org/mozilla-central/source/devtools/client/shared/components/reps/reps/rep-utils.js#129-141 with object.name
instead of trying to reuse symbolText
Hi Nicolas,
so i tried to call https://searchfox.org/mozilla-central/source/devtools/client/shared/components/reps/reps/symbol.js#63 with object.name
instead of symbolText
.
const config = getElementConfig(
{
shouldRenderTooltip,
className,
name,
},
object
);
So instead of reusing symbolText, i directly used name
, and this works fine,all the jest test cases are passing.
Recommend
About Joyk
Aggregate valuable and interesting links.
Joyk means Joy of geeK