[meta] Switch devtools to async/await from task.js/yield
Categories
(DevTools :: General, enhancement, P5)
Tracking
(firefox91 fixed)
Tracking | Status | |
---|---|---|
firefox91 | --- | fixed |
People
(Reporter: tromey, Assigned: hexagonrecursion)
References
(Depends on 1 open bug, Blocks 1 open bug)
Details
(Keywords: meta)
Attachments
(5 files)
Like bug 1353542 did for other parts of the browser, it would be good to switch devtools from Task to real async+await. That bug has some scripts used for the conversion. Also, Florian offered to help a bit in https://bugzilla.mozilla.org/show_bug.cgi?id=1353542#c8
Comment 2•7 years ago
|
||
it might be better doing this at the same time as bug 1414177.
Here's a tweaked version of Florian's conversion script that I was able to run on RDM. To use: <obj-dir>/dist/bin/xpcshell <script> <directory to convert>
Also try running: To use: <obj-dir>/dist/bin/xpcshell <script> <directory to convert> --replace-generators as a second pass to replace non-task generators (which is usually correct, but may require more review).
Comment 6•6 years ago
|
||
Quick summary of Task.jsm refactoring. All devtools folders are now converted to async/await! All? No, almost all. I ignored gcli, old webconsole tests and old debugger. Which are planned to be dropped. And various tests that started failing when converted to async/await. They are all tracked in dependent bugs.
Updated•6 years ago
|
Comment 7•6 years ago
|
||
Note: As of bug 1446833 the remaining devtools tests added as generators are not being run (for instance, ./mach test devtools/client/memory/test/unit/test_action-toggle-inverted.js succeeds with only a single subtest). I discovered this while investigating bug 1451333 and thought you might want to know :)
Comment 8•6 years ago
|
||
Thanks for the heads up. I didn't particularly chased the "function*" occurences, we probably should as a second part of this meta bug. Also, there is a couple of untracked usages of Task still: * Docs still mention Task devtools/docs/contributing/javascript.md devtools/docs/tests/writing-tests.md * scratchpad uses it (I may have blacklisted it while doing the big automated patches): devtools/client/scratchpad/test/browser_scratchpad_autocomplete.js: .then(Task.async(runTests)) devtools/client/scratchpad/test/browser_scratchpad_close_toolbox.js: .then(Task.async(runTests)) devtools/client/scratchpad/test/browser_scratchpad_disable_view_menu_items.js: .then(Task.async(runTests)) devtools/client/scratchpad/test/browser_scratchpad_inspect_primitives.js: .then(Task.async(runTests)) * a couple of tests uses it in framework: devtools/client/framework/test/browser_toolbox_tool_remote_reopen.js devtools/client/framework/test/browser_target_support.js devtools/client/framework/test/browser_toolbox_tool_ready.js * this test only mention Task in a comment: devtools/client/framework/toolbox-highlighter-utils.js
Comment 9•6 years ago
|
||
(In reply to Alexandre Poirot [:ochameau] from comment #8) > Thanks for the heads up. > I didn't particularly chased the "function*" occurences, we probably should > as a second part of this meta bug. You should also look for 'yield' if you weren't already. Note that bug 1446833 only affected xpcshell tests, not mochitests.
Updated•6 years ago
|
Updated•6 years ago
|
Comment 10•6 years ago
|
||
The cleanup is almost finished. There is only one last usage in production code: https://searchfox.org/mozilla-central/source/devtools/client/shared/redux/middleware/task.js#25 This code should be easy to remove as no other code should be producing Task generators... They should all be async functions now. Otherwise, it is only tests, mostly old debugger tests and a couple of comments still referring to Tasks: https://searchfox.org/mozilla-central/search?q=%5CsTask%5C.&case=true®exp=true&path=devtools%2F Finally, we should be able to remove the task module: https://searchfox.org/mozilla-central/source/devtools/shared/task.js
Updated•5 years ago
|
Assignee | ||
Comment 12•3 years ago
|
||
Here is one more implementation of Task https://searchfox.org/mozilla-central/source/devtools/client/debugger/src/utils/task.js
I can not find any places where this code is used. Can this be removed?
Assignee | ||
Comment 13•3 years ago
|
||
(In reply to Alexandre Poirot [:ochameau] from comment #10)
https://searchfox.org/mozilla-central/source/devtools/client/shared/redux/
middleware/task.js#25
This code should be easy to remove as no other code should be producing
Task generators... They should all be async functions now.
Nope. There are still dispatch()
calls and asynchronous redux actions that depend on this middleware. Example:
https://searchfox.org/mozilla-central/rev/16d30bafd4e5276d6d3c632fb52a6c71e739cc44/devtools/client/memory/actions/filter.js#24
Comment 14•3 years ago
|
||
Thanks Andrey for looking into this!
(In reply to Andrey Bienkowski from comment #12)
Here is one more implementation of Task https://searchfox.org/mozilla-central/source/devtools/client/debugger/src/utils/task.js
I can not find any places where this code is used. Can this be removed?
I didn't know about this other one.
Same, I couldn't see any usage of it
(In reply to Andrey Bienkowski from comment #13)
(In reply to Alexandre Poirot [:ochameau] from comment #10)
https://searchfox.org/mozilla-central/source/devtools/client/shared/redux/
middleware/task.js#25
This code should be easy to remove as no other code should be producing
Task generators... They should all be async functions now.Nope. There are still
dispatch()
calls and asynchronous redux actions that depend on this middleware. Example:
https://searchfox.org/mozilla-central/rev/16d30bafd4e5276d6d3c632fb52a6c71e739cc44/devtools/client/memory/actions/filter.js#24
I thought I got rid of all the generators in some dependencies of this bug...
It looks like I was incomplete in my generator's refactoring.
Assignee | ||
Comment 15•3 years ago
|
||
For the purposes of giving credit where credit is due: I found out about https://searchfox.org/mozilla-central/rev/16d30bafd4e5276d6d3c632fb52a6c71e739cc44/devtools/client/memory/actions/filter.js#24 from https://bugzilla.mozilla.org/show_bug.cgi?id=1459567#c0
- Dispatch calls that use generators, such as https://searchfox.org/mozilla-central/source/devtools/client/memory/actions/filter.js#25
Assignee | ||
Comment 16•3 years ago
|
||
Updated•3 years ago
|
Assignee | ||
Comment 17•3 years ago
|
||
Depends on D117173
Assignee | ||
Comment 18•3 years ago
|
||
Depends on D117174
Assignee | ||
Comment 19•3 years ago
|
||
So long and thanks for all the fish!
Depends on D117175
Comment 20•3 years ago
|
||
Pushed by jdescottes@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/d0c52e87ef5d Convert to async/await in devtools/client/memory/actions/filter.js r=jdescottes https://hg.mozilla.org/integration/autoland/rev/292be5a99a63 Refactor devtools/client/memory/actions/filter.js r=jdescottes https://hg.mozilla.org/integration/autoland/rev/f6fafb901311 Stop using devtools/shared/task from the task middleware r=jdescottes https://hg.mozilla.org/integration/autoland/rev/f2e64b982c7c Delete devtools/shared/task.js r=jdescottes
Comment 21•3 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/d0c52e87ef5d
https://hg.mozilla.org/mozilla-central/rev/292be5a99a63
https://hg.mozilla.org/mozilla-central/rev/f6fafb901311
https://hg.mozilla.org/mozilla-central/rev/f2e64b982c7c
Description
•