1772157 - Add ObjectInspector/ObjectActor support for records and tuples

Status: NEW

(DevTools :: Shared Components, task)

Description

[Tim Chevalier]

I'm fixing bug 1765477 so that calling console.log() on a record or tuple doesn't cause the tab to crash. In the dev console, once my fix is applied, records and tuples will just print as "[record]" or "[tuple]" since previewing them properly is much more of an adventure (and might require adding Record and Tuple types to React; I don't understand the code well enough to know). But eventually, they should be previewable just as arrays and objects are.

Comment 1

[Tim Chevalier]

Noting some things I tried:

• In https://searchfox.org/mozilla-central/source/devtools/server/actors/object/utils.js#115, I tried adding a case like:

    case "tuple":
      return { type: "tuple", text: value.toString() }; 

and similar for records. This didn't work because the valid values of the "type" field are defined by the ReactPropTypes object ( https://searchfox.org/mozilla-central/source/devtools/client/shared/vendor/react-prop-types.js#145 ). Using the "any" type instead of "tuple" didn't work either (though this wouldn't be too useful even if it worked.)

• I tried re-using the case for "object" in the same code, but this resulted in the output InvisibleToDebugger { undefined }, possibly because records/tuples don't have a class property.

Comment 2

[Tim Chevalier]

Also, this case would work for tuples:

    case "tuple":
      return createStringGrip(pool, value.toString());

(It would be better to be able to expand the elements too, but this would at least be an improvement.) However, if I try to add an analogous case for records, the console crashes with: TypeError: can't access property "getGrip", terminalEagerResult is undefined. Executing #{'a': 1}.toString() in the REPL works fine. This should probably be tracked as a separate bug, but I don't have time to isolate the problem further right now.)

Comment 3

[Nicolas Chevobbe [:nchevobbe]]

Thanks for filing this Tim
I'll take care of it

Comment 4

[Nicolas Chevobbe [:nchevobbe]]

Working on this, I was doing some changes to makeDebuggeeValue to get a proper DebuggerObject

@@ -420,7 +424,7 @@ function getModifiersForEvent(rawObj) {
  *         Debuggee value for |value|.
  */
 function makeDebuggeeValue(targetActor, value) {
-  if (isObject(value)) {
+  if (isObject(value) || typeof value == "record" || typeof value == "tuple") {

But there's an issue when we try to get the global for a record.
It can easily be reproduced executing the following snippet in the Browser Toolbox console:

Cu.getGlobalForObject(#{hello: 1})

We're hitting https://searchfox.org/mozilla-central/rev/4c3f6e8bf87fffb7c62feb4c76a14e0eb0b94c1f/js/xpconnect/src/XPCComponents.cpp#1839-1841 , so I guess object.isPrimitive returns true for records, which I'm not sure is expected? (I'd assume not)

Comment 5

[Tim Chevalier]

object.isPrimitive should be true for records. Records are represented internally in SpiderMonkey as objects, but that should be behind the abstraction barrier for JS code. See the hasObjectPayload() predicate in https://searchfox.org/mozilla-central/source/js/public/Value.h#719 -- you will want to call that instead of isObject().

Comment 6

[Nicolas Chevobbe [:nchevobbe]]

Attachment: WIP: Bug 1772157 - [devtools] Add support for Records and Tuples.

Comment 7

[Tim Chevalier]

Attachment: WIP: Bug 1772157 - [devtools] Add support for Records and Tuples.