1427077 - Fail to run devtools network monitor component at browser

Status: RESOLVED FIXED

(DevTools :: Netmonitor, defect, P2)

Description

[Rongjian Zhang]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_13_2) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/63.0.3239.108 Safari/537.36

Steps to reproduce:

1. Clone `mozilla-central` repo
2. Go to `devtools/client/netmonitor` folder
3. run `yarn install; yarn start`
4. Open `http://localhost:8000`


Actual results:

Browser throw an error `loader is not defined`


Expected results:

It should throw no error and works correctly

Comment 1

[Rongjian Zhang]

Attachment: Bug 1427077 - Make devtools/client/netmonitor work correctly at browser

Replace `loader.lazyGetter` pattern with normal variable assignment

Review commit: https://reviewboard.mozilla.org/r/209360/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/209360/

Comment 2

[[e-mail]]

Thanks for the patch! We just come back from the year-end vacation, will review it soon.

Ricky, could you take a look on this patch?

Comment 3

[Ricky Chien [:rickychien] (inactive)]

Comment on attachment 8938860 [details]
Bug 1427077 - Make devtools/client/netmonitor work correctly at browser

https://reviewboard.mozilla.org/r/209360/#review215318

Thanks for the patch @pd4d10! Using rewrite-lazy-getter.js is great for solving this issue. Patch has verfifed and launchpad works right now. \o/

I'm going to approve this patch since right now rewrite-lazy-getter makes sense in our case. Please fix the issue before landing this patch.

::: devtools/client/netmonitor/src/components/PropertiesView.js:18
(Diff revision 1)
>  const { FILTER_SEARCH_DELAY } = require("../constants");
>  
>  // Components
>  const TreeViewClass = require("devtools/client/shared/components/tree/TreeView");
>  const TreeView = createFactory(TreeViewClass);
>  const SearchBox = createFactory(require("devtools/client/shared/components/SearchBox"));

Searchbox rendering will be determined by this.shouldRenderSearchBox(), so it's necessary to be imported via loader.lazyGetter.

Comment 4

[Rongjian Zhang]

Comment on attachment 8938860 [details]
Bug 1427077 - Make devtools/client/netmonitor work correctly at browser

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/209360/diff/1-2/

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/591cf97d4777
Make devtools/client/netmonitor work correctly at browser r=rickychien

Comment 6

[Noemi Erli[:noemi_erli]]

Backed out changeset 591cf97d4777 (bug 1427077) for ES failures in /builds/worker/checkouts/gecko/devtools/client/shared/webpack/rewrite-lazy-getter.js r=backout on a CLOSED TREE

Backout: https://hg.mozilla.org/integration/autoland/rev/0eb526012bdb75e7d27e1e5064d0040cb0a39ff4

Push that was backed out: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=591cf97d4777207e6b6964d28f66ce61795d77d2&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=success&filter-resultStatus=pending

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/mozilla-central/rev/379d966c93d2
Make devtools/client/netmonitor work correctly at browser r=rickychien
https://hg.mozilla.org/mozilla-central/rev/e6aed81c3f04
Backed out changeset 591cf97d4777 for ES failures in /builds/worker/checkouts/gecko/devtools/client/shared/webpack/rewrite-lazy-getter.js r=backout on a CLOSED TREE

Comment 8

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/379d966c93d2

Comment 9

[Ricky Chien [:rickychien] (inactive)]

Reopen this bug since patch has been backout for ES linter failures.

pd4d10, could you take a look? thanks

Comment 10

[Ricky Chien [:rickychien] (inactive)]

Comment on attachment 8938860 [details]
Bug 1427077 - Make devtools/client/netmonitor work correctly at browser

https://reviewboard.mozilla.org/r/209360/#review216058

::: devtools/client/shared/webpack/rewrite-lazy-getter.js:5
(Diff revision 2)
> +/* This Source Code Form is subject to the terms of the Mozilla Public
> + * License, v. 2.0. If a copy of the MPL was not distributed with this
> + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
> +
> + "use strict";

nit: remove whitespace before "use strict";

Comment 11

[Rongjian Zhang]

(In reply to Ricky Chien [:rickychien] from comment #10)
> Comment on attachment 8938860 [details]
> Bug 1427077 - Make devtools/client/netmonitor work correctly at browser
> 
> https://reviewboard.mozilla.org/r/209360/#review216058
> 
> ::: devtools/client/shared/webpack/rewrite-lazy-getter.js:5
> (Diff revision 2)
> > +/* This Source Code Form is subject to the terms of the Mozilla Public
> > + * License, v. 2.0. If a copy of the MPL was not distributed with this
> > + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
> > +
> > + "use strict";
> 
> nit: remove whitespace before "use strict";

Hi Ricky, I remove whitespace and run `hg commit --amend`, then `hg push review`, receiving messages below:

remote: adding changesets
remote: adding manifests
remote: adding file changes
remote: added 1 changesets with 1 changes to 4 files (+1 heads)
remote: recorded push in pushlog
submitting 1 changesets for review
abort: Review request is submitted or discarded.
You must reopen the review request before it can be updated.
Review requests should only be reopened if your changes have not landed or have
been backed out - file new bugs for follow-up work.

Seems that the new commit has been pushed to remote successfully, but the review request failed. Did I miss something?

Comment 12

[Ricky Chien [:rickychien] (inactive)]

Attachment: Screen Shot 2018-01-08 at 5.07.39 PM.png

Hi pd4d10,

The only thing you need to do for re-submitting to a closed review is to reopen the reviewboard, and then you can do `hg push review` again to overwrite the original commit as usual.

Comment 13

[Ricky Chien [:rickychien] (inactive)]

For how to reopen closed review, see attachment 8940648 [details].

Comment 14

[Rongjian Zhang]

Comment on attachment 8938860 [details]
Bug 1427077 - Make devtools/client/netmonitor work correctly at browser

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/209360/diff/2-3/

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7c76e6168da7
Make devtools/client/netmonitor work correctly at browser r=rickychien

Comment 16

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/7c76e6168da7