Use ES Classes for protocol.js Fronts

RESOLVED FIXED in Firefox 66

Status

enhancement
P2
normal
RESOLVED FIXED
9 months ago
3 months ago

People

(Reporter: ochameau, Assigned: ochameau)

Tracking

(Blocks 3 bugs)

unspecified
Firefox 66
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox66 fixed)

Details

Attachments

(14 attachments)

47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
https://github.com/devtools-html/rfcs/issues/56

Following this RFC, we should convert all Fronts to use ES classes and:
 - switch from using SDK's `extend` in protocol.js [1] to classes `extends` and mixins
 - convert all `initialize` method to `constructor`
 - for async front, still use an async initialize method returned by the constructor
 - Get rid of protocol.custom and use class inheritance
 - Get rid of protocol.preEvent and register "pre event" listeners via Front.before. This is still being discussed in the RFC.

This work is going to help merge all Target fronts with the base Target class (bug 1465635) by making class inheritance easier to use in fronts.

[1] https://searchfox.org/mozilla-central/rev/0859e6b10fb901875c80de8f8fc33cbb77b2505e/devtools/shared/protocol.js#1614-1626
Duplicate of this bug: 1393615
MozReview-Commit-ID: JegR2JsyNrI
This is based on:
  https://gist.github.com/ochameau/91457f1d8f887dcbdb6f3dbb72e517a3
It is imperfect and requires manual fixes in the following revisions.

MozReview-Commit-ID: 39reix536G4

Depends on D13780
The automated script left an eslint error that I fix here.

MozReview-Commit-ID: 7HJ89ji3cPB

Depends on D13781
The automated script already helped refactoring this, but I kept some manual work instead of spending too much time with regexp!

MozReview-Commit-ID: 6VOHJqjHwQT

Depends on D13782
Firefox doesn't yet support fields set on classes.
Move them to constructors by setting a property while executing the constructor.

MozReview-Commit-ID: CoWCDHsuUEw

Depends on D13783
async initialize are not being converted to constructor,
but at the same time it was matched by regexps and refactored to use super().

MozReview-Commit-ID: 7RCQQEkAZlZ

Depends on D13784
MozReview-Commit-ID: Z7hUItG4ml

Depends on D13785
The regexp stripped all objects commas in order to ease converting everything to classes,
but it also replaces objects that aren't Front and aren't converted to classes.

MozReview-Commit-ID: 5W8IJeFIzDM

Depends on D13786
css-properties.js doesn't export the front via export.MyFront = MyFront;
and so the regexp didn't matched and didn't added registerFront(MyFront) call.

MozReview-Commit-ID: AkAurVqeKXt

Depends on D13787
MozReview-Commit-ID: I8urhoXV3S2

Depends on D13788
Various front were having quite unique pattern that the regexp did not matched.

MozReview-Commit-ID: 61EAS3EHKUM

Depends on D13789
ES classes forces to use "new" to instantiate classes.

MozReview-Commit-ID: 3AZfpmParGN

Depends on D13790
I only apply the regexp script to devtools/shared/fronts,
so I manually refactor fronts defined outside of this folder in this changeset.

MozReview-Commit-ID: LUIgkTLCdkM

Depends on D13791
These are the main protocol.js tests. It is easier to rewrite them manually.

MozReview-Commit-ID: KKNL8PqODld

Depends on D13792
I'm most likely going to merge all these revisions into a single one before landing.
But I imagine having these splits is going to help the review?!
The very first revision changes protocol.js and is the most important one.
The last may be the second one to look into as it adapt protocol.js tests to the new API and give a good insight of the impact of these changes. Then, the one before "Adapt tests to new API" may be another good one to look at first as it adapt a couple of other tests. All the others should be considered as all-applied-at-once as they moslty fixes things that the automated rewrite failed rewriting correctly, or just did not even tried addressing.
Blocks: 1512153
Blocks: 1512154
Pushed by apoirot@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/286004dae904
Make protocol.js support es6 classes for fronts. r=yulia
https://hg.mozilla.org/mozilla-central/rev/286004dae904
Status: NEW → RESOLVED
Closed: 8 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 66
Blocks: 1515116
You need to log in before you can comment on or make changes to this bug.