Open
Bug 1378173
Opened 4 years ago
Updated 5 months ago
Remove Promise.jsm
Categories
(Toolkit :: Async Tooling, enhancement)
Toolkit
Async Tooling
Tracking
()
NEW
People
(Reporter: marco, Unassigned)
References
(Depends on 1 open bug, Blocks 1 open bug)
Details
It's now only used in mobile/*, devtools/*, addon-sdk/* and some tests. We should stop using it and remove it from the tree.
Comment 1•4 years ago
|
||
(In reply to Marco Castelluccio [:marco] from comment #0) > It's now only used in mobile/*, devtools/*, addon-sdk/* and some tests. > We should stop using it and remove it from the tree. This should wait until after 57, as removing it now would most likely break lots of add-ons.
Updated•4 years ago
|
Blocks: 856878, post-57-api-changes
Comment 2•4 years ago
|
||
Can we replace it with a stub for DOM promises in the mean time? That'd mean a lot less code to ship and load.
Comment 3•4 years ago
|
||
(In reply to Kris Maglione [:kmag] from comment #2) > Can we replace it with a stub for DOM promises in the mean time? That'd mean > a lot less code to ship and load. We would still have to load the module first, so I don't think it will provide a substantial benefit. Depending on how this is done it may even add more C++/JS boundary traversals.
Comment 4•4 years ago
|
||
(In reply to :Paolo Amadini from comment #3) > We would still have to load the module first, so I don't think it will > provide a substantial benefit. We'd still have a lot less code to load, though, especially since the bulk of the module code is currently loaded into the module scope via loadSubScript. > Depending on how this is done it may even add more C++/JS boundary > traversals. I doubt that, given how Promise.jsm stack saving and uncaught rejection handling works (and which mostly relies on XPConnect). But that doesn't especially worry me either way. WebIDL bindings are extremely fast.
Comment 5•4 years ago
|
||
I don't think that loading less code will necessarily make a difference, and we've already removed most imports of the module from the Firefox front-end. It seems to me that simply removing the remaining imports of Promise.jsm from mozilla-central would be a better use of our development time. I suspect that replacing the uncaught rejection detection system would require test changes as well. I'm not opposed to reviewing a patch that passes all tests, including support for the way the module is loaded in the Add-on SDK and the devtools debugger. However, it might be easier and more useful to remove the imports. If we wait until the Add-on SDK is removed from mozilla-central, it's also one less folder to worry about.
Comment 6•4 years ago
|
||
(In reply to :Paolo Amadini from comment #5) > I don't think that loading less code will necessarily make a difference, and > we've already removed most imports of the module from the Firefox front-end. > It seems to me that simply removing the remaining imports of Promise.jsm > from mozilla-central would be a better use of our development time. Every extra script we load during startup is measurably expensive. At least the script preloader saves us from a lot of the overhead of synchronously reading and decoding the bytecode, but the overhead is still there. And the memory and performance overhead of using the Promise.jsm code is not trivial either. I agree that we should focus on removing the in-tree imports, but I think that getting the Promise.jsm code out of the tree should be a priority, whether it happens before or after those removals. > I suspect that replacing the uncaught rejection detection system would > require test changes as well. I'm not opposed to reviewing a patch that > passes all tests, including support for the way the module is loaded in the > Add-on SDK and the devtools debugger. However, it might be easier and more > useful to remove the imports. DOM promises already have their own uncaught rejection handling, with their own tests, so I'm not especially worried about this. The devtools debugger is going to have to switch to using native promises, regardless, so that's a separate issue.
Comment 7•4 years ago
|
||
Promise.jsm is used quite a bit in comm-central (calendar, mailnews), so it there a replacement, or does C-C need to adopt it?
Comment 8•4 years ago
|
||
Code using "new Promise" can just use the native JavaScript promise implementation, while for Promise.defer() we have the equivalent PromiseUtils.defer(), although this module will eventually be removed too. See bug 1362882 and bug 1034599 for examples of the conversions we made in mozilla-central. You'll need to adopt Promise.jsm only if you want to support legacy add-ons that use it.
Comment 9•4 years ago
|
||
(In reply to Jorg K (GMT+2) from comment #7) > Promise.jsm is used quite a bit in comm-central (calendar, mailnews), so it > there a replacement, or does C-C need to adopt it? It doesn't seem to be used: http://searchfox.org/comm-central/search?q=Promise.defer&case=true But to actually answer your question: old style Promise.jsm promises were created using Promise.defer. We switched away from that using scripts in bug 1362882. For the few cases where we actually needed a deferred object, PromiseUtils.jsm can be used instead (that's what we did by hand in bug 1034599). If you never use Promise.defer, you can just remove the Promise.jsm imports, like I did in bug 1368456 (there's a script you can re-use). Unless you are worried about add-on compat of course...
Comment 10•4 years ago
|
||
Personally, I'm not so worried about add-on compatibility. IMHO, add-on authors should maintain add-ons and make adjustments where necessary.
> If you never use Promise.defer, you can just remove the Promise.jsm imports,
> like I did in bug 1368456
And that's it? Nothing else? I can do that ;-)
Comment 11•4 years ago
|
||
Some files are using PromiseUtils.jsm too. Is this also planned to remove or is this something different?
Flags: needinfo?(florian)
Comment 12•4 years ago
|
||
(In reply to Richard Marti (:Paenglab) from comment #11) > Some files are using PromiseUtils.jsm too. Is this also planned to remove or > is this something different? We have no plan to remove PromiseUtils.jsm in the near future.
Flags: needinfo?(florian)
You need to log in
before you can comment on or make changes to this bug.
Description
•