Closed
Bug 1024054
Opened 9 years ago
Closed 9 years ago
Create a previewer for ES6 Symbol
Categories
(DevTools :: Debugger, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
Firefox 34
People
(Reporter: fitzgen, Assigned: fitzgen)
References
(Blocks 1 open bug)
Details
Attachments
(3 files, 1 obsolete file)
We should have an object previewer for ES6 Symbols that we can use in the debugger and the webconsole. I suggest the following: Symbol Preview ------------------------------------- Symbol() Symbol() s = "foo"; Symbol(s) Symbol("foo")
Assignee | ||
Comment 2•9 years ago
|
||
In bug 1048054, :bz made the suggestion that for well-known (as defined by the spec) symbols, we use @@foo. For example: > Symbol.iterator @@iterator
Comment 3•9 years ago
|
||
(In reply to Nick Fitzgerald [:fitzgen] from comment #2) > In bug 1048054, :bz made the suggestion that for well-known (as defined by > the spec) symbols, we use @@foo. For example: > > > Symbol.iterator > @@iterator Why wouldn't this just use the [[Description]] field of the Symbol? https://people.mozilla.org/~jorendorff/es6-draft.html#sec-well-known-symbols
Assignee | ||
Comment 4•9 years ago
|
||
(In reply to Rick Waldron [:rwaldron] from comment #3) > (In reply to Nick Fitzgerald [:fitzgen] from comment #2) > > In bug 1048054, :bz made the suggestion that for well-known (as defined by > > the spec) symbols, we use @@foo. For example: > > > > > Symbol.iterator > > @@iterator > > > Why wouldn't this just use the [[Description]] field of the Symbol? > https://people.mozilla.org/~jorendorff/es6-draft.html#sec-well-known-symbols This sounds like a fine thing to do.
![]() |
||
Comment 5•9 years ago
|
||
Note that we also need changes to console.log to deal with symbols, presumably; right now trying to log a symbol totally fails, because it can't be stringified. That might want a separate bug.
Comment 6•9 years ago
|
||
I just tried out Chrome 38's Symbol in its dev tools to see how they are displaying the preview, I've attached an image. This isn't to say that there's is the right way, but I do think there is value in consistency. It would be nice if there were coordination between implementers for new things like this :)
Assignee | ||
Comment 7•9 years ago
|
||
(In reply to Rick Waldron [:rwaldron] from comment #6) > Created attachment 8470539 [details] > symbol-preview-chrome.png > > I just tried out Chrome 38's Symbol in its dev tools to see how they are > displaying the preview, I've attached an image. This isn't to say that > there's is the right way, but I do think there is value in consistency. It > would be nice if there were coordination between implementers for new things > like this :) Yeah all else being equal, and they're existing implementation being reasonable, we should just follow suit. Sometimes it doesn't make sense, but in this case, I'd argue its fine (and pretty similar to what we were discussing, anyways).
Assignee | ||
Comment 8•9 years ago
|
||
(In reply to On vacation Aug 5-18. Please do not request review. from comment #5) > Note that we also need changes to console.log to deal with symbols, > presumably; right now trying to log a symbol totally fails, because it can't > be stringified. That might want a separate bug. Previewers are shared between the console and debugger, so I don't think we should need another bug.
![]() |
||
Comment 9•9 years ago
|
||
> Previewers are shared between the console and debugger
Hmm. And structured cloning of a Symbol (from a worker, say) already works?
Assignee | ||
Comment 10•9 years ago
|
||
(In reply to Boris Zbarsky [:bz] from comment #9) > > Previewers are shared between the console and debugger > > Hmm. And structured cloning of a Symbol (from a worker, say) already works? Seems not: foo.html: <!DOCTYPE html> <body> <script src="../src/strace.js/strace.js"></script> <script> w = new Worker("foo.js") w.onmessage = console.log.bind(console); </script> </body> foo.js: self.postMessage(Symbol("foo")) Results in this in the console: TypeError: unsupported type for structured data foo.js:1 Note that the remote debugging protocol doesn't rely on structured cloning. Instead, it creates a JSON-able representation for all values (hence bug 881480, but I imagine knocking out both that and this at the same time makes sense).
Assignee | ||
Comment 11•9 years ago
|
||
(Ignore the strace.js stuff, that's left over from my last experiment that used foo.html and doesn't affect this test case.)
Comment 12•9 years ago
|
||
Knocking out structured cloning support for symbols is very close to good-first-bug territory, for what it's worth. I filed bug 1057699 for it, given the slight poke here.
Assignee | ||
Comment 13•9 years ago
|
||
Assignee: nobody → nfitzgerald
Assignee | ||
Comment 14•9 years ago
|
||
Pretty straight forward, but I haven't taken the time to write a test yet. Turns out that the form itself is good enough and we don't actually need a previewer, just the variables view strigifier.
Attachment #8477836 -
Flags: feedback?(past)
![]() |
||
Comment 15•9 years ago
|
||
> Seems not:
What I actually meant was what happens if you console.log(Symbol("foo")) in the worker?
Assignee | ||
Comment 16•9 years ago
|
||
(In reply to Boris Zbarsky [:bz] from comment #15) > > Seems not: > > What I actually meant was what happens if you console.log(Symbol("foo")) in > the worker? I'm not getting the console message, nor any errors in the web console, browser console, or on the worker.
![]() |
||
Comment 17•9 years ago
|
||
Well, right, right now; console.log suppresses any exceptions thrown while it's operating on its arguments. The point is we should make that work; it's possible that just making Symbols structured clonable will be enough, but we'll want to add a test.
Comment 18•9 years ago
|
||
Comment on attachment 8477836 [details] [diff] [review] symbol-variables-view.patch Review of attachment 8477836 [details] [diff] [review]: ----------------------------------------------------------------- Nice and simple.
Attachment #8477836 -
Flags: feedback?(past) → feedback+
Assignee | ||
Comment 19•9 years ago
|
||
Testing was more straightforward than I thought it'd be :) https://tbpl.mozilla.org/?tree=Try&rev=1b6adcbf7a98
Attachment #8477836 -
Attachment is obsolete: true
Attachment #8478441 -
Flags: review?(past)
Assignee | ||
Comment 20•9 years ago
|
||
(In reply to Boris Zbarsky [:bz] from comment #17) > Well, right, right now; console.log suppresses any exceptions thrown while > it's operating on its arguments. Which is great! Definitely the right thing to do. But maybe it should still log a warning so devs don't pull their hair out? > The point is we should make that work; > it's possible that just making Symbols structured clonable will be enough, > but we'll want to add a test. Right. Filed as a follow up: bug 1058130
Comment 21•9 years ago
|
||
Comment on attachment 8478441 [details] [diff] [review] symbol-variables-view.patch Review of attachment 8478441 [details] [diff] [review]: ----------------------------------------------------------------- Still good.
Attachment #8478441 -
Flags: review?(past) → review+
Assignee | ||
Updated•9 years ago
|
Keywords: checkin-needed
Comment 22•9 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/6ab8a358adb9
Keywords: checkin-needed
Whiteboard: [fixed-in-fx-team]
Comment 23•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/6ab8a358adb9
Status: NEW → RESOLVED
Closed: 9 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 34
Updated•5 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•