Closed Bug 1592277 Opened 2 years ago Closed 1 year ago

ES6 classes are displayed as functions

Categories

(DevTools :: Console, defect, P3)

Product:
DevTools
Component:
Console
Type:
defect

Tracking

(firefox75 verified)

Status:
VERIFIED FIXED
Milestone:
Firefox 75
Tracking Flags:
Tracking Status
firefox75 --- verified

People

(Reporter: nchevobbe, Assigned: simakin.eugene, Mentored)

References

Details

(Keywords: good-first-bug)

Attachments

(1 file)

Bug 1592277 - Support class constructors in devtools reps. r=nchevobbe.
47 bytes, text/x-phabricator-request
Details | Review

Steps to reproduce

  1. Open the console
  2. Evaluate the following:
class MyClass {};
MyClass;

Expected results

The result is class MyClass {}

Actual results

The result is function MyClass()

When Bug 1351635 lands, it should be about checking the isClassConstructor property on the grip in the Function rep

I've never contributed for dev-tools in the past and this one seems great to start.
Is anyone else working on it or I can tackle it?

Hello Katherine, thanks for offering help!

Nobody was working on it, so I assigned it to you :)

You can start by setting up the dev environment: http://docs.firefox-dev.tools/getting-started/
If you have any question, don't hesitate to join our Slack so we can chat.

Once you have everything set up, you can start working on this issue.
To give you a bit of background, we try to display all the variables using a shared set of functions that return React elements, and that we call Reps.

The one we are going to modify for this bug is probably https://searchfox.org/mozilla-central/rev/11d9c7b7fa82fdfb8ac2a8f0864e9d8d5fe2b926/devtools/client/debugger/packages/devtools-reps/src/reps/function.js.

You can see that the FunctionRep function takes an object in parameter, which has an object property (that we rename grip in the function).
This object is a representation of the original javascript function (and a class is a function).
In this object, we added not long ago a isClassConstructor field, that indicates if the function is a class or not.

So in the function, we should check for this property, and if it is true, return a different React element (one that would output class MyClass {}).

In order to debug the changes you do, you can use the Browser Toolbox: https://developer.mozilla.org/en-US/docs/Tools/Browser_Toolbox , where you will be able to see console.log you added, or set breakpoints in reps code.

One last thing to mention is that reps are bundled into one big file https://searchfox.org/mozilla-central/rev/35873cfc312a6285f54aa5e4ec2d4ab911157522/devtools/client/shared/components/reps/reps.js , which means each time you change the function.js file, you need to run cd devtools/client/debugger && node bin/bundle.js.

Let me know if you have any trouble, I will be happy to help you :)

Assignee: nobody → kate

Hi there!

Can I take the bug? Is Katherine working on it?

Thanks, Eugene

Hello Eugene,

Thank you for your patience (I was off for a bit)
I didn't hear back from Katherine, so I guess you can work on it :)

Let me know if you have any question.

Assignee: kate → simakin.eugene
Status: NEW → ASSIGNED

Hi guys,

Apologies for not hearing from me for that long. I was ill the entire time unable to work. :(

Thank you Eugene for taking the bug :)

(In reply to Nicolas Chevobbe [:nchevobbe] from comment #4)

Hello Eugene,

Thank you for your patience (I was off for a bit)
I didn't hear back from Katherine, so I guess you can work on it :)

Let me know if you have any question.

Hello Nicolas,

I submitted a patch. Please, review it.
Not sure if my implementation is a complete one but it solves the simple cases (see the tests).

Thanks,
Eugene

Attachment #9122142 - Attachment description: Bug 1592277 - Support class constructors in devtools reps → Bug 1592277 - Support class constructors in devtools reps. r=nchevobbe.

Hello Eugene, I'm not sure you saw that I reviewed your patch, hence this little message :)

Thanks, I missed that you reviewed the patch.

Done:

  • function rep behavior changed for the tiny mode
  • tests added for the tiny mode
  • lint errors should be fixed

Please, take a look!

Just noticed that some tests are failing (didn't get an email).

Fixing it...

Hello Eugene, do you need any help with this patch?

Flags: needinfo?(simakin.eugene)

Hi!
Yes, that would be great! I can't crack the mochitests yet...

Flags: needinfo?(simakin.eugene)
Pushed by nchevobbe@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/6cb4593d8d8a
Support class constructors in devtools reps. r=nchevobbe.

https://hg.mozilla.org/mozilla-central/rev/6cb4593d8d8a

Status: ASSIGNED → RESOLVED
Closed: 1 year ago
status-firefox75: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 75

Confirmed reported issue with 73.0b12 on Windows 10.
Verified with 75.0a1 (2020-02-24); on Kubuntu 19, Windows 10, macOS 10.13.
Patch reached 73.0.1 as well.

Status: RESOLVED → VERIFIED
status-firefox75: fixed → verified
You need to log in before you can comment on or make changes to this bug.