Remove flow types from the debugger
Categories
(DevTools :: Debugger, task, P3)
Tracking
(firefox88 fixed)
| Tracking | Status | |
|---|---|---|
| firefox88 | --- | fixed |
People
(Reporter: ochameau, Assigned: ochameau)
References
(Blocks 2 open bugs)
Details
Attachments
(3 files, 7 obsolete files)
Debugger panel is the only panel to use flow and this require:
- a build step, which:
- require to run
./mach build fasterbefore seeing the changes applied - doesn't support source map and makes it hard to understand stacktraces as locations aren't matching sources
- require to run
- a couple of non-trivial workarounds in order to address flow type system edge cases. Example
stringToSourceActorId:
https://searchfox.org/mozilla-central/source/devtools/client/debugger/src/reducers/source-actors.js#176-182 - using yarn && yarn flow from debugger folder. Ideally, we should be able to work on any panel only by using ./mach commands.
|
Assignee | |
Updated•4 years ago
|
|
||
Comment 1•4 years ago
|
||
I'm 100% in favor of removing flow.
(continuing the conversation on slack about losing a safety net when removing flow)
From what I remember, when we discussed debugger cleanups, we pushed back about removing the jest tests, because we were afraid we might lose coverage. I don't remember we had this argument about flow though.
Every time flow breaks, I update the types as quickly as I can to make it pass. It never "caught" any regression or issue for me.
I don't feel like flow brings much here. It's not well integrated in our tool chain and it's inconsistent with the rest of the code base.
|
||
Updated•4 years ago
|
|
|
Assignee | |
Comment 4•4 years ago
|
||
Green try:
https://treeherder.mozilla.org/jobs?repo=try&revision=864bcfd77aa42278b2c3b045dd3a41e964496f19
|
|
Assignee | |
Comment 5•4 years ago
|
||
This is an automated change, done via:
$ yarn global add flow-remove-type
$ ~/.yarn/bin/flow-remove-types devtools/client/debugger/src/ --out-dir devtools/client/debugger/src2
$ cp -r devtools/client/debugger/src2/* devtools/client/debugger/src/
|
|
Assignee | |
Comment 7•4 years ago
|
||
./mach eslint wasn't able to auto-address all issues.
I'm going over all of them here.
|
|
Assignee | |
Comment 8•4 years ago
|
||
Remove references to flow in build files.
This effectively stop stipping flow from debugger modules.
So any leftover would now start being kept in production file and do an error.
|
|
Assignee | |
Comment 9•4 years ago
|
||
Some manual fixes are necessary.
Especially lots of attributes being types in the beginning of classes,
which ended up overloading the real implementation of attributes/functions.
|
|
Assignee | |
Comment 10•4 years ago
|
||
|
||
Updated•4 years ago
|
|
|
Assignee | |
Comment 11•4 years ago
|
||
|
|
Assignee | |
Comment 12•4 years ago
|
||
|
|
Assignee | |
Comment 13•4 years ago
|
||
|
|
Assignee | |
Comment 14•4 years ago
|
||
|
|
Assignee | |
Comment 15•4 years ago
|
||
|
|
||
Updated•4 years ago
|
|
||
Comment 16•4 years ago
|
||
Pushed by apoirot@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/552023cfd750 [devtools] - Strip flow from the debugger sources. r=nchevobbe,bomsy https://hg.mozilla.org/integration/autoland/rev/3a94715fdff0 [devtools] - Remove eslint settings about flow. r=nchevobbe,bomsy https://hg.mozilla.org/integration/autoland/rev/828c62bfc0a2 [devtools] - Remove flow plugin and stop running flow on try r=nchevobbe,bomsy
|
|
||
Updated•4 years ago
|
|
|
||
Updated•4 years ago
|
|
|
||
Updated•4 years ago
|
|
|
||
Updated•4 years ago
|
|
|
||
Updated•4 years ago
|
|
|
||
Updated•4 years ago
|
|
|
||
Updated•4 years ago
|
|
||
Comment 17•4 years ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/552023cfd750
https://hg.mozilla.org/mozilla-central/rev/3a94715fdff0
https://hg.mozilla.org/mozilla-central/rev/828c62bfc0a2
Description
•