Closed
Bug 1399390
Opened 7 years ago
Closed 7 years ago
Net monitor broken when running on top of the Launchpad again
Categories
(DevTools :: Netmonitor, enhancement, P1)
DevTools
Netmonitor
Tracking
(firefox57 fixed)
RESOLVED
FIXED
Firefox 57
Tracking | Status | |
---|---|---|
firefox57 | --- | fixed |
People
(Reporter: Honza, Assigned: Honza)
References
Details
Attachments
(2 files)
1.97 KB,
patch
|
Details | Diff | Splinter Review | |
59 bytes,
text/x-review-board-request
|
jdescottes
:
review+
|
Details |
Bug 1396600 introduced lazy loading of modules: For example (devtools/shared/css/color.js): loader.lazyRequireGetter(this, "CSS_ANGLEUNIT", "devtools/shared/css/properties-db", true); Webpack is not resolving such require and there is runtime error (displayed in the Console panel) complaining about undefined 'loader' object. Honza
Assignee | ||
Comment 1•7 years ago
|
||
We need automated tests since it's currently simple to break WebPack bundling. Should be done in separate bug. Honza
Assignee | ||
Comment 2•7 years ago
|
||
Here is a workaround for anyone who want to run the Network panel with the Launchpad in the meantime. Honza
Assignee | ||
Comment 3•7 years ago
|
||
Perhaps we could change: loader.lazyRequireGetter(this, "CSS_ANGLEUNIT", "devtools/shared/css/properties-db", true); to: const {CSS_ANGLEUNIT} = require("lazy-loader!devtools/shared/css/properties-db"); 1) And implement the support in DevTools loader (similarly to how 'raw' prefix is done) 2) Plus, introduce simple 'lazy-loader' webpack loader (so webpack understands the prefix) @Alex What do you think about #1? Honza
Flags: needinfo?(poirot.alex)
Comment 4•7 years ago
|
||
I don't see how lazy-loader! would be implemented? Can you do like inspector and have this rewrite? http://searchfox.org/mozilla-central/source/devtools/client/inspector/webpack/rewrite-lazy-require.js
Flags: needinfo?(poirot.alex)
Comment hidden (mozreview-request) |
Assignee | ||
Updated•7 years ago
|
Comment 6•7 years ago
|
||
Comment on attachment 8907617 [details] Bug 1399390 - Use lazy-require in Netmonitor's webpack.config; Sorry, I know nothing about webpack.
Attachment #8907617 -
Flags: review?(poirot.alex) → review?(jdescottes)
Comment 7•7 years ago
|
||
mozreview-review |
Comment on attachment 8907617 [details] Bug 1399390 - Use lazy-require in Netmonitor's webpack.config; https://reviewboard.mozilla.org/r/179300/#review184428 ::: devtools/client/netmonitor/webpack.config.js:39 (Diff revision 1) > */ > test: /\.js/, > loader: "rewrite-raw", > }, > + { > + test: /client(\/|\\)inspector(\/|\\).*\.js$/, you probably want netmonitor instead of inspector in the regexp here?
Attachment #8907617 -
Flags: review?(jdescottes)
Comment hidden (mozreview-request) |
Assignee | ||
Comment 9•7 years ago
|
||
(In reply to Julian Descottes [:jdescottes] from comment #7) > you probably want netmonitor instead of inspector in the regexp here? yes, fixed. (btw. Alex, I forwarded the review to :gl since the patch is changing bits from the Inspector dir) Honza
Comment 10•7 years ago
|
||
mozreview-review |
Comment on attachment 8907617 [details] Bug 1399390 - Use lazy-require in Netmonitor's webpack.config; https://reviewboard.mozilla.org/r/179300/#review184458 I can still review this. The inspector's webpack config is not working at the moment so feel free to go forward with your patch.
Attachment #8907617 -
Flags: review+
Comment 11•7 years ago
|
||
mozreview-review |
Comment on attachment 8907617 [details] Bug 1399390 - Use lazy-require in Netmonitor's webpack.config; https://reviewboard.mozilla.org/r/179300/#review184586 I think we should add a README to the /shared/webpack/ folder to explain what is in this folder, who consumes the modules and how it is used.
Attachment #8907617 -
Flags: review?(gl)
Comment hidden (mozreview-request) |
Assignee | ||
Comment 13•7 years ago
|
||
(In reply to Julian Descottes [:jdescottes] from comment #10) > Comment on attachment 8907617 [details] > Bug 1399390 - Use lazy-require in Netmonitor's webpack.config; > > https://reviewboard.mozilla.org/r/179300/#review184458 > > I can still review this. The inspector's webpack config is not working at > the moment so feel free to go forward with your patch. Thanks for the review Julian! (In reply to Gabriel [:gl] (ΦωΦ) from comment #11) > I think we should add a README to the /shared/webpack/ folder to explain > what is in this folder, who consumes the modules and how it is used. Done Honza
Comment 14•7 years ago
|
||
Pushed by jodvarko@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/2194f20eac24 Use lazy-require in Netmonitor's webpack.config; r=jdescottes
Comment 15•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/2194f20eac24
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox57:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 57
Updated•6 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•