Closed Bug 1368939 Opened 3 years ago Closed 2 years ago

[meta] Stop using sdk/core/heritage in DevTools

Categories

(DevTools :: General, enhancement, P3)

enhancement

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: pbro, Unassigned)

References

Details

(Keywords: meta, Whiteboard: [nosdk])

In bug 1350645 we're trying to get rid of our dependencies on the soon-to-be-removed addon-on SDK APIs.
One such API is sdk/core/heritage which we use in ~50 places:

http://searchfox.org/mozilla-central/search?q=sdk%2Fcore%2Fheritage&path=devtools

We should replace this with ES classes now that they are supported.
Blocks: 1350645
DevTools bug triage (filter on CLIMBING SHOES).
Priority: -- → P2
Some additional information:

1. We should probably ignore the projectEditor since is going away (Bug 1361311), that's bring down to 25 the place where `heritage` is used (view-helpers.js mentioned heritage module, but without actually requiring it).

2. In our code base we have actually two ways to use `heritage` module: one imports `Class`; and one imports `extend`.

The usage of `Class` should be probably replaced by ES6 classes, and where is possible `extend` as well. However, if there is any performance issue in some hot code of using ES6 classes, since they're not JITted (bug 1167472), our most common usage of `extend` is basically this:

   Object.create(prototype, Object.getOwnPropertyDescriptor(properties));

(Actually `extend` can takes more than on property descriptor objects, but I think we don't have this scenario in our code base).

So we could just replace:

    const { extend } = require("sdk/core/heritage");

With:

    const extend = (prototype, properties) =>   Object.create(prototype, Object.getOwnPropertyDescriptor(properties));

For the time being.
(In reply to Matteo Ferretti [:zer0] [:matteo] from comment #2)

> our most common usage of `extend` is basically this:
> 
>    Object.create(prototype, Object.getOwnPropertyDescriptor(properties));
>
> So we could just replace:
> 
>     const { extend } = require("sdk/core/heritage");
> 
> With:
> 
>     const extend = (prototype, properties) =>   Object.create(prototype,
> Object.getOwnPropertyDescriptor(properties));

I'm noticing just now the typo; the actual code is with "getOwnPropertyDescriptors" (with "s"); adding for future reference.
Flags: qe-verify-
Whiteboard: [nosdk]
Keywords: meta
Priority: P2 → --
Summary: Stop using sdk/core/heritage in DevTools → `[meta] Stop using sdk/core/heritage in DevTools
Priority: -- → P3
Summary: `[meta] Stop using sdk/core/heritage in DevTools → [meta] Stop using sdk/core/heritage in DevTools
No longer depends on: 1392635
Given that no dependencies are left, and that the only appearances of sdk/core/heritage on the codebase are in comments, I declare this bug CLOSED! (hopefully)
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.